1874951 - (SQLite3.45.1) Upgrade to SQLite 3.45.1

Status: RESOLVED FIXED

(Toolkit :: Storage, task)

Description

[Ryan VanderMeulen [:RyanVM]]

+++ This bug was initially created as a clone of Bug #1858473 +++

Changes from version 3.45.0 (2024-01-15):

• Added the SQLITE_RESULT_SUBTYPE property for application-defined SQL functions. All application defined SQL functions that invokes sqlite3_result_subtype() must be registered with this new property. Failure to do so might cause the call to sqlite3_result_subtype() to behave as a no-op. Compile with -DSQLITE_STRICT_SUBTYPE=1 to cause an SQL error to be raised if a function that is not SQLITE_RESULT_SUBTYPE tries invokes sqlite3_result_subtype(). The use of -DSQLITE_STRICT_SUBTYPE=1 is a recommended compile-time option for every application that makes use of subtypes.
• Enhancements to the JSON SQL functions:
  • All JSON functions are rewritten to use a new internal parse tree format called JSONB. The new parse-tree format is serializable and hence can be stored in the database to avoid unnecessary re-parsing whenever the JSON value is used.
  • New versions of JSON-generating functions generate binary JSONB instead of JSON text.
  • The json_valid() function adds an optional second argument that specifies what it means for the first argument to be "well-formed".
• Add the FTS5 tokendata option to the FTS5 virtual table.
• The SQLITE_DIRECT_OVERFLOW_READ optimization is now enabled by default. Disable it at compile-time using -DSQLITE_DIRECT_OVERFLOW_READ=0.
• Query planner improvements:
  • Do not allow the transitive constraint optimization to trick the query planner into using a range constraint when a better equality constraint is available. (Forum post 2568d1f6e6.)
  • The query planner now does a better job of disregarding indexes that ANALYZE identifies as low-quality. (Forum post 6f0958b03b.)
• Increase the default value for SQLITE_MAX_PAGE_COUNT from 1073741824 to 4294967294.
• Enhancements to the CLI:
  • Improvements to the display of UTF-8 content on Windows
  • Automatically detect playback of ".dump" scripts and make appropriate changes to settings such as ".dbconfig defensive off" and ".dbconfig dqs_dll on".

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Attachment: Bug 1874951 - Upgrade to SQLite 3.45.1. r=mak

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

The SQLITE_DIRECT_OVERFLOW_READ optimization is interesting (in a good way):

When this option is present, content contained in overflow pages of the database file is read directly from disk, bypassing the page cache, during read transactions. In applications that do a lot of reads of large BLOBs or strings, this option improves read performance.

As of version 3.45.0 (2024-01-15), this option is enabled by default. To disable it, using -DSQLITE_DIRECT_OVERFLOW_READ=0.

The btree usage uses sqlite3OsRead which is just a wrapper around a call to the VFS xRead method so there's no actual concerns about our encrypted usage or anything (not that we would expect any, but it's always nice to check). This potentially could do some very nice things to our performance for large structured clone payloads; although our use of WITHOUT ROWID means we get an index b-tree instead of a table b-tree, the docs on b-tree pages do indicate that we end up using overflow pages, with the unique bit being that our keys can overflow there, not just our values.

It might be interesting to see if we see a perf improvement in the new IDB performance tests or if we would need to add larger structured serializations to get a change in numbers.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Try push: https://treeherder.mozilla.org/jobs?repo=try&revision=4138d098b5392b10cff5a3b7eb56a96a700e57bc

Comment 4

[Marco Bonardo [:mak]]

Thank you, I prefer taking this in early 124, so I'll approve it next week.

Comment 5

[Marco Bonardo [:mak]]

we'll take 3.45.1, likely next week.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Changes in this specific patch release, version 3.45.1 (2024-01-30):

• Restore the JSON BLOB input bug, and promise to support the anomaly in subsequent releases, for backward compatibility.
• Fix the PRAGMA integrity_check command so that it works on read-only databases that contain FTS3 and FTS5 tables. This resolves an issue introduced in version 3.44.0 but was undiscovered until after the 3.45.0 release.
• Fix issues associated with processing corrupt JSONB inputs:
  • Prevent exponential runtime when converting a corrupt JSONB into text.
  • Fix a possible read of one byte past the end of the JSONB blob when converting a corrupt JSONB into text.
  • Enhanced testing using jfuzz to prevent any future JSONB problems such as the above.
• Fix a long-standing bug in which a read of a few bytes past the end of a memory-mapped segment might occur when accessing a craftily corrupted database using memory-mapped database.
• Fix a long-standing bug in which a NULL pointer dereference might occur in the bytecode engine due to incorrect bytecode being generated for a class of SQL statements that are deliberately designed to stress the query planner but which are otherwise pointless.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3282e5ae8346
Upgrade to SQLite 3.45.1. r=mak

Comment 8

[pstanciu]

Backed out for causing wpt failures in cache-add.https.any.html.

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | /service-workers/cache-storage/cache-add.https.any.html | Cache.add with request with null body (not consumed) - promise_test: Unhandled rejection with value: object "[Exception... "File error: Corrupted" nsresult: "0x8052000b (NS_ERROR_FILE_CORRUPTED)" location: "<unknown>" data: no]"

Comment 9

[Marco Bonardo [:mak]]

uh, this is apparently in dom cache, Andrew could you please find someone to investigate it?

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Of particular relevance, all the failures were in the wpt-privatebrowsing variant of the test suite (TIL that's a thing we run nowadays...).

Comment 11

[Jan Varga [:janv]]

That indicates that it has something to do with our ObfuscatingVFS.

Comment 12

[Andrew Sutherland [:asuth] (he/him)]

Ah, I wonder if the SQLITE_DIRECT_OVERFLOW_READ I mentioned in comment 2 changes the access pattern of requests through the VFS (which are not handled).

Comment 13

[Andrew Sutherland [:asuth] (he/him)]

Redirecting to Harveer to take a look; as per comment 12, I might expect a read request comes in looking different than we might expect.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Confirmed that tests pass with SQLITE_DIRECT_OVERFLOW_READ set to 0 in moz.build. I don't personally have a strong preference as to whether we land this update with that set for now with a follow-up bug for removing it later on or just wait a bit for more investigation first.

Comment 15

[Marco Bonardo [:mak]]

I'll delegate the decision to the DOM Storage team, off-hand I think it would be acceptable, and probably easier to track, to land with SQLITE_DIRECT_OVERFLOW_READ disabled and file a separate bug to enable it.

Comment 16

[Jan Varga [:janv]]

Yeah, I don't see a reason to block the upgrade. SQLITE_DIRECT_OVERFLOW_READ just enables a new optimization which we don't need right now.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2d3365b1475e
Upgrade to SQLite 3.45.1. r=mak

Comment 18

[Marco Bonardo [:mak]]

Thanks Ryan for taking care of this.

Comment 19

[Harveer Singh]

Looking at the ObfuscatingVFS, it seems that we might be getting requests from sqlite to read short pages (and we don't try to decrypt the short page, https://searchfox.org/mozilla-central/rev/14dc8f0e748d44778a02ffcf9ebcda3851b2bf9e/storage/ObfuscatingVFS.cpp#319). Maybe that's what making sqlite believe that database file is corrupted. I will investigate this further and report back on the related bug 1878311

Comment 20

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/2d3365b1475e