Possible bug with INNER JOIN in 3.38.2

(1) By Daniel Lockyer (daniellockyer) on 2022-04-25 10:23:14 [source]

Hi :)

I'm one of the maintainers of node-sqlite3 and we recently had a bug report in our library: https://github.com/TryGhost/node-sqlite3/issues/1589

node-sqlite3 recently jumped from SQLite v3.34.0 to v3.38.2. The user is reporting that their query doesn't return any results on the version we ship (3.38.2).

They've provided a DB and query, and I can reproduce it. I believe this happens to be a bug in SQLite. On v3.34.0, I can run the query against the DB and I get the expected results. On v3.38.2, I run the query, get no results, run it again and I get the expected results.

SELECT c.id AS "c_ID", b.id AS "b_ID" FROM localized_defineservice_c_closingfolderorgunittp c INNER JOIN localized_defineservice_c_closinghierarchynodetp b ON (b.id = c.to_closinghierarchynode_id);

I think I've bisected this back to https://github.com/sqlite/sqlite/commit/c1085ea412b5c78d58cad59273d71f44d39843c5

Apologies if this was already reported/fixed. I tried searching on the forum and I've tried compiling from the latest code - same result.

Thanks, Daniel

(2) By Dan Kennedy (dan) on 2022-04-25 10:44:08 in reply to 1 [link] [source]

Can you run the ".fullschema" command in the shell tool on the database showing the bug and post the results?

Thanks,

Dan.

(3) By Ryan Smith (cuz) on 2022-04-25 10:57:30 in reply to 2 [link] [source]

I believe the entire DB is available here if you wish to test it directly.

(4) By Daniel Lockyer (daniellockyer) on 2022-04-25 11:45:10 in reply to 2 [link] [source]

Hey Dan,

Sure thing - please see the following link: https://gist.githubusercontent.com/daniellockyer/fd2845ccbaae25dc66c47a66aa5a42eb/raw/4c3e192789dc83b7d65df3d0c05f8634697367b5/fullschema

I've also provided a plaintext dump here: https://gist.githubusercontent.com/daniellockyer/fd2845ccbaae25dc66c47a66aa5a42eb/raw/4c3e192789dc83b7d65df3d0c05f8634697367b5/dump

It's worth noting that it looks like localized_defineservice_c_closingfolderorgunittp and localized_defineservice_c_closinghierarchynodetp are views. Their capitalized names are localized_DefineService_C_ClosingFolderOrgUnitTP and localized_DefineService_C_ClosingHierarchyNodeTP.

As Ryan mentions, the DB is available on that link :)

Daniel

(5) By Richard Hipp (drh) on 2022-04-27 09:43:52 in reply to 1 [link] [source]

Thank you for the bug report.

This problem is fixed by check-in 134cfb18ff930e4b. This fix has also been backported to version 3.38 and version 3.38.3 will be released shortly.

Analysis

This is a simplified test case:

  CREATE TABLE t1(a INT, b INT);
  CREATE TABLE t2(c INT, d INT);
  CREATE TABLE t3(e TEXT, f TEXT);
  INSERT INTO t1 VALUES(1, 1);
  INSERT INTO t2 VALUES(1, 2);

  SELECT * FROM t1 JOIN t2 ON (t2.c=t1.a) LEFT JOIN t3 ON (t2.d=1)

SQLite computes the join of t1 and t2 using an "automatic index". An automatic index is a transient index that is computed for a single query and only lives for the duration of that one query. Other RDBMSes would typically call this a "hash join". For the 3.38.0 release, the query planner got a little too aggressive about trying to minimize the size of this automatic index, and ended up using the "t2.d=1" constraint from the LEFT JOIN to try to reduce the size of the transient index. The use of the LEFT JOIN constraint is incorrect and resulted in the automatic index that omitted rows, which in turn resulted in rows being omitted from the final output.

(6) By Daniel Lockyer (daniellockyer) on 2022-04-27 12:28:34 in reply to 5 [link] [source]

Hey Richard,

Perfect - thank you! It seems good to me, so I look forward to v3.38.3.

Kind regards,

Daniel

(7) By Michael A. Cleverly (cleverly) on 2022-04-27 18:31:00 in reply to 5 [link] [source]

There is a typo (undoubtedly due to muscle memory) at line 2291 of src/expr.c:

/*
** Check pExpr to see if it is an invariant constraint on data source pSrc.
** This is an optimization. False negatives will perhaps cause slower
** queries, but false positives will yield incorrect answers.  So when in
** double, return 0.

s/double/doubt/