Releasing reserved bytes on decrypting about sqleet HOT 4 CLOSED

Thanks for your work. The comment is indeed inaccurate and IIRC I had to apply similar patch in sqlite3RunVacuum to get it to actually truncate the pages. This resulted in corrupted databases in my experiments, but it is possible that I did something wrong. The corruption might have happened after performing write operations on the decrypted database.

Would you consider to apply this change?

The downside is that it will affect negatively the forward & backward compatibility across SQLite versions because sqlite3RunVacuum tends to get updated now and then. But an intuitively working sqlite3_rekey_v2 decrypting is so big win that I am willing to merge such a patch if it does not cause any major issues (like corrupted databases). This means that sqleet will have to target only the latest version of SQLite3 in the future, but I guess that is completely acceptable. Especially now that the versioning has been changed to match SQLite3 version numbers.

Feel free to send me a PR if you have the patch ready. However, I'd prefer it if you save the modified version of sqlite3RunVacuum in vacuum.c and include that in sqleet.c. This would probably help a bit with keeping the function up to date when SQLite3 releases a new version.

from sqleet.

AFAICT the adjusted version of sqlite3RunVacuum seems to work without problems, but I agree that more thorough tests should be conducted to be sure that database corruption is unlikely to happen.

It is true that sqlite3RunVacuum gets updated from time to time (changes occurred in August 2017, June 2017, and several in 2016). OTOH SQLite claims that the database format is almost completely compatible among all 3.x versions. So I assume that executing a VACUUM command should usually not corrupt a database file, even if the database file was created with a later version.

Nevertheless, on upgrading the included SQLite version changes to sqlite2RunVacuum - if any - should be also incorporated. This slightly increases the effort to upgrade to a new SQLite version, but it's certainly worth it.

from sqleet.

Yes. Due to the stable database format, it is unlikely that using mismatching versions of SQLite3 and sqlite3RunVacuum somehow corrupts the database or works incorrectly. I am more worried about the private internals of SQLite3 changing so that sqleet's modified vacuum code no longer compiles straight out of the box. But such a forward/backward compatibility break is not a deal-breaker for this.

Btw, I figured out why I got corrupted databases before. It was because I manipulated the return value of sqlite3BtreeGetOptimalReserve by overwriting pBt->pageSize, pBt->usableSize and pBt->optimalReserve (since sqlite3BtreeSetPageSize had no effect). This seemed to work in some cases, but in other cases corrupted the DB.

from sqleet.

I am more worried about the private internals of SQLite3 changing so that sqleet's modified vacuum code no longer compiles straight out of the box.

Well, that can happen for other reasons, too. I had to adjust my wxSQLite3 encryption extension several times in the course of time, since for example the codec handling changed. This is to be expected in the future again. Since the encryption extension is usually bundled with a certain SQLite version, it can make the upgrading process a bit more difficult.

Yes, tampering with pagesize and optimal reserve can have bad side effects. In conjunction with sqlite3RunVacuum calling the function sqlite3BtreeSetPageSize has no effect, since sqlite3RunVacuum calls the function internally with different values. Therefore the adjusted vacuum function is needed.

from sqleet.