1698592 - (SQLite3.35.4) Upgrade to SQLite 3.35.4

Status: RESOLVED FIXED

(Core :: SQLite and Embedded Database Bindings, task, P3)

Description

[Ryan VanderMeulen [:RyanVM]]

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

SQLite version 3.35.0 is a routine maintenance release. This release adds a number of new language features, including support for ALTER TABLE DROP COLUMN, built-in math functions, generalized UPSERT, and the MATERIALIZED hint on common table expressions. There are also query planner optimizations and incremental CLI improvements.

Detailed changelog:
https://www.sqlite.org/releaselog/3_35_1.html

Comment 1

[Marco Bonardo [:mak]]

let's pick this at the beginning of the 89 cycle.

Comment 2

[Arthur K. (he/him)]

And now we have 3.35.2: https://www.sqlite.org/releaselog/3_35_2.html

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Attachment: Bug 1698592 - Upgrade to SQLite 3.35.4. r=mak

Comment 4

[Ryan VanderMeulen [:RyanVM]]

This has test failures on Try:
https://treeherder.mozilla.org/logviewer?job_id=334506204&repo=try&lineNumber=4610

INFO - TEST-UNEXPECTED-FAIL | browser/components/places/tests/browser/browser_forgetthissite_single.js | The Forget this site menu item should  be hidden with 2 items selected - Got false, expected true

INFO - Console message: [JavaScript Error: "NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsINavHistoryContainerResultNode.getChild]" {file: "chrome://browser/content/places/treeView.js" line: 292}]
INFO - PTV__getNodeForRow@chrome://browser/content/places/treeView.js:292:26
INFO - nodeForTreeIndex@chrome://browser/content/places/treeView.js:1229:17
INFO - get selectedNodes@chrome://browser/content/places/places-tree.js:243:33
INFO - PC_isCommandEnabled@chrome://browser/content/places/controller.js:142:26
INFO - updateCommands@resource:///modules/PlacesUIUtils.jsm:490:34
INFO - oncommandupdate@chrome://browser/content/places/places.xhtml:1:15

Comment 5

[Marco Bonardo [:mak]]

I must debug that, the log only tells me we're trying to fetch a child that doesn't exist, but why? either a timing problem or some query is returning different results...

Comment 6

[Marco Bonardo [:mak]]

3.35.3 is out, but I actually think there's a bug in this version, that our ugly query undisclosed. I made a reduced test case that I sent to the SQLite team, so it's possible we'll see a 3.35.4. I'll update this at a later time when it is confirmed or dismissed as an upstream bug.

Comment 7

[Marco Bonardo [:mak]]

To annotate my discovery, this is the failing query:

SELECT h.id, h.url, h.title AS page_title, h.rev_host, h.visit_count, h.last_visit_date, null, null, null, null, null, null AS tags , h.frecency, h.hidden, h.guid, null, null, null
FROM moz_places h
WHERE 1
AND hidden = 0
AND last_visit_date NOTNULL
AND ( EXISTS (SELECT 1 FROM moz_historyvisits WHERE place_id = h.id  AND  visit_date >=  1616968800000000  AND  visit_date <=  1617055200000000 LIMIT 1) )
ORDER BY 6 DESC;

The LIMIT 1 in the subquery is somehow applied outside of it, possibly an optimization not working as expected.
This query is a bit ugly because it's built automatically by the history code, that would require some adjustments and rewrite.

Comment 8

[Marco Bonardo [:mak]]

The bug is actually caused by this optimization in the release notes:
"Attempt to process EXISTS operators in the WHERE clause as if they were IN operators, in cases where this is a valid transformation and seems likely to improve performance. "

I see that a fix has been pushed to the Sqlite src, thus it's just matter of waiting for the next release, whatever it is.

Comment 9

[Marco Bonardo [:mak]]

3.35.4 is being prepared (in draft), the upstream bug is https://sqlite.org/src/info/de7db14784a08053

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Try is happier now:
https://treeherder.mozilla.org/jobs?repo=try&group_state=expanded&revision=94c1ff8709e5d4d6aef61f9fd0f337c67df62b67

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8f1f35c4a8e7
Upgrade to SQLite 3.35.4. r=mak

Comment 12

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/8f1f35c4a8e7

Comment 13

[Arthur K. (he/him)]

FYI

Additional changes in version 3.35.5 (2021-04-19):

Fix defects in the new ALTER TABLE DROP COLUMN feature that could corrupt the database file.
Fix an obscure query optimizer problem that might cause an incorrect query result.

Comment 14

[Marco Bonardo [:mak]]

(In reply to Arthur K. [He/Him] from comment #13)

Fix defects in the new ALTER TABLE DROP COLUMN feature that could corrupt the database file.

This doesn't affect us yet, because nobody is using it.

Fix an obscure query optimizer problem that might cause an incorrect query result.

This probably doesn't affect us, I did a quick code search and cannot find cases where we do "(col IS/NOT NULL AND <expr>) OR col == NULL".
I'd be ok taking the dot release in 90 as maintenance, but we don't need an uplift, imo.

Comment 15

[Arthur K. (he/him)]

3.36 got released today. Worthy of going into 91?

https://www.sqlite.org/releaselog/3_36_0.html

Comment 16

[Marco Bonardo [:mak]]

It looks like maintenance fixes for the most part, we can take it.