Check the SQLite analyze cases against sqlite3 in goldeneye, and make sqlc agree with it - #4610
Merged
Merged
Conversation
The analyze cases under internal/endtoend/testdata were only checked against a real database for ClickHouse. Now the sqlite ones are checked too, against one more sqlite3 shell that `install sqlite` builds, with SQLITE_ENABLE_COLUMN_METADATA and every extension option: `.stats stmt` says which table column each result column is read from, the query's own rows over the fixture and over no rows type the expressions and find the NULLs, and the parameters are followed through the bytecode EXPLAIN prints to the column each is compared with, stored into or seeks by. The query-file parsing, placeholder rewriting and the JSON shape of an engine's answer move from the clickhouse package into endtoend, where both engines share them. An analyze_expressions case with a fixture covers aggregates, IN lists, LIMIT, an outer join and RETURNING for SQLite; the five sqlite cases match what SQLite reports. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PdKaVjSmdkmXPeUkxetyDY
Three things the goldeneye check found sqlc's analysis saying that SQLite does not. sum, abs, ceil, floor and trunc hand back an integer for an integer and a real for a real, so the dialect now spells their return type "$1", the argument's type, rather than widening to real; unixepoch, which is a real only with 'subsec', keeps real. The legacy compiler learns to resolve "$n" to the named column's or literal's type, so sum(int_val) generates NullInt64 rather than NullFloat64. IS and IS NOT test for NULL rather than propagate it, so their result is never NULL, as are SQLite's postfix ISNULL and NOTNULL, which used to be typed as their operand. And LIKE, GLOB, REGEXP and MATCH are predicates, listed with the dialect's comparison operators under the names the parser gives them, in place of the ~~ spellings it never uses. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PdKaVjSmdkmXPeUkxetyDY
The legacy compiler takes a dialect function's return type as it is spelled, so it is not taught to follow "$1" to an argument; sum, abs, ceil, floor and trunc generate an untyped value on that path, the way max and min already do, and the analysis core resolves them. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PdKaVjSmdkmXPeUkxetyDY
A function that returns an integer for an integer and a real for a real is now written the way PostgreSQL's catalog writes sum: once over any, returning the real a text or blob argument gets, and once more per spelling types.jsonl gives integer and real, returning that type. The analysis core picks the overload whose parameter is the argument's type, so sum over an INTEGER or BIGINT column is an integer; the legacy compiler resolves by arity alone and takes the first overload, the one over any, so what it generates is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PdKaVjSmdkmXPeUkxetyDY
The JSON sqlc analyze prints — a query, its columns, its parameters and their types — lived in endtoend beside the case finder. It is its own thing, so it moves to the analysis package, and the Analyzed prefix goes with it: analysis.Query, analysis.Column, analysis.Param. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PdKaVjSmdkmXPeUkxetyDY
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
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.
The analyze cases under
internal/endtoend/testdatawere only checked against a real database for ClickHouse. This extends goldeneye's analysis check to SQLite, and fixes what the check found sqlc's analysis saying that SQLite does not.The check
install sqlitebuilds one more shell, underanalysis/, withSQLITE_ENABLE_COLUMN_METADATAand every extension option at once. The check asks it three things about each query and prints the answer in the shapesqlc analyzeuses:.stats stmt, which reports the table column each result column is read from; that column's declared type is the result column's, and its NOT NULL decides nullability, the rowid counting as NOT NULL.fixture.sqlwith each parameter bound to a value of its own column, and again over no rows; a NULL from either run makes the column nullable, which catches aggregates over nothing and the far side of an outer join.EXPLAINprints, the SQLite counterpart of ClickHouse's query tree. Each is followed from the register itsVariableloads, through copies and expressions, to the first opcode that uses it against something nameable: a comparison operand, a rowid or index seek, anInsertrecord position, or the ephemeral table of anINlist.MustBeInttypes a LIMIT parameter as an integer.The query-file parsing, placeholder rewriting and the JSON output types move from the clickhouse package into
endtoend, since both engines share them. A newanalyze_expressions/sqlitecase with a fixture covers aggregates, an IN list, LIMIT/OFFSET, a LEFT JOIN, a concatenated LIKE parameter and INSERT RETURNING. All five SQLite cases and the six ClickHouse ones match their database.What the check found
sum,abs,ceil,floorandtruncreturn an integer for an integer and a real for a real; the dialect widened them toreal. They are now written the way PostgreSQL's catalog writessum: once overany, returning the real a text or blob argument gets, and once per spellingtypes.jsonlgives integer and real, returning that type. The analysis core picks the overload whose parameter is the argument's type; the legacy compiler resolves by arity alone and takes the first, so what it generates is unchanged (the goldens are byte-for-byte what main produces).ISandIS NOTtest for NULL rather than propagate it, so their result is never NULL; the core was markingx IS NULLnullable. SQLite's postfixISNULLandNOTNULL, which arrive as operators with no right operand, are predicates too.LIKE,GLOB,REGEXPandMATCHwere typed astextbecause the dialect listed them under PostgreSQL's~~spelling that the parser never uses. They are now in the dialect's comparison list under their real names.One disagreement is left as is and documented in the README: SQLite stores a predicate such as
x IS NULLas an integer, while sqlc types itboolean, which is what its generated code has always used.🤖 Generated with Claude Code
https://claude.ai/code/session_01PdKaVjSmdkmXPeUkxetyDY
Generated by Claude Code