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

Hitting "Delete" key doesn't do what you expect, kills app, and probably your DB #1391

Closed
broofa opened this Issue May 18, 2018 · 9 comments

Comments

Projects
None yet
3 participants
@broofa
Copy link

broofa commented May 18, 2018

  1. Have a table with a unique primary key constraint on at least one column. Have 1,500 rows in that table.
  2. Go browse that table
  3. Select All
  4. Hit the "Delete" key

Expected result: All rows get deleted from the DB in a single transaction (preceded by a "Are you sure you want to do this?" dialogue, of course). Note: This is the behavior for the Sequel Pro app.

Actual result: App tries to null out every single field of every row, using a separate query for each [field,row] tuple, which seems horribly inefficient but hey, it works.

... except it doesn't. Instead, the primary key constraint gets violated and you start getting alerts about Unique Constraint Failed. Click "okay"... another alert... click "okay" again... another alert ... repeat 1,500 times for every row in your table. Or, in practice, resign yourself to the app being toast and kill -9 it from the command line.

And for bonus points, notice that your DB has been corrupted because only some of the fields in some of the rows have been set to null and... ah, screw it, I'll just restore from backup.

(But, thank you for an awesome app! Seriously. It's been really helpful!)

@broofa broofa changed the title Hitting "Delete" key doesn't do what you expect, and also kills app Hitting "Delete" key doesn't do what you expect, kills app, and probably your DB May 18, 2018

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented May 18, 2018

Ouch, that definitely sounds like a bug.

Which version is this happening in, and on which OS? 😄

@justinclift justinclift added the bug label May 18, 2018

@broofa

This comment has been minimized.

Copy link
Author

broofa commented May 19, 2018

MAcOS 10.13.2
DB Browser for SQLite 3.10.1
Qt Version 5.7.1
SQLCipher Version 3.15.2

@broofa

This comment has been minimized.

Copy link
Author

broofa commented May 19, 2018

Note: The reason I'm getting the unique constraint error is that the primary key column doesn't have NOT NULL on it (my bad). So once the first row's key is NULL'ed out, attempts to null out the key in subsequent rows fail.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented May 19, 2018

Thanks. If you have a minute to try with our latest nightly version then it's possible this might be fixed already. We're getting kinda close to the next release, so there's lots of bug fixes / tweaks / similar in our current nightly builds. 😄

    https://nightlies.sqlitebrowser.org/latest/

If that helps. 😉

@broofa

This comment has been minimized.

Copy link
Author

broofa commented May 19, 2018

Still an issue

image

image

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented May 19, 2018

Doh! Oh well. So much for hoping.

Thanks for checking @broofa. 😁

MKleusberg added a commit that referenced this issue May 20, 2018

When selecting an entire row and deleting it, remove the record
When selecting an entire row or multiple entire rows in the table view
and hitting the delete key, delete the records and not just the cell
contents.

See issue #1391.
@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented May 20, 2018

Can you try again with tomorrow's nightly build, @broofa? I have just changed it so that selecting an entire row (or multiple rows, or all rows as in your case) and hitting the delete key actually deletes the record(s) and doesn't just clear the cell contents. That should solve the immediate issue you're facing here 😄

@broofa

This comment has been minimized.

Copy link
Author

broofa commented May 25, 2018

Appears to be fixed in nightly. Feel free to close. Thanks!

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented May 25, 2018

Excellent. Thanks for checking @broofa. 😄

Well done @MKleusberg. 😁

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