fix: let SQLite name the missing column in null_absent_columns - #399
Merged
Conversation
The first version guessed which bare words in a statement were column references, with a rule carried over from Core Data work that only considered names beginning with Z. That is useless for other column names, and it failed silently: it compiled, passed CI, and did nothing on the artifacts it was added for. It now compiles the query with EXPLAIN and replaces whatever SQLite objects to, repeatedly, until it compiles. No guessing, and qualified and bare references behave the same. Proved out in ALEAPP PR #1077. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Replaces
null_absent_columns()with the version proved out in ALEAPP (#1077).Why the first version needed replacing
It guessed which bare words in a SQL statement were column references, using a rule carried over from Core Data work that only considered names beginning with
Z. That is correct for iOS stores and useless for names like_source,ap_temporlastUpdated.The consequence was the worst kind: it compiled, passed lint, passed CI, and did nothing at all on the artifacts it had been added for. Only a corpus sweep caught it — the same
no such columnlines were still there afterwards.What it does now
It compiles the query with
EXPLAINand replaces whatever SQLite objects to, repeatedly, until the statement compiles. SQLite owns the syntax, so it names the missing column itself and there is nothing left to guess. Qualified and bare references are handled identically, andEXPLAINcompiles without running, so it costs nothing on a large table.It still emits
NULL AS <name>where the reference is a select item in its own right, because artifacts read rows by name and a bareNULLrenames the output column.Scope and verification
No call sites in this repo, so no artifact behaviour changes; this keeps the helper correct for when it is used.
Verified the ported function behaves identically across cores against a real
ThreeBars.sqlitemissingZLOWQUALITY: 97 rows, column name preserved, valueNone— the same result in ALEAPP, DLEAPP, RLEAPP and VLEAPP.In ALEAPP, where it does have call sites, the corrected version took a 12-corpus sweep from 45 error lines to 8, recovered 23 artifact/corpus pairs (including 4,474 temperature readings and 839 netstat rows the previous version left on the floor), with 2,630 pairs identical and zero regressions.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com