Stop casting variables to TEXT on PostgreSQL, MySQL and SQL Server - #1397
Open
lovasoa wants to merge 12 commits into
Open
Stop casting variables to TEXT on PostgreSQL, MySQL and SQL Server#1397lovasoa wants to merge 12 commits into
lovasoa wants to merge 12 commits into
Conversation
… and SQL Server SQLPage binds every variable as a string. The generated CAST(? AS TEXT) forced the database to type the parameter as text, which is only needed where parameter type inference is unpredictable (SQLite, ODBC). On the natively supported databases the cast was redundant, and on SQL Server it was harmful: the parameter is bound as NVARCHAR(MAX), and casting it to a narrow VARCHAR mangled non-ASCII values before comparing them to nvarchar columns. Generated SQL is now cleaner, e.g. WHERE id = $1 instead of WHERE id = CAST($1 AS TEXT). SQLite and ODBC-backed databases (Oracle, DuckDB, Snowflake, Generic) keep the cast to preserve their comparison semantics. Verified with the full test suite against SQLite, PostgreSQL, MySQL and SQL Server, including new fixtures for integer-column comparisons, numeric-literal comparisons, and unicode nvarchar comparisons on SQL Server.
The ODBC job in CI failed because the cast was removed based on the database name behind the driver: PostgreSQL reached through psqlodbc no longer received CAST(? AS TEXT), and the driver could not determine the type of context-free parameters such as in 'WHERE ? <> ? OR ? IS NULL', failing with 'could not determine data type of parameter'. The cast decision now keys off the connection kind: native PostgreSQL, MySQL and SQL Server connections keep no cast, while every ODBC connection keeps the previous per-database cast, since ODBC drivers provide no parameter type information. Adds a fixture comparing variables without any surrounding type context, which exercises exactly this scenario on every database.
…C too ODBC connections were conservatively keeping the cast for every database. Testing against real ODBC drivers shows the cast is needed only where the parameter type cannot be determined without it: - psqlodbc -> PostgreSQL: needed. Without it, context-free parameters fail with 'could not determine data type of parameter' (the earlier CI failure). - sqliteodbc -> SQLite: needed. Without it, '? = 1' compares text against an integer and silently returns false, like on native SQLite. - duckdb-odbc: not needed. The full test suite passes without the cast, as DuckDB defaults untyped parameters to VARCHAR. The cast is therefore dropped for MySQL, SQL Server and DuckDB behind ODBC, mirroring their native behavior (MySQL and SQL Server convert the bound string at execution time, which also fixes the unicode mangling for SQL Server reached through ODBC), and kept for PostgreSQL, SQLite, Oracle, Snowflake and unknown databases. Verified with the full test suite on native SQLite, PostgreSQL, MySQL and SQL Server, and through ODBC on PostgreSQL, SQLite and DuckDB. The only ODBC failure is a pre-existing database-filesystem timestamp test that also fails on main.
The previous entry described internal CAST(? AS TEXT) generation and load-bearing type affinity details. Rephrase for users: focus on the visible fix (MSSQL nvarchar Unicode mangling) and the general simplification (no unnecessary text cast where the database infers the type).
lovasoa
force-pushed
the
simplify-variable-casts
branch
from
August 22, 2026 20:55
0da757f to
11ab192
Compare
…#1154 - Replace verbose pattern-matching in sql.rs with helpers sql_for/ odbc_sql_for and table-driven asserts; keep coverage but drop ceremony and duplicated error messages. - Keep limit and mssql tests as one-liners checking the generated SQL string. - Trim .sql fixtures to minimal scaffold and add GH issue links as comments. New fixtures: * variable_limit_offset (MySQL, fixes #1154) — LIMIT/OFFSET with SET variables must not be wrapped in CAST. * variable_mssql_contains (MSSQL, fixes #516) — EXEC sp_executesql with a variable must not be wrapped in CAST. - Simplify existing variable fixtures and add issue links, keep them short and readable.
Remove 'same root cause' and inaccurate GH links that referenced other files. Each fixture now describes its own invariant without assuming reader context from another file.
- Fix variable_mssql_contains test: avoid nested single quotes in sp_executesql string that caused 'Incorrect syntax near It' on CI. Use parameterised expected value instead of embedding 'It works !' inside the inner N'...' string. - Improve CHANGELOG: one main bullet about removing CAST with subpoints for PostgreSQL/MySQL/DuckDB, SQL Server nvarchar/ CONTAINS/EXEC, MySQL LIMIT/OFFSET, and retained cast on SQLite/ ODBC. Move fixes:/see: to PR description.
Restructure tests so files that only work on a single database engine are organized under `database-specific/<engine>/` instead of using long `_no...` suffixes. Add a dedicated test that runs these files only when the current database matches, and simplify the generic test runner by extracting shared execution logic.
The file reproduces issue #516 (CONTAINS rejects CAST expressions), but the query itself uses sp_executesql, which has the same restriction without needing a full-text index. Rename the fixture to reflect that and add a comment explaining why CONTAINS is not used directly.
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.
What
SQLPage binds every variable as a string and currently wraps each generated parameter in a database-specific text cast:
CAST($1 AS TEXT)CAST(? AS CHAR)CAST(@p1 AS VARCHAR(MAX))CAST(? AS VARCHAR(4000))/CAST(? AS VARCHAR)This PR removes the cast on PostgreSQL, MySQL and SQL Server, where it is redundant or harmful:
TEXTwhen preparing the statement, so the cast is a no-op. Generated SQL goes fromWHERE id = CAST($1 AS TEXT)::inttoWHERE id = $1::int.NVARCHAR(MAX), and the generatedCAST(@p1 AS VARCHAR(MAX))round-trips the value through the database's narrow code page, silently mangling non-Latin characters before comparing withnvarcharcolumns. E.g.SELECT * FROM t WHERE name = $namefailed to matchname = '日本語'even when the value was correct. Now the comparison isnvarchar = nvarcharand works (verified live against SQL Server: 0 rows before, 1 row after).The cast is kept on SQLite and ODBC-backed databases (Oracle, DuckDB, Snowflake, Generic), where it is genuinely load-bearing: on SQLite, removing it silently flips comparison results (
'1' = 1is false without the cast, true with it — the v0.15 regression), and ODBC driver parameter typing is unpredictable.The condition is therefore a pure function of the database: no AST-context analysis, so the generated SQL stays fully predictable.
Testing
Full test suite (
cargo test) run and passing against SQLite, PostgreSQL, MySQL and SQL Server (containers).New tests:
text_cast_is_only_generated_when_parameter_typing_is_unpredictablelocks the per-database cast matrix.mssql_parameters_are_not_cast_to_narrow_varchar.variable_compared_to_integer_column_nopostgres.sql—WHERE id = $xon an integer column works on SQLite/MySQL/SQL Server/Oracle (on PostgreSQL it requires an explicit cast and always has, since sqlx pins parameters as TEXT).variable_compared_to_number_literal_nopostgres.sql— guards the SQLite cast ($x = 1must compare as text there).variable_unicode_noduckdb_..._nosqlite.sql— guards the SQL Server unicode fix with a realnvarcharcomparison.Also manually verified the
todo application (PostgreSQL)example end-to-end against PostgreSQL.cargo fmtandcargo clippy --all-targets --all-features -- -D warningsare clean. CHANGELOG and the data-model documentation were updated.