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?

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....

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.

No, it was a new database, so no indexes ... fresh empty database.

Whoa. Something's not right then. 😦

@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?

@chrisjlocke Ping :D

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?

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.

@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). 😄

@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? 😄

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.

Cool, sounds like we're making progress at least. 😄

One interesting thing though. The export of a database is still a 'transaction'. The export begins 'begin transaction' and end 'commit'. However, if you cancel the import, it doesn't cancel the transaction - it has completed up until you cancelled it. This would explain the slowness of the import - each import statement is its own transaction. On the sqlite website, it mentions this is 'bad' as there is a limit of a few dozen transactions per second.
https://www.sqlite.org/faq.html#q19

I assume the import now ignores the transaction in the exported SQL, but does it add its own?

I appreciate I can revert changes in DB4S, but a transaction at the import level would greatly speed up the import. I assume that was the 'original plan', but it was being done twice - at the export into the SQL, and again at the import.

Yep, that's interesting.

My reading of this fix is that it just removes all occurrences of BEGIN TRANSACTION and COMMIT on the fly from the SQL file being read in.

In theory, DB4S is supposed to be adding it's own BEGIN TRANSACTION and COMMIT at 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. 😇

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?

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.

Yeah, that's a good idea. 😄

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. 😵

what happens if you issue a ROLLBACK from the Execute SQL window?

Is that the same as clicking 'Discard Changes' ?
Rollback deleted the half-imported table, so that would imply it was in a transaction?

k, that Rollback behaviour does indicate it was in a transaction.

... with 'Discard Changes'... I'm personally not sure atm.

So there aren't multiple transactions? I looked a bit over the code and the only thing I saw related to this was a setSavepoint call, which actually executes the query SAVEPOINT <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 setSavepoint call, 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.

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

I re-read the SAVEPOINT documentation 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 the setSavepoint call.

Yep, same thought here. Measured stats would definitely help.

Would you be ok to investigate _after_ you're back from vacation?

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.

Sure thing.

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'.