Repository navigation
Create temp view incorrectly marks RO DB "dirty" #1514
Description
Details for the issue
What did you do?
Open a DB Read only.
In Execute SQL tab, execute
create temp view foo as select * from table1;
File => Close DB
What did you expect to see?
Nothing
What did you see instead?
? Do you want to save the changes made to the database file ...?
While it is reasonable not to parse all the commands & responses in an "Execute" tab to determine if actions modify the database, it is guaranteed (I hope) that a database open as Read-Only can not be modified!
So, Option 1: Don't pay attention to the "Dirty" flag if a DB is open read-only,
However, it appears that you may be able to use SQLite to detect a modified DB fairly painlessly.
Option 2: Section 1.3.6 of the DB header spec says:
The file change counter is a 4-byte big-endian integer at offset 24 that is incremented whenever the database file is unlocked after having been modified. When two or more processes are reading the same database file, each process can detect database changes from other processes by monitoring the change counter. A process will normally want to flush its database page cache when another process modified the database, since the cache has become stale. The file change counter facilitates this.
In WAL mode, changes to the database are detected using the wal-index and so the change counter is not needed. Hence, the change counter might not be incremented on each transaction in WAL mode.
So unless the DB is in WAL mode, you can rely on it instead of your own dirty bit. (The API indicates that sqlite_file_control(SQLITE_FCNTL_FILE_POINTER) returns a pointer to sqlite3_file object, which includes a pointer to a sqlite3_io_methods object, which should allow you to xRead the header) There should be an API to access this directly, but a quick scan of the documentation didn't turn it up.
If this works as expected, you would not have to maintain your own "dirty" flag for R/W databases either. I would expect that to simplify your code, even if you have to save/restore the journaling mode to avoid WAL.
Useful extra information
The info below often helps, please fill it out if you're able to. :)
What operating system are you using?
- Windows: ( version: 10 pro___ )
- Linux: ( distro: ___ )
- Mac OS: ( version: ___ )
- Other: ___
What is your DB4S version?
- 3.10.1
- 3.10.0
- 3.9.1
- Other: ___Aug 23 nightly
Did you also
- Try out the latest nightly version: https://github.com/sqlitebrowser/sqlitebrowser#nightly-builds
- Search for an existing similar issue: https://github.com/sqlitebrowser/sqlitebrowser/issues?utf8=%E2%9C%93&q=is%3Aissue%20
Activity
sky5walk commentedon Aug 23, 2018
I had a similar problem WITH RECURSIVE queries.
chrisjlocke commentedon Aug 24, 2018
Well, not really. Open DB4S. Open a database. Do some mundane SELECT queries. Open another copy of DB4S. Add a table. Return to copy #1. Close database. "Do you want to save changes?" as the change counter has changed. Bummer.
tlhackque commentedon Sep 6, 2018
You're right, of course. That "solution" is invalid. I must have been in single user mode. (Your quote omitted the qualifier 'If this works as expected,')
It's still a bug - a read-only database can't have changes to be saved. And even a writable database is not modified by creating a TEMP view.
At the risk of being wrong again, sqlite3_total_chantges() being non-zero may work. The documentation doesn't talk about metadata changes (setting a pragma, adding a column, creating a table, index or view).
Although the doc says that only INSERT/UPDATE/DELETE update the count, perhaps metadata changes turn into INSERT/DELETE/UPDATE of SQLITE_MASTER internally. Even if they don't, they're a lot simpler to parse than those three...
I suppose I should leave the solution to you.
mgrojo commentedon Sep 6, 2018
That function is only counting rows, not changes to the DB structure.
I don't know if this has ever been consider, but I think it wouldn't work. You can easily test it using the total_changes() SQL function. A simple CREATE TABLE statement does not increment the counter.
Nevertheless, it might be useful for ensuring that we don't miss data changes if our current guessing fails to spot them.
Never mark the DB dirty if it is read-only. Issue #1514
chrisjlocke commentedon Jan 4, 2019
@tlhackque - Can you check via the nightlies whether this issue is still an issue?
mgrojo commentedon Jan 4, 2019
I have to warn that the referred commit is not included in the master branch (and consequently not present in the nightlies). It is a workaround for the problem (not the ideal solution) and it is pending to be accepted or rejected in pull request #1536. It can only be tested compiling from the corresponding branch.
justinclift commentedon Jan 4, 2019
Ahhh, should we create Win and OSX builds with it, for people to test?
mgrojo commentedon Jan 4, 2019
Not sure if it would pay off. Maybe I should withdraw it, because it'd have to be rewritten. Wouldn't be too difficult, but @MKleusberg wasn't very convinced, nevertheless.
chrisjlocke commentedon Jan 15, 2019
Can't we just have a 'read only' flag, set when 'open database read only' is opened, *or* we open a database and the file permissions on that are read only.
Then, before the 'do you want to save changes?' dialog, we can check the flag... if yes (ie, read only) don't display the dialog.
Never mark the DB dirty if it is read-only. Issue #1514