Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trying to save after successful import fails #1777

Closed
mtissington opened this Issue Mar 2, 2019 · 19 comments

Comments

Projects
None yet
3 participants
@mtissington
Copy link

mtissington commented Mar 2, 2019

OS - Windows 10 x64
DB build latest nightly 2nd March

I have a reasonably sized sql file with a bunch of delete tables, create tables and insert data etc.

I successfully import the file (could be a new database or an existing database).

When I click on Write Changes (to save the database) I get the attached dialog.

multi png

The release build does not have this issue.

@justinclift justinclift added the bug label Mar 2, 2019

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Mar 2, 2019

Thanks @mtissington. Guessing here, but it's probably a bug introduced with 0f6946c.

Any interesting in trying a build from the day prior?

If so, I think this should be the right one. 😄

@mtissington

This comment has been minimized.

Copy link
Author

mtissington commented Mar 2, 2019

confirmed 02-25 works correctly

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Mar 2, 2019

Cool. 😄

The bug should be present in this one too then, if you're ok to check and confirm:

If it is, that pretty much guarantees 0f6946c is the problem. 😄

@mtissington

This comment has been minimized.

Copy link
Author

mtissington commented Mar 2, 2019

Confirmed - that was the build I had installed prior to needing to install the release build.

@mtissington

This comment has been minimized.

Copy link
Author

mtissington commented Mar 2, 2019

FYI - the import file I'm using has both a BEGIN TRANS and a COMMIT.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Mar 2, 2019

Thanks @mtissington. 😄

Btw, TRANS or TRANSACTION?

@mtissington

This comment has been minimized.

Copy link
Author

mtissington commented Mar 2, 2019

My mistake - BEGIN TRANSACTION;

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Mar 2, 2019

Cool. 😄

That commit is @MKleusberg's, so he'll probably get the fixing started (etc) sometime over the next few days. 😄

MKleusberg added a commit that referenced this issue Mar 3, 2019

Fix multiple problems in 0f6946c
Fix multiple problems which were introduced in commit
0f6946c. They all are about detection
of transaction statements. First it turned out that the commit slightly
changed the behaviour of the dirty parameter which, although there did
not seem to be any consequences of this, was turned back into the old
behaviour. Then the detection of transaction statements depended on the
structure updated flag which is totally wrong and needed to be fixed.
Next, it turned out that SQLite executed ";COMMIT;" as one statement
instead of two. This means we need to skip until after the first
semicolon and not until before it. And finally, after skipping until
after the semicolon we should perform the check for transaction
statements again before blindly giving the next statement to SQLite.

See issue #1777.
@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Mar 3, 2019

Thanks, @mtissington, for the as always helpful report! 😄 The fix for this was trickier than expected because there were multiple problems involved which all yield the same problem. But I hope I could catch them all. Can you check again with tomorrow's nightly build if it works for your file as well?

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Mar 3, 2019

Not sure if it'll help, but I've just kicked off the Win64 nightly build script manually. A rebuilt nightly for today (with the new commit included) should be online in ~40 mins. 😄

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Mar 3, 2019

On that note, we'd probably find decent use in having automatic builds for Windows and macOS too, like we now have for Linux.

@mtissington

This comment has been minimized.

Copy link
Author

mtissington commented Mar 3, 2019

Sorry guys, I've just downloaded the latest nightly 3rd March, installed it and I get the same error dialog..
(check the file version/date to confirm I was running the correct version)

@mtissington

This comment has been minimized.

Copy link
Author

mtissington commented Mar 3, 2019

@MKleusberg happy to send you the sql file privately if needed.

@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Mar 6, 2019

@mtissington Yes, that would definitely help 😄 It works for my test cases and I'm not sure what's different in your file. You can send the file to mkleusberg@gmail.com.

@mtissington

This comment has been minimized.

Copy link
Author

mtissington commented Mar 6, 2019

@MKleusberg email sent

@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Mar 25, 2019

Sorry for the late reply! I actually tried using your file and it just worked fine for me but then I didn't have enough time to investigate further.

But now seeing that issue #1814 isn't working either I start to believe that this issue here might have the same build problem. So if there is a build problem in #1814, it's worth checking this issue again after the build problem is solved.

@justinclift justinclift referenced this issue Mar 26, 2019

Closed

3.11.2 - outstanding pieces #1773

13 of 13 tasks complete
@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Mar 30, 2019

@mtissington Would you have time to check if this is fixed?

@justinclift justinclift changed the title Trying to save after successfull import fails Trying to save after successful import fails Mar 30, 2019

MKleusberg added a commit that referenced this issue Mar 30, 2019

Fix multiple problems in 0f6946c
Fix multiple problems which were introduced in commit
0f6946c. They all are about detection
of transaction statements. First it turned out that the commit slightly
changed the behaviour of the dirty parameter which, although there did
not seem to be any consequences of this, was turned back into the old
behaviour. Then the detection of transaction statements depended on the
structure updated flag which is totally wrong and needed to be fixed.
Next, it turned out that SQLite executed ";COMMIT;" as one statement
instead of two. This means we need to skip until after the first
semicolon and not until before it. And finally, after skipping until
after the semicolon we should perform the check for transaction
statements again before blindly giving the next statement to SQLite.

See issue #1777.

@MKleusberg MKleusberg self-assigned this Mar 30, 2019

@MKleusberg MKleusberg added this to the 3.11.2 milestone Mar 30, 2019

@mtissington

This comment has been minimized.

Copy link
Author

mtissington commented Mar 30, 2019

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Mar 30, 2019

Fantastic. Thanks heaps @mtissington. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.