fix: MySQL comprehensions generated SQL that silently matched nothing - #44
Merged
Conversation
Ports SPANDigital/cel2sql#177 (and the MySQL half of #169). MySQL used the default comprehension source, JSON_TABLE(...) AS x, where a bare reference to x is an unknown column — and the integration harness carried an assumption skipping every MySQL comprehension case ('comprehensions not supported') instead of a fix. On top of that, the MySQL 8.x optimizer transforms a correlated EXISTS into a semijoin and loses the correlation to JSON_TABLE, so exists()/all() would execute without error and match nothing (works from 9.x; verified against MySQL 8.4.11 and 9.7.1). - MySqlDialect.writeComprehensionSource renames the value column to the iteration variable through a derived table, the same idiom SqliteDialect already uses for json_each - New Dialect.writeComprehensionExists/writeComprehensionNotExists default to EXISTS/NOT EXISTS; MySQL emits COUNT comparisons, which the 8.x optimizer never transforms - The MySQL comprehension skip is removed from the integration tests, so comp_all/comp_exists/comp_exists_one now execute against the MySQL container in CI - Unit expectations added for all five macros on MySQL
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports SPANDigital/cel2sql#177 and the MySQL half of SPANDigital/cel2sql#169 to the Java port.
The bugs
MySQL used the default comprehension source —
JSON_TABLE(...) AS x— where a bare reference toxis an unknown column (JSON_TABLE is table-valued). The integration harness knew: it carriedAssumptions.assumeTrue(dialect.name() != MYSQL, "MySQL: comprehensions not supported")instead of a fix, so every MySQL comprehension case was skipped.Fixing the column reference alone isn't enough: the MySQL 8.x optimizer transforms a correlated
EXISTSinto a semijoin and loses the correlation to the JSON_TABLE source, soexists()/all()execute without error and silently match nothing. Works from MySQL 9.x; verified against 8.4.11 and 9.7.1 locally (semijoin=offfixes it,LIMIT 1does not, COUNT comparisons are never transformed).The fix
MySqlDialect.writeComprehensionSource→(SELECT value AS x FROM JSON_TABLE(...) AS jt) AS _t, the same derived-table idiomSqliteDialectalready uses forjson_eachDialect.writeComprehensionExists/writeComprehensionNotExistsdefaults (EXISTS/NOT EXISTS) with MySQL overriding to(SELECT COUNT(*) FROM ...) > 0/= 0comp_all/comp_exists/comp_exists_onenow execute against the MySQL container in CI, which is the real verification for this PRall/exists/exists_one/filter/map) on MySQLOther dialects' output is unchanged.
Same fix landed in Go (cel2sql v3.8.10) and Python (pycel2sql v0.4.2); a C# port PR follows.