Uh oh!
There was an error while loading. Please reload this page.
fix(datagrid): quote filter values by the column's declared type - #2029
Merged
Conversation
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
datlechinforce-pushed
the
fix/filter-column-type-quoting
branch
from
August 6, 2026 16:30
58316fe to
e7c9a48CompareUh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
Filtering a
varcharcolumncodewith= equalsand the value68produced:It should produce
`code` = '68'. Typing68awas already correct.Root cause
FilterSQLGenerator.escapeValue()picked the SQL literal type from the shape of the typed string, never from the column:On MySQL
varchar_col = 68coerces the column to a number on every row, so it matches the wrong rows ('68abc',' 68','0068') and the index goes unused. On ClickHouse and Trino it is a hard error.The same function also promoted a typed
NULLto the SQL keyword andTRUE/FALSEto booleans with no type gate, which made the literal text "NULL" in a text column impossible to filter for.The rule
Quoting now follows the column's type category, with the value's shape used only as a secondary check inside an already-numeric column. This is the pattern
InClauseConverteralready used correctly, now extracted so both share one definition.68NULLTRUE'68''NULL''TRUE'68IS NULL'TRUE''68'IS NULL'68'IS NULL'TRUE'68IS NULLAlways quoting was rejected: Trino and BigQuery reject
int_col = '68', and ClickHouse silently drops the atom from index pruning on an out of range numeric string. The unknown case keeps today's heuristic for the same reason.Quoting enum columns matters on its own: MySQL and ClickHouse compare enums by ordinal, so
status = 1selected the first member rather than the member named "1".Scope
The type data was already in
TableRows.columnTypes, index aligned withcolumnsand in scope at every call site. It just was not forwarded.FilterSQLGeneratorgained defaultedcolumns:/columnTypes:parameters, so existing construction sites compile unchanged.MSSQL, Oracle, BigQuery and SurrealDB build their own filter queries and each reimplemented the same bug. They now go through a shared renderer in PluginKit.
TableFiltergains no field, so nothing new is persisted and saved filters keep working.PluginKit ABI
scripts/check-pluginkit-abi.sh origin/mainreports additions only, no removed symbols, so nocurrentPluginKitVersionbump. Needs theabi-additivelabel. The four registry plugins need a re-release to pick up their side of the fix; until then they keep the old behavior rather than failing to load.Also changed
IN/NOT INstripped the tokenNULLfrom every list regardless of column type, socode IN ('68a', 'NULL')on a text column lost the real string. Fixed by the same per element rendering.IS EMPTYemitted(col IS NULL OR col = '')for every type. The= ''half is a type mismatch error on PostgreSQL, ClickHouse, MSSQL, Oracle and Trino, so a non text column now emits onlycol IS NULL. This changes the rows returned by a saved IS EMPTY filter on a non text column, where the previous result was already wrong or erroring.Double(value) != nil, which accepts0x1F,nanandinfinity. It now uses PluginKit's existing strictPluginNumericLiteral.isValid.Not covered
Foreign key navigation builds its filter before the target table's rows load, so no column types exist yet and it falls back to the heuristic. The iOS copy of the generator is unchanged; it has no column type pipeline to thread.
Tests
FilterSQLGeneratorColumnTypeTestscovers the reported case first, plus a guard that a genuinely numeric column keeps an unquoted literal, the enum ordinal case, NULL/TRUE gating, the unknown type fallback, duplicate and missing column names, and the IN/BETWEEN element paths.ColumnTypeSQLQuotingTestspins the literal shapes. One existing test title claimed a general rule that no longer holds and was renamed; its assertion is unchanged because it passes no column type.https://claude.ai/code/session_01PaouzGXduVq1dr5SBCgVH8