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

Importing Records With Existing Data Creates 'Invalid File Format' Error And Hangs DB4S #1590

Closed
chrisjlocke opened this Issue Oct 23, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@chrisjlocke
Copy link
Contributor

chrisjlocke commented Oct 23, 2018

Details for the issue

This semi-relates to an issue raised in #1585. Upon further testing, a reproducible set of steps was identified.

When importing data into a table, if an imported record fails a constraint check (eg, primary key violation, eg, importing record #1 again) DB4S throws up a 'invalid file format' error, then a further 'the system is busy - do you want to cancel?' dialog. If the user presses No, DB4S locks up*.

*DB4S didn't come out of the 'not responding state'

Tried the alpha, nightly, and the recent 'special build' for #1575.
See screencast: https://screencast-o-matic.com/watch/cF60IJYAsP

If record #1 is removed from the table and the import retried, then DB4S reports the same error, but this time on record #2. This confirms something. Not sure what. But something.

image

What did you do?

Imported records from a CSV file, which was from an exported table. The import was into the same table, so records were known to violate the primary key.

What did you expect to see?

The records known to violate the primary key would be ignored.

What did you see instead?

DB4S got kicked in the nuts, fell down on its knees and didn't get back up again.

Useful extra information

Please see the below screencast:
https://screencast-o-matic.com/watch/cF60IJYAsP

Note clicking Yes or No both locks up DB4S. So you're stuffed either way. 😆

The info below often helps, please fill it out if you're able to. :)

What operating system are you using?

  • Windows: ( version: 10_ )
  • Linux: ( distro: ___ )
  • Mac OS: ( version: ___ )
  • Other: ___

What is your DB4S version?

  • 3.11.0-alpha*
  • 3.10.1
  • Other: ___

Did you also

@chrisjlocke chrisjlocke changed the title Importing Records With Existing Data Creates 'Invalid File Format' Error Importing Records With Existing Data Creates 'Invalid File Format' Error And Hangs DB4S Oct 23, 2018

@MKleusberg MKleusberg self-assigned this Oct 23, 2018

MKleusberg added a commit that referenced this issue Oct 23, 2018

Better error handling in import CSV dialog
Make sure to show the correct error message when there is an error
during CSV import.

Make sure to release the DB handle used for the import before rolling
back to the last savepoint in case of an error in the CSV import. This
avoids a deadlock situation.

See issue #1590.

MKleusberg added a commit that referenced this issue Oct 23, 2018

Better error handling in import CSV dialog
Make sure to show the correct error message when there is an error
during CSV import.

Make sure to release the DB handle used for the import before rolling
back to the last savepoint in case of an error in the CSV import. This
avoids a deadlock situation.

See issue #1590.
@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Oct 23, 2018

Thanks Chris! That's a good find and a very helpful error description 👍 I think I have fixed both issues, wrong error message and crash. Can you check tomorrow's nightly?

I have also already cherry-picked this commit to the v3.11.x branch as it fixes an important regression which was introduced after 3.10.1 - it also doesn't change any user-visible strings, so no translation update needed 😄

@chrisjlocke

This comment has been minimized.

Copy link
Contributor Author

chrisjlocke commented Oct 24, 2018

Credit goes to @antonybits who mentioned the error in #1585, but in his case it raised different errors.... so we'll see what you've corrected and how far it now gets! 😉

@chrisjlocke

This comment has been minimized.

Copy link
Contributor Author

chrisjlocke commented Oct 24, 2018

HAve just tried the latest nightly, and while there is a more 'valid' error:

image

the import terminates.

image

Would it be possible to continue, or because we've received an error, and potentially that error could be anything, its 'safer' to terminate?
If the former, then would it be also possible to ignore certain errors?

Theoretically, if I've deleted some records but have a table/database backup, I could import just the missing records. I can't do that currently, as the import terminates at an error. In an ideal world, DB4S could throw up a 'Retry, Ignore, Cancel' dialog...

image

This could give the user a chance to open the database in another instance to correct the error, and retry.

@chrisjlocke

This comment has been minimized.

Copy link
Contributor Author

chrisjlocke commented Oct 24, 2018

Would still like an answer to the above, but I'll close this issue, as the main problem has been resolved. The above question is more in line with answering #1585.

@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Oct 24, 2018

Thanks for testing, Chris! You're quite right about the other point. I think your dialog would also need a 'Remember this action' checkbox or some similar option for larger CSV files. Maybe we also need to distinguish between different sorts of errors for that - but that is quite tricky because we only get a very vague error code from SQLite and the error string as seen in the message box. The first probably doesn't contain enough information on the error to be of any help here and the second is subject to change and hard to parse. I guess the easiest solution is to just end the import as we do now but add a dropdown box to allow changing the ON CONFLICT strategy, just as @antonybits suggested in #1585. The only downside is that you might have to start the import a second time if you didn't get the settings right on the first run. But since the import is so much faster now... 😝

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment