fix: comprehension SQL broken on SQLite, DuckDB, and MySQL - #27
Merged
Conversation
Ports two fixes from the Go cel2sql (SPANDigital/cel2sql#169, #177), where they were found by executing generated comprehension SQL against real databases: - SQLite json_each and MySQL JSON_TABLE are table-valued, so a bare reference to the iteration variable was 'no such column'; DuckDB's FROM UNNEST(arr) AS a binds a to a STRUCT row, making a = 'x' a cast error. A new Dialect.write_comprehension_source hook lets each dialect bind the value to the variable (derived-table rename for json_each/JSON_TABLE, AS _t(var) for DuckDB). - MySQL 8.x transforms a correlated EXISTS into a semijoin and loses the correlation to JSON_TABLE, so exists()/all() executed without error and silently matched nothing (works from 9.x; verified against MySQL 8.4.11 and 9.7.1). New write_comprehension_exists / write_comprehension_not_exists hooks let MySQL emit COUNT comparisons, which are never transformed. Adds integration tests that execute exists/all/exists_one against PostgreSQL, DuckDB, SQLite, and 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 the two comprehension fixes from the Go cel2sql (SPANDigital/cel2sql#169 and SPANDigital/cel2sql#177) — and it turns out pycel2sql was worse off than Go was: three dialects generated comprehension SQL that parses but cannot run correctly.
The bugs
EXISTS (SELECT 1 FROM json_each(arr) AS a WHERE a = ?)→ "no such column: a" (json_each is table-valued: rows of key, value, type, …)FROM UNNEST(arr) AS a WHERE a = 'x'→ cast error:abinds aSTRUCT(unnest VARCHAR)row, not the valueEXISTSinto a semijoin, loses the JSON_TABLE correlation, and silently matches nothing (works from 9.x; verified against 8.4.11 and 9.7.1 locally)The fix
Two new dialect hooks, mirroring the Go/Java/C# family design:
write_comprehension_source— binds the element value to the iteration variable: derived-table rename(SELECT value AS x FROM json_each(...)) AS _tfor SQLite/MySQL (same idiom cel2sql4j already uses for SQLite),UNNEST(...) AS _t(x)for DuckDBwrite_comprehension_exists/write_comprehension_not_exists— defaultEXISTS/NOT EXISTS; MySQL emits(SELECT COUNT(*) FROM ...) > 0/= 0, which the 8.x optimizer never transformsPostgreSQL/BigQuery/Spark output is unchanged.
Testing
tests/integration/test_comprehensions.pyexecutesexists/all/exists_oneagainst PostgreSQL, DuckDB, SQLite, and MySQL (the missing coverage that let this ship)src/ tests/), and strict mypy all clean