Repository navigation
Cannot import export of SQL table #568
Description
I had a database that threw up an error- 'database disk image is malformed', but DB Browser was able to export the tables to .SQL files.
However, when I created a new database and told DB Browser to import this SQL file, it told me , "Nope" - "Error importing data: Error in statement #1: cannot start a transaction within a transaction.
Aborting execution".
Indeed, the SQL's first command is 'Begin Transaction'...
I presume the import routine should say, 'if the first line is a begin transaction, then I'll do nothing, otherwise I'll add one...'
Or do we have to delete the generated 'begin transaction' lines from the SQL before importing?
DB Browser: 3.8.0
OS: Windows 8.1 64bit
Activity
justinclift commentedon Apr 19, 2016
Hmmm, yeah, that sounds like a design bug. 😦
As you mention, does the removing the BEGIN TRANSACTION statement at the start (and the COMMIT at the end) allow the import to succeed?
chrisjlocke commentedon Apr 19, 2016
It does, yes. I was going to post that the 'executing sql...' dialog was stuck and didn't update, but its just doing it v..e..r..y slowly. The .sql has 505,000 lines, and after 3 hours, its on 9%. The import is going to take a while....
justinclift commentedon Apr 19, 2016
Oh wow, that's exceptionally slow.
Are there pre-existing indexes on some of the fields or something? If so - as a hint for future imports - it's a good idea to remove the indexes before larger imports, then recreating them afterwards. That way it doesn't try to update the index for every single imported row (super slow), and just does a single big one at the end.
chrisjlocke commentedon Apr 20, 2016
No, it was a new database, so no indexes ... fresh empty database.
justinclift commentedon Apr 20, 2016
Whoa. Something's not right then. 😦
justinclift commentedon Jul 15, 2016
@chrisjlocke Interested in testing out a fix for this?
@revolter Has created a proposed PR ready for testing. I can create a Windows package with the fix if you're ok to test?
revolter commentedon Jul 19, 2016
@chrisjlocke Ping :D
chrisjlocke commentedon Jul 19, 2016
Sorry - email has been playing up and I didn't see this notification...
Sure - happy to test all fixes! Should it fix the 'not in a transaction' bug, or the speed bug? Or should I just report my findings? Aah, just saw #653, so will be the first one then...
I presume this is in the nightly Windows download?
revolter commentedon Jul 19, 2016
No problem.
It's about the nested transaction indeed.
And, no, the PR that fixes it isn't merged yet. Maybe @justinclift can help us out with an once-off Windows build.
justinclift commentedon Jul 19, 2016
@chrisjlocke Try this one:
http://nightlies.sqlitebrowser.org/win64-onceoffs/sqlitebrowser-win64-issue556v1-201607152315.exe
Made that for the same issue (reported also in #556). 😄
justinclift commentedon Jul 25, 2016
@chrisjlocke If/when you have a minute to test, the latest nightly build should include the fix for this. Any chance you could give it a shot to double check? 😄
chrisjlocke commentedon Jul 25, 2016
Yes, the import now works. The only 'niggle' is that the cursor remains the 'waiting' circle of doom when the 'Import completed' message box appears.
This also occurs if you import where the table already exists - DB4S correctly moans, but the cursor is 'waiting'. Someone might think, "blimey - this is taking a while" when in fact its just the cursor hasn't changed.
I'm just going to test the '500,000 lines seems slow' thing. That might have just been my end.
2 remaining items
justinclift commentedon Jul 25, 2016
Yep, that's interesting.
My reading of this fix is that it just removes all occurrences of
BEGIN TRANSACTIONandCOMMITon the fly from the SQL file being read in.In theory, DB4S is supposed to be adding it's own
BEGIN TRANSACTIONandCOMMITat the start and end of each SQL file import. It's weird to me that it's not functioning this way.As a disclaimer though... I haven't personally dug into the data handing side of the code yet. It sounds like there's some weirdness there which needs fixing.
That'll probably be done for 3.10.0, rather than 3.9.0. @innermous has already pointed out that there's some mess in our data handling already (thus our constraints problems), which we need to get sorted out after this upcoming 3.9.0 release. 😇
justinclift commentedon Jul 25, 2016
As a curiosity thing, if after you cancel out of the transaction (and the data is still there), what happens if you issue a ROLLBACK from the Execute SQL window?
revolter commentedon Jul 25, 2016
Well, indeed this is not the best solution, but was the easiest. Throwing an error for an SQL created by the same app is not ok. But for 3.10.0, we could, instead of removing the transaction, detect if the query starts with a transaction and stop starting our own transaction. As this works as the fix intended, could we close it and open another issue regarding the multiple transactions on a big import? So we don't mix things up. And assign it to 3.10.0 too.
justinclift commentedon Jul 25, 2016
Yeah, that's a good idea. 😄
justinclift commentedon Jul 25, 2016
Would either of you be ok to create the new issue for tracking? I've been staring at the computer for too long, and need to take a break for a bit. 😵
chrisjlocke commentedon Jul 25, 2016
Is that the same as clicking 'Discard Changes' ?
Rollback deleted the half-imported table, so that would imply it was in a transaction?
justinclift commentedon Jul 25, 2016
k, that Rollback behaviour does indicate it was in a transaction.
... with 'Discard Changes'... I'm personally not sure atm.
revolter commentedon Jul 25, 2016
So there aren't multiple transactions? I looked a bit over the code and the only thing I saw related to this was a
setSavepointcall, which actually executes the querySAVEPOINT <name>.I read about it now, and it looks like it works inside a transaction. But we are setting the savepoint (which is similar to starting a transaction) before executing the SQL, which in turn starts a transaction.
Instead, we should detect if the SQL contains a transaction. If it does, remove it from the SQL and manually execute it before the
setSavepointcall, then set the savepoint, then execute the rest of the SQL, and it should work. But, knowing that there is this one savepoint already created, and it acts like a transaction, would it make sense to do what I described? If not, should we mark as invalid and close #675 and leave this as it is?It looks like it won't improve the performance at all.
So, yes, there is only one transaction. And the import being slow question remains valid, right?
It took me a while to write this so the ideas might got mangled. Let me know if you understand what I tried to say and what do you think about it.
justinclift commentedon Jul 25, 2016
Hmmm, wonder if the slowdown is related to the Savepoint?
@chrisjlocke Is the database you're importing into on an SSD? eg a high I/O transaction device
revolter commentedon Jul 25, 2016
I re-read the
SAVEPOINTdocumentation and I personally understood that they actually are exactly transaction, but they have a name and you can nest them. So I don't think it should influence the speed. But we can easily test this with a large data set by temporarily removing thesetSavepointcall.justinclift commentedon Jul 25, 2016
Yep, same thought here. Measured stats would definitely help.
Would you be ok to investigate _after_ you're back from vacation?
justinclift commentedon Jul 25, 2016
As an idea - for measuring this it would be useful to record a log of the start and end time for each line of the imported file. Shouldn't be hard, adding some timestamp calls. Millisecond accuracy perhaps?
This would let us determine with reasonable accuracy if there's a slowdown over time during the import, and/or see if it's a steady rate (etc).
[Imagine that info in a graph here] ← 😄
That should give us very useful information for analysing what's going on, and where to look next.
revolter commentedon Jul 26, 2016
Sure thing.
chrisjlocke commentedon Oct 16, 2016
I'm happy to test anything related to this, and in the above issue (# 827) I've posted a link to an SQL file causing 'delays'.