From 7e9adb14e5d232480bdba45678ec033f20dbec56 Mon Sep 17 00:00:00 2001 From: Ophir Lojkine Date: Sat, 22 Aug 2026 15:44:13 +0000 Subject: [PATCH 01/12] feat(database) :: stop casting variables to TEXT on PostgreSQL, MySQL 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. --- CHANGELOG.md | 1 + examples/official-site/extensions-to-sql.md | 4 + src/webserver/database/sql.rs | 86 +++++++++++++++++-- src/webserver/database/sql/rewrite.rs | 20 +++-- ..._compared_to_integer_column_nopostgres.sql | 7 ++ ..._compared_to_number_literal_nopostgres.sql | 5 ++ ...oracle_nopostgres_nosnowflake_nosqlite.sql | 9 ++ 7 files changed, 119 insertions(+), 13 deletions(-) create mode 100644 tests/sql_test_files/data/variable_compared_to_integer_column_nopostgres.sql create mode 100644 tests/sql_test_files/data/variable_compared_to_number_literal_nopostgres.sql create mode 100644 tests/sql_test_files/data/variable_unicode_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index 5144ae1e..fe4af459 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## unreleased + - SQLPage no longer wraps request variables in a generated `CAST(? AS TEXT)` when talking to PostgreSQL, MySQL, or Microsoft SQL Server. Those databases resolve the parameter type from the surrounding expression, so the cast was redundant there — and on SQL Server it converted the bound Unicode value to a narrow `varchar`, silently mangling non-Latin characters in comparisons with `nvarchar` columns, which now work correctly. SQLite and ODBC-backed databases keep the explicit cast. Generated SQL is now easier to read, e.g. `WHERE id = $1` instead of `WHERE id = CAST($1 AS TEXT)`. - AWS Lambda builds and documentation now use the supported Amazon Linux 2023 custom runtime instead of the end-of-life Amazon Linux 2 runtime. Release artifacts include the configuration directory required on Lambda's read-only filesystem. - Added a `toast` component with plain-text or Markdown content, icons, colors, six screen placements, configurable auto-dismiss timing, optional manual dismissal, URL-fragment triggers, and automatic stacking of queued notifications. - `sqlpage.send_mail` now supports rich email bodies. Use `body_html` for a caller-provided HTML alternative, or `body_md` to render Markdown as HTML. Messages retain a plain-text alternative; `body` may be omitted when `body_md` is used, and `body_md` and `body_html` cannot be combined. diff --git a/examples/official-site/extensions-to-sql.md b/examples/official-site/extensions-to-sql.md index 36eb57af..43eceaa9 100644 --- a/examples/official-site/extensions-to-sql.md +++ b/examples/official-site/extensions-to-sql.md @@ -143,6 +143,10 @@ This means `SET` variables always take precedence over request parameters when u Only a single textual value (**string or `NULL`**) is stored. `SET id = 1` will store the string `'1'`, not the number `1`. +Variables are always sent to the database as text. +On SQLite and databases connected through ODBC, SQLPage wraps variables in an explicit cast to text, because their parameter type inference would otherwise make comparisons unpredictable. +On PostgreSQL, the variable is passed as text, and comparing it to a non-text column requires an explicit cast. +On MySQL and Microsoft SQL Server, the database converts the variable to the type expected by the surrounding expression. On databases with a strict type system, such as PostgreSQL, if you need a number, you will need to cast your variables: `SELECT * FROM post WHERE id = $id::int`. Complex structures can be stored as json strings. diff --git a/src/webserver/database/sql.rs b/src/webserver/database/sql.rs index 3ceeb050..56b74378 100644 --- a/src/webserver/database/sql.rs +++ b/src/webserver/database/sql.rs @@ -432,7 +432,7 @@ mod tests { }; assert_eq!(query.bindings.len(), 1); assert!(query.computed_columns.is_empty()); - assert!(query.sql.contains("upper(CAST($1 AS TEXT))")); + assert!(query.sql.contains("upper($1)")); } #[test] @@ -519,10 +519,7 @@ mod tests { else { panic!("expected database query"); }; - assert_eq!( - query.sql, - "WITH c AS (SELECT CAST(? AS CHAR) AS x) SELECT CAST(? AS CHAR) AS y FROM c" - ); + assert_eq!(query.sql, "WITH c AS (SELECT ? AS x) SELECT ? AS y FROM c"); assert_eq!(query.bindings.as_ref(), [variable("a"), variable("b")]); } @@ -742,7 +739,7 @@ mod tests { "select coalesce(upper(sqlpage.url_encode($prefix)), sqlpage.url_encode(value)) as result from t" ), DatabaseQuery { - sql: "SELECT value AS \"__sqlpage_input_0\", upper(CAST($1 AS TEXT)) AS \"__sqlpage_input_1\" FROM t".into(), + sql: "SELECT value AS \"__sqlpage_input_0\", upper($1) AS \"__sqlpage_input_1\" FROM t".into(), bindings: Box::new([call(SqlPageFunctionName::url_encode, [variable("prefix")])]), row_input_json: Box::new([false, false]), computed_columns: Box::new([OutputColumn { @@ -764,8 +761,7 @@ mod tests { "select sqlpage.url_encode(value) as encoded from t where sqlpage.url_encode($expected) = 'x'" ), DatabaseQuery { - sql: "SELECT value AS \"__sqlpage_input_0\" FROM t WHERE CAST($1 AS TEXT) = 'x'" - .into(), + sql: "SELECT value AS \"__sqlpage_input_0\" FROM t WHERE $1 = 'x'".into(), bindings: Box::new([call( SqlPageFunctionName::url_encode, [variable("expected")] @@ -804,4 +800,78 @@ mod tests { } ); } + + #[test] + fn text_cast_is_only_generated_when_parameter_typing_is_unpredictable() { + for database_type in [ + SupportedDatabase::Sqlite, + SupportedDatabase::Oracle, + SupportedDatabase::Duckdb, + SupportedDatabase::Snowflake, + SupportedDatabase::Generic, + ] { + let FileStatement::Query(Query { + body: QueryBody::Database(query), + .. + }) = one_for(database_type, "select $a as value from t") + else { + panic!("expected database query"); + }; + assert!( + query.sql.to_lowercase().contains("cast"), + "{database_type:?} should keep the text cast: {}", + query.sql + ); + } + for database_type in [ + SupportedDatabase::Postgres, + SupportedDatabase::MySql, + SupportedDatabase::Mssql, + ] { + let FileStatement::Query(Query { + body: QueryBody::Database(query), + .. + }) = one_for(database_type, "select $a as value from t") + else { + panic!("expected database query"); + }; + assert!( + !query.sql.to_lowercase().contains("cast"), + "{database_type:?} should not generate a cast: {}", + query.sql + ); + } + } + + #[test] + fn postgres_limit_parameter_is_not_wrapped_in_a_cast() { + // The database still requires an integer for LIMIT, so this query + // fails at execution time with a string parameter; this test only + // locks in that SQLPage does not generate a text cast around it. + assert_eq!( + rewrite_database("select value from t limit $n"), + DatabaseQuery { + sql: "SELECT value FROM t LIMIT $1".into(), + bindings: Box::new([variable("n")]), + row_input_json: Box::new([]), + computed_columns: Box::new([]), + json_columns: Box::new([]), + } + ); + } + + #[test] + fn mssql_parameters_are_not_cast_to_narrow_varchar() { + let FileStatement::Query(Query { + body: QueryBody::Database(query), + .. + }) = one_for( + SupportedDatabase::Mssql, + "select name from t where name = $x", + ) + else { + panic!("expected database query"); + }; + assert_eq!(query.sql, "SELECT name FROM t WHERE name = @p1"); + } } diff --git a/src/webserver/database/sql/rewrite.rs b/src/webserver/database/sql/rewrite.rs index 5b090f84..f6baf323 100644 --- a/src/webserver/database/sql/rewrite.rs +++ b/src/webserver/database/sql/rewrite.rs @@ -1035,17 +1035,27 @@ fn variable_source(prefix: char) -> VariableSource { } } -/// Wraps a generated placeholder in the backend-specific text cast expected -/// by `SQLPage`'s string-valued binding interface. +/// Wraps a generated placeholder in the backend-specific text cast when the +/// database cannot reliably infer that the parameter is a string. +/// +/// `SQLPage` always binds parameters as strings. `PostgreSQL` (which pins the +/// parameter type to `TEXT` when preparing the statement), `MySQL` and `SQL +/// Server` (which convert the bound string to the type expected by the +/// surrounding expression) do not need the cast, and it can even be harmful: +/// on `SQL Server` the parameter is bound as `NVARCHAR(MAX)`, and casting it +/// to a narrow `VARCHAR` mangles non-ASCII values. `SQLite` and ODBC-backed +/// databases, whose parameter type inference is unpredictable, keep the +/// explicit cast. fn cast_placeholder(placeholder: String, database: SupportedDatabase) -> SqlExpr { let data_type = match database { - SupportedDatabase::MySql => DataType::Char(None), - SupportedDatabase::Mssql => DataType::Varchar(Some(CharacterLength::Max)), - SupportedDatabase::Postgres | SupportedDatabase::Sqlite => DataType::Text, + SupportedDatabase::Sqlite => DataType::Text, SupportedDatabase::Oracle => DataType::Varchar(Some(CharacterLength::IntegerLength { length: 4000, unit: None, })), + SupportedDatabase::Postgres | SupportedDatabase::MySql | SupportedDatabase::Mssql => { + return SqlExpr::value(Value::Placeholder(placeholder)); + } _ => DataType::Varchar(None), }; SqlExpr::Cast { diff --git a/tests/sql_test_files/data/variable_compared_to_integer_column_nopostgres.sql b/tests/sql_test_files/data/variable_compared_to_integer_column_nopostgres.sql new file mode 100644 index 00000000..0c2b152e --- /dev/null +++ b/tests/sql_test_files/data/variable_compared_to_integer_column_nopostgres.sql @@ -0,0 +1,7 @@ +drop table if exists variable_integer_comparison_t; +create table variable_integer_comparison_t(id int primary key, name varchar(100)); +insert into variable_integer_comparison_t(id, name) values (1, 'It works !'); + +select 'It works !' as expected, name as actual +from variable_integer_comparison_t +where id = $x; diff --git a/tests/sql_test_files/data/variable_compared_to_number_literal_nopostgres.sql b/tests/sql_test_files/data/variable_compared_to_number_literal_nopostgres.sql new file mode 100644 index 00000000..2af57e09 --- /dev/null +++ b/tests/sql_test_files/data/variable_compared_to_number_literal_nopostgres.sql @@ -0,0 +1,5 @@ +-- Comparing a variable to a numeric literal must work on every database. +-- This exercises the text cast that SQLPage keeps around variables on +-- databases that cannot infer a text parameter type from the query context. +select 'It works !' as expected, + case when $x = 1 then 'It works !' else 'It does not work' end as actual; diff --git a/tests/sql_test_files/data/variable_unicode_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql b/tests/sql_test_files/data/variable_unicode_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql new file mode 100644 index 00000000..be45d360 --- /dev/null +++ b/tests/sql_test_files/data/variable_unicode_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql @@ -0,0 +1,9 @@ +-- Comparing a variable to an nvarchar column must not mangle non-Latin +-- characters. SQLPage must not cast the variable to a narrow varchar type. +drop table if exists variable_unicode_t; +create table variable_unicode_t(name nvarchar(100)); +insert into variable_unicode_t(name) values (N'日本語'); + +SET x = N'日本語'; + +select N'日本語' as expected, name as actual from variable_unicode_t where name = $x; From 42cf5b619ac240b417380060f79bc8332d4cb85d Mon Sep 17 00:00:00 2001 From: Ophir Lojkine Date: Sat, 22 Aug 2026 20:19:20 +0000 Subject: [PATCH 02/12] fix(odbc) :: keep the text cast around variables on ODBC connections 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. --- src/webserver/database/sql.rs | 27 ++++++++++++ src/webserver/database/sql/rewrite.rs | 44 +++++++++++-------- ...riable_comparison_without_type_context.sql | 8 ++++ 3 files changed, 61 insertions(+), 18 deletions(-) create mode 100644 tests/sql_test_files/data/variable_comparison_without_type_context.sql diff --git a/src/webserver/database/sql.rs b/src/webserver/database/sql.rs index 56b74378..d60a71a6 100644 --- a/src/webserver/database/sql.rs +++ b/src/webserver/database/sql.rs @@ -843,6 +843,33 @@ mod tests { } } + #[test] + fn odbc_connections_keep_the_text_cast_even_for_postgres() { + // ODBC drivers provide no parameter type information, so the cast is + // needed even when the database behind the driver would normally + // infer it from the query context. + let database = DbInfo { + dbms_name: "PostgreSQL".into(), + database_type: SupportedDatabase::Postgres, + kind: AnyKind::Odbc, + }; + let FileStatement::Query(Query { + body: QueryBody::Database(query), + .. + }) = parse_sql( + &database, + &PostgreSqlDialect {}, + "select $a as value from t", + ) + .unwrap() + .next() + .unwrap() + else { + panic!("expected database query"); + }; + assert_eq!(query.sql, "SELECT CAST(? AS TEXT) AS value FROM t"); + } + #[test] fn postgres_limit_parameter_is_not_wrapped_in_a_cast() { // The database still requires an integer for LIMIT, so this query diff --git a/src/webserver/database/sql/rewrite.rs b/src/webserver/database/sql/rewrite.rs index f6baf323..8fe638e3 100644 --- a/src/webserver/database/sql/rewrite.rs +++ b/src/webserver/database/sql/rewrite.rs @@ -41,6 +41,7 @@ use crate::webserver::database::sqlpage_expr::{ }; use crate::webserver::database::sqlpage_functions::functions::SqlPageFunctionName; use crate::webserver::database::{DbInfo, SupportedDatabase}; +use sqlx::any::AnyKind; const SQLPAGE_INPUT_PREFIX: &str = "__sqlpage_input_"; @@ -620,7 +621,7 @@ impl QueryRewriter<'_> { PlaceholderStyle::Numbered { prefix } => format!("{prefix}{}", sequence + 1), PlaceholderStyle::Positional { .. } => format!("${}", sequence + 1), }; - cast_placeholder(placeholder, self.database.database_type) + cast_placeholder(placeholder, self.database) } fn add_row_input(&mut self, mut expression: SqlExpr) -> anyhow::Result { @@ -1038,25 +1039,32 @@ fn variable_source(prefix: char) -> VariableSource { /// Wraps a generated placeholder in the backend-specific text cast when the /// database cannot reliably infer that the parameter is a string. /// -/// `SQLPage` always binds parameters as strings. `PostgreSQL` (which pins the -/// parameter type to `TEXT` when preparing the statement), `MySQL` and `SQL -/// Server` (which convert the bound string to the type expected by the -/// surrounding expression) do not need the cast, and it can even be harmful: -/// on `SQL Server` the parameter is bound as `NVARCHAR(MAX)`, and casting it -/// to a narrow `VARCHAR` mangles non-ASCII values. `SQLite` and ODBC-backed -/// databases, whose parameter type inference is unpredictable, keep the -/// explicit cast. -fn cast_placeholder(placeholder: String, database: SupportedDatabase) -> SqlExpr { - let data_type = match database { - SupportedDatabase::Sqlite => DataType::Text, - SupportedDatabase::Oracle => DataType::Varchar(Some(CharacterLength::IntegerLength { - length: 4000, - unit: None, - })), - SupportedDatabase::Postgres | SupportedDatabase::MySql | SupportedDatabase::Mssql => { +/// `SQLPage` always binds parameters as strings. Native `PostgreSQL` (which +/// pins the parameter type to `TEXT` when preparing the statement), `MySQL` +/// and `SQL Server` (which convert the bound string to the type expected by +/// the surrounding expression) do not need the cast, and it can even be +/// harmful: on `SQL Server` the parameter is bound as `NVARCHAR(MAX)`, and +/// casting it to a narrow `VARCHAR` mangles non-ASCII values. `SQLite` and +/// ODBC connections, whose parameter type inference is unpredictable, keep +/// the explicit cast — through ODBC the cast is needed even for databases +/// that would otherwise infer the parameter type from the query context, +/// because the driver provides no parameter type information. +fn cast_placeholder(placeholder: String, database: &DbInfo) -> SqlExpr { + let data_type = match database.kind { + AnyKind::Sqlite => DataType::Text, + AnyKind::Postgres | AnyKind::MySql | AnyKind::Mssql => { return SqlExpr::value(Value::Placeholder(placeholder)); } - _ => DataType::Varchar(None), + AnyKind::Odbc => match database.database_type { + SupportedDatabase::MySql => DataType::Char(None), + SupportedDatabase::Mssql => DataType::Varchar(Some(CharacterLength::Max)), + SupportedDatabase::Postgres | SupportedDatabase::Sqlite => DataType::Text, + SupportedDatabase::Oracle => DataType::Varchar(Some(CharacterLength::IntegerLength { + length: 4000, + unit: None, + })), + _ => DataType::Varchar(None), + }, }; SqlExpr::Cast { expr: Box::new(SqlExpr::value(Value::Placeholder(placeholder))), diff --git a/tests/sql_test_files/data/variable_comparison_without_type_context.sql b/tests/sql_test_files/data/variable_comparison_without_type_context.sql new file mode 100644 index 00000000..8ae3ed0e --- /dev/null +++ b/tests/sql_test_files/data/variable_comparison_without_type_context.sql @@ -0,0 +1,8 @@ +-- Comparing variables without any surrounding type context (no column, no +-- literal) must work everywhere. This guards the text cast SQLPage keeps +-- around variables on SQLite and ODBC connections, where the database or +-- driver cannot determine the parameter type by itself. +SET other = 'other'; + +select 'It works !' as expected, 'It works !' as actual +where $x <> $other or $x is null; From 903be5b18528ba739c340f9bf73884a58b431260 Mon Sep 17 00:00:00 2001 From: Ophir Lojkine Date: Sat, 22 Aug 2026 20:39:39 +0000 Subject: [PATCH 03/12] refine: drop the text cast on MySQL, SQL Server and DuckDB behind ODBC 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. --- CHANGELOG.md | 2 +- examples/official-site/extensions-to-sql.md | 4 +- src/webserver/database/sql.rs | 77 ++++++++++++++------- src/webserver/database/sql/rewrite.rs | 23 ++++-- 4 files changed, 71 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe4af459..b0a38fe6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## unreleased - - SQLPage no longer wraps request variables in a generated `CAST(? AS TEXT)` when talking to PostgreSQL, MySQL, or Microsoft SQL Server. Those databases resolve the parameter type from the surrounding expression, so the cast was redundant there — and on SQL Server it converted the bound Unicode value to a narrow `varchar`, silently mangling non-Latin characters in comparisons with `nvarchar` columns, which now work correctly. SQLite and ODBC-backed databases keep the explicit cast. Generated SQL is now easier to read, e.g. `WHERE id = $1` instead of `WHERE id = CAST($1 AS TEXT)`. + - SQLPage no longer wraps request variables in a generated `CAST(? AS TEXT)` when talking to PostgreSQL, MySQL, Microsoft SQL Server, or DuckDB. Those databases resolve the parameter type from the surrounding expression, so the cast was redundant there — and on SQL Server it converted the bound Unicode value to a narrow `varchar`, silently mangling non-Latin characters in comparisons with `nvarchar` columns, which now work correctly. The cast is kept where it is load-bearing: on SQLite, whose comparisons need the text affinity, and on ODBC connections to PostgreSQL (whose driver provides no parameter type information), SQLite, Oracle, Snowflake, and unknown databases. Generated SQL is now easier to read, e.g. `WHERE id = $1` instead of `WHERE id = CAST($1 AS TEXT)`. - AWS Lambda builds and documentation now use the supported Amazon Linux 2023 custom runtime instead of the end-of-life Amazon Linux 2 runtime. Release artifacts include the configuration directory required on Lambda's read-only filesystem. - Added a `toast` component with plain-text or Markdown content, icons, colors, six screen placements, configurable auto-dismiss timing, optional manual dismissal, URL-fragment triggers, and automatic stacking of queued notifications. - `sqlpage.send_mail` now supports rich email bodies. Use `body_html` for a caller-provided HTML alternative, or `body_md` to render Markdown as HTML. Messages retain a plain-text alternative; `body` may be omitted when `body_md` is used, and `body_md` and `body_html` cannot be combined. diff --git a/examples/official-site/extensions-to-sql.md b/examples/official-site/extensions-to-sql.md index 43eceaa9..f49d71ad 100644 --- a/examples/official-site/extensions-to-sql.md +++ b/examples/official-site/extensions-to-sql.md @@ -144,9 +144,9 @@ Only a single textual value (**string or `NULL`**) is stored. `SET id = 1` will store the string `'1'`, not the number `1`. Variables are always sent to the database as text. -On SQLite and databases connected through ODBC, SQLPage wraps variables in an explicit cast to text, because their parameter type inference would otherwise make comparisons unpredictable. +On SQLite, and on ODBC connections to PostgreSQL, SQLite, Oracle, Snowflake, or other databases, SQLPage wraps variables in an explicit cast to text, because their parameter type handling would otherwise make comparisons unpredictable. On PostgreSQL, the variable is passed as text, and comparing it to a non-text column requires an explicit cast. -On MySQL and Microsoft SQL Server, the database converts the variable to the type expected by the surrounding expression. +On MySQL, Microsoft SQL Server, and DuckDB, the database converts the variable to the type expected by the surrounding expression. On databases with a strict type system, such as PostgreSQL, if you need a number, you will need to cast your variables: `SELECT * FROM post WHERE id = $id::int`. Complex structures can be stored as json strings. diff --git a/src/webserver/database/sql.rs b/src/webserver/database/sql.rs index d60a71a6..904ca4b6 100644 --- a/src/webserver/database/sql.rs +++ b/src/webserver/database/sql.rs @@ -806,7 +806,6 @@ mod tests { for database_type in [ SupportedDatabase::Sqlite, SupportedDatabase::Oracle, - SupportedDatabase::Duckdb, SupportedDatabase::Snowflake, SupportedDatabase::Generic, ] { @@ -827,6 +826,7 @@ mod tests { SupportedDatabase::Postgres, SupportedDatabase::MySql, SupportedDatabase::Mssql, + SupportedDatabase::Duckdb, ] { let FileStatement::Query(Query { body: QueryBody::Database(query), @@ -844,30 +844,57 @@ mod tests { } #[test] - fn odbc_connections_keep_the_text_cast_even_for_postgres() { - // ODBC drivers provide no parameter type information, so the cast is - // needed even when the database behind the driver would normally - // infer it from the query context. - let database = DbInfo { - dbms_name: "PostgreSQL".into(), - database_type: SupportedDatabase::Postgres, - kind: AnyKind::Odbc, - }; - let FileStatement::Query(Query { - body: QueryBody::Database(query), - .. - }) = parse_sql( - &database, - &PostgreSqlDialect {}, - "select $a as value from t", - ) - .unwrap() - .next() - .unwrap() - else { - panic!("expected database query"); - }; - assert_eq!(query.sql, "SELECT CAST(? AS TEXT) AS value FROM t"); + fn odbc_cast_follows_the_database_behind_the_driver() { + for (database_type, keeps_cast, expected_sql) in [ + // psqlodbc provides no parameter type information, so PostgreSQL + // fails on context-free parameters without the cast. + ( + SupportedDatabase::Postgres, + true, + "SELECT CAST(? AS TEXT) AS value FROM t", + ), + // SQLite needs the cast for text affinity in comparisons, just + // like native connections. + ( + SupportedDatabase::Sqlite, + true, + "SELECT CAST(? AS TEXT) AS value FROM t", + ), + // These databases convert the bound string at execution time or + // default untyped parameters to strings, like native connections. + (SupportedDatabase::MySql, false, "SELECT ? AS value FROM t"), + (SupportedDatabase::Mssql, false, "SELECT ? AS value FROM t"), + (SupportedDatabase::Duckdb, false, "SELECT ? AS value FROM t"), + ] { + let database = DbInfo { + dbms_name: database_type.display_name().to_owned(), + database_type, + kind: AnyKind::Odbc, + }; + let FileStatement::Query(Query { + body: QueryBody::Database(query), + .. + }) = parse_sql( + &database, + &PostgreSqlDialect {}, + "select $a as value from t", + ) + .unwrap() + .next() + .unwrap() + else { + panic!("expected database query"); + }; + assert_eq!( + query.sql, expected_sql, + "{database_type:?} behind ODBC generated unexpected SQL" + ); + assert_eq!( + query.sql.to_lowercase().contains("cast"), + keeps_cast, + "{database_type:?} behind ODBC: cast presence mismatch" + ); + } } #[test] diff --git a/src/webserver/database/sql/rewrite.rs b/src/webserver/database/sql/rewrite.rs index 8fe638e3..819de120 100644 --- a/src/webserver/database/sql/rewrite.rs +++ b/src/webserver/database/sql/rewrite.rs @@ -1044,11 +1044,19 @@ fn variable_source(prefix: char) -> VariableSource { /// and `SQL Server` (which convert the bound string to the type expected by /// the surrounding expression) do not need the cast, and it can even be /// harmful: on `SQL Server` the parameter is bound as `NVARCHAR(MAX)`, and -/// casting it to a narrow `VARCHAR` mangles non-ASCII values. `SQLite` and -/// ODBC connections, whose parameter type inference is unpredictable, keep -/// the explicit cast — through ODBC the cast is needed even for databases -/// that would otherwise infer the parameter type from the query context, -/// because the driver provides no parameter type information. +/// casting it to a narrow `VARCHAR` mangles non-ASCII values. `SQLite` +/// needs it to keep text affinity in comparisons with numbers. +/// +/// Through ODBC, the decision follows the database behind the driver, since +/// `SQLPage` knows it from the driver's reported name: +/// - `PostgreSQL` keeps the cast: `psqlodbc` provides no parameter type +/// information, and the server then fails on context-free parameters +/// (`could not determine data type of parameter`). +/// - `SQLite` keeps it for the same affinity reasons as native connections. +/// - `MySQL`, `SQL Server` and `DuckDB` drop it, like their native +/// counterparts: the former two convert the string at execution time, and +/// `DuckDB` defaults untyped parameters to `VARCHAR`. +/// - `Oracle`, `Snowflake` and unknown databases keep it conservatively. fn cast_placeholder(placeholder: String, database: &DbInfo) -> SqlExpr { let data_type = match database.kind { AnyKind::Sqlite => DataType::Text, @@ -1056,13 +1064,14 @@ fn cast_placeholder(placeholder: String, database: &DbInfo) -> SqlExpr { return SqlExpr::value(Value::Placeholder(placeholder)); } AnyKind::Odbc => match database.database_type { - SupportedDatabase::MySql => DataType::Char(None), - SupportedDatabase::Mssql => DataType::Varchar(Some(CharacterLength::Max)), SupportedDatabase::Postgres | SupportedDatabase::Sqlite => DataType::Text, SupportedDatabase::Oracle => DataType::Varchar(Some(CharacterLength::IntegerLength { length: 4000, unit: None, })), + SupportedDatabase::MySql | SupportedDatabase::Mssql | SupportedDatabase::Duckdb => { + return SqlExpr::value(Value::Placeholder(placeholder)); + } _ => DataType::Varchar(None), }, }; From 45695116df302ff5680b434d7f3e6e4bfeb16d74 Mon Sep 17 00:00:00 2001 From: Ophir Lojkine Date: Sat, 22 Aug 2026 20:52:16 +0000 Subject: [PATCH 04/12] docs(changelog): make variable cast entry concise and user-oriented 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). --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b0a38fe6..fb5c5c4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## unreleased - - SQLPage no longer wraps request variables in a generated `CAST(? AS TEXT)` when talking to PostgreSQL, MySQL, Microsoft SQL Server, or DuckDB. Those databases resolve the parameter type from the surrounding expression, so the cast was redundant there — and on SQL Server it converted the bound Unicode value to a narrow `varchar`, silently mangling non-Latin characters in comparisons with `nvarchar` columns, which now work correctly. The cast is kept where it is load-bearing: on SQLite, whose comparisons need the text affinity, and on ODBC connections to PostgreSQL (whose driver provides no parameter type information), SQLite, Oracle, Snowflake, and unknown databases. Generated SQL is now easier to read, e.g. `WHERE id = $1` instead of `WHERE id = CAST($1 AS TEXT)`. + - Fixed `nvarchar` comparisons with non-ASCII characters on Microsoft SQL Server, which were silently mangled by an unnecessary cast to `varchar`. Request variables are no longer cast to text on PostgreSQL, MySQL, SQL Server and DuckDB, where the database infers the type from context; the cast is retained on SQLite and on ODBC connections where it is required for correct comparisons. - AWS Lambda builds and documentation now use the supported Amazon Linux 2023 custom runtime instead of the end-of-life Amazon Linux 2 runtime. Release artifacts include the configuration directory required on Lambda's read-only filesystem. - Added a `toast` component with plain-text or Markdown content, icons, colors, six screen placements, configurable auto-dismiss timing, optional manual dismissal, URL-fragment triggers, and automatic stacking of queued notifications. - `sqlpage.send_mail` now supports rich email bodies. Use `body_html` for a caller-provided HTML alternative, or `body_md` to render Markdown as HTML. Messages retain a plain-text alternative; `body` may be omitted when `body_md` is used, and `body_md` and `body_html` cannot be combined. From 11ab19233dd8c0b79c9af0b0cf27e0d7dfb25058 Mon Sep 17 00:00:00 2001 From: Ophir Lojkine Date: Sat, 22 Aug 2026 20:55:27 +0000 Subject: [PATCH 05/12] docs(changelog): add issue references for variable cast fix Fixes: #516 (CONTAINS with MSSQL variable fails due to CAST), #1154 (LIMIT/OFFSET with variables fails on MySQL/MariaDB). See: #1317 (per-database logic still scattered, this is a step toward the SqlDialect abstraction). --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb5c5c4e..f5a8a949 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## unreleased - - Fixed `nvarchar` comparisons with non-ASCII characters on Microsoft SQL Server, which were silently mangled by an unnecessary cast to `varchar`. Request variables are no longer cast to text on PostgreSQL, MySQL, SQL Server and DuckDB, where the database infers the type from context; the cast is retained on SQLite and on ODBC connections where it is required for correct comparisons. + - Fixed `nvarchar` comparisons with non-ASCII characters on Microsoft SQL Server, which were silently mangled by an unnecessary cast to `varchar`. Request variables are no longer cast to text on PostgreSQL, MySQL, SQL Server and DuckDB, where the database infers the type from context; the cast is retained on SQLite and on ODBC connections where it is required for correct comparisons. fixes: #516, #1154. see: #1317. - AWS Lambda builds and documentation now use the supported Amazon Linux 2023 custom runtime instead of the end-of-life Amazon Linux 2 runtime. Release artifacts include the configuration directory required on Lambda's read-only filesystem. - Added a `toast` component with plain-text or Markdown content, icons, colors, six screen placements, configurable auto-dismiss timing, optional manual dismissal, URL-fragment triggers, and automatic stacking of queued notifications. - `sqlpage.send_mail` now supports rich email bodies. Use `body_html` for a caller-provided HTML alternative, or `body_md` to render Markdown as HTML. Messages retain a plain-text alternative; `body` may be omitted when `body_md` is used, and `body_md` and `body_html` cannot be combined. From c96bbc0fe240f4f682874d783959bbab9f80228c Mon Sep 17 00:00:00 2001 From: Ophir Lojkine Date: Sat, 22 Aug 2026 21:03:39 +0000 Subject: [PATCH 06/12] test: simplify variable cast tests and add focused repros for #516 and #1154 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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. --- src/webserver/database/sql.rs | 169 +++++++----------- ..._compared_to_integer_column_nopostgres.sql | 8 +- ..._compared_to_number_literal_nopostgres.sql | 7 +- ...riable_comparison_without_type_context.sql | 9 +- ...oracle_nopostgres_nosnowflake_nosqlite.sql | 6 + ...oracle_nopostgres_nosnowflake_nosqlite.sql | 3 + ...oracle_nopostgres_nosnowflake_nosqlite.sql | 7 +- 7 files changed, 85 insertions(+), 124 deletions(-) create mode 100644 tests/sql_test_files/data/variable_limit_offset_noduckdb_nogeneric_nomssql_nooracle_nopostgres_nosnowflake_nosqlite.sql create mode 100644 tests/sql_test_files/data/variable_mssql_contains_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql diff --git a/src/webserver/database/sql.rs b/src/webserver/database/sql.rs index 904ca4b6..0ab7581c 100644 --- a/src/webserver/database/sql.rs +++ b/src/webserver/database/sql.rs @@ -801,131 +801,96 @@ mod tests { ); } + fn sql_for(db: SupportedDatabase, sql: &str) -> String { + let FileStatement::Query(Query { + body: QueryBody::Database(q), + .. + }) = one_for(db, sql) + else { + panic!("expected database query"); + }; + q.sql + } + + fn odbc_sql_for(db: SupportedDatabase, sql: &str) -> String { + let info = DbInfo { + dbms_name: db.display_name().to_owned(), + database_type: db, + kind: AnyKind::Odbc, + }; + let FileStatement::Query(Query { + body: QueryBody::Database(q), + .. + }) = parse_sql(&info, &PostgreSqlDialect {}, sql) + .unwrap() + .next() + .unwrap() + else { + panic!("expected database query"); + }; + q.sql + } + #[test] - fn text_cast_is_only_generated_when_parameter_typing_is_unpredictable() { - for database_type in [ + fn variables_keep_cast_only_where_typing_is_unpredictable() { + for db in [ SupportedDatabase::Sqlite, SupportedDatabase::Oracle, SupportedDatabase::Snowflake, SupportedDatabase::Generic, ] { - let FileStatement::Query(Query { - body: QueryBody::Database(query), - .. - }) = one_for(database_type, "select $a as value from t") - else { - panic!("expected database query"); - }; - assert!( - query.sql.to_lowercase().contains("cast"), - "{database_type:?} should keep the text cast: {}", - query.sql - ); + assert!(sql_for(db, "select $a as value from t").contains("CAST")); } - for database_type in [ + for db in [ SupportedDatabase::Postgres, SupportedDatabase::MySql, SupportedDatabase::Mssql, SupportedDatabase::Duckdb, ] { - let FileStatement::Query(Query { - body: QueryBody::Database(query), - .. - }) = one_for(database_type, "select $a as value from t") - else { - panic!("expected database query"); - }; - assert!( - !query.sql.to_lowercase().contains("cast"), - "{database_type:?} should not generate a cast: {}", - query.sql - ); + assert!(!sql_for(db, "select $a as value from t").contains("CAST")); } } #[test] - fn odbc_cast_follows_the_database_behind_the_driver() { - for (database_type, keeps_cast, expected_sql) in [ - // psqlodbc provides no parameter type information, so PostgreSQL - // fails on context-free parameters without the cast. - ( - SupportedDatabase::Postgres, - true, - "SELECT CAST(? AS TEXT) AS value FROM t", - ), - // SQLite needs the cast for text affinity in comparisons, just - // like native connections. - ( - SupportedDatabase::Sqlite, - true, - "SELECT CAST(? AS TEXT) AS value FROM t", - ), - // These databases convert the bound string at execution time or - // default untyped parameters to strings, like native connections. - (SupportedDatabase::MySql, false, "SELECT ? AS value FROM t"), - (SupportedDatabase::Mssql, false, "SELECT ? AS value FROM t"), - (SupportedDatabase::Duckdb, false, "SELECT ? AS value FROM t"), - ] { - let database = DbInfo { - dbms_name: database_type.display_name().to_owned(), - database_type, - kind: AnyKind::Odbc, - }; - let FileStatement::Query(Query { - body: QueryBody::Database(query), - .. - }) = parse_sql( - &database, - &PostgreSqlDialect {}, - "select $a as value from t", - ) - .unwrap() - .next() - .unwrap() - else { - panic!("expected database query"); - }; - assert_eq!( - query.sql, expected_sql, - "{database_type:?} behind ODBC generated unexpected SQL" - ); - assert_eq!( - query.sql.to_lowercase().contains("cast"), - keeps_cast, - "{database_type:?} behind ODBC: cast presence mismatch" - ); - } + fn odbc_cast_follows_database() { + assert_eq!( + odbc_sql_for(SupportedDatabase::Postgres, "select $a as value from t"), + "SELECT CAST(? AS TEXT) AS value FROM t" + ); + assert_eq!( + odbc_sql_for(SupportedDatabase::Sqlite, "select $a as value from t"), + "SELECT CAST(? AS TEXT) AS value FROM t" + ); + assert_eq!( + odbc_sql_for(SupportedDatabase::MySql, "select $a as value from t"), + "SELECT ? AS value FROM t" + ); + assert_eq!( + odbc_sql_for(SupportedDatabase::Mssql, "select $a as value from t"), + "SELECT ? AS value FROM t" + ); + assert_eq!( + odbc_sql_for(SupportedDatabase::Duckdb, "select $a as value from t"), + "SELECT ? AS value FROM t" + ); } #[test] - fn postgres_limit_parameter_is_not_wrapped_in_a_cast() { - // The database still requires an integer for LIMIT, so this query - // fails at execution time with a string parameter; this test only - // locks in that SQLPage does not generate a text cast around it. + fn limit_uses_bare_parameter() { assert_eq!( - rewrite_database("select value from t limit $n"), - DatabaseQuery { - sql: "SELECT value FROM t LIMIT $1".into(), - bindings: Box::new([variable("n")]), - row_input_json: Box::new([]), - computed_columns: Box::new([]), - json_columns: Box::new([]), - } + sql_for(SupportedDatabase::Postgres, "select value from t limit $n"), + "SELECT value FROM t LIMIT $1" ); } #[test] - fn mssql_parameters_are_not_cast_to_narrow_varchar() { - let FileStatement::Query(Query { - body: QueryBody::Database(query), - .. - }) = one_for( - SupportedDatabase::Mssql, - "select name from t where name = $x", - ) - else { - panic!("expected database query"); - }; - assert_eq!(query.sql, "SELECT name FROM t WHERE name = @p1"); + fn mssql_uses_nvarchar_parameter() { + assert_eq!( + sql_for( + SupportedDatabase::Mssql, + "select name from t where name = $x" + ), + "SELECT name FROM t WHERE name = @p1" + ); } } diff --git a/tests/sql_test_files/data/variable_compared_to_integer_column_nopostgres.sql b/tests/sql_test_files/data/variable_compared_to_integer_column_nopostgres.sql index 0c2b152e..716c0486 100644 --- a/tests/sql_test_files/data/variable_compared_to_integer_column_nopostgres.sql +++ b/tests/sql_test_files/data/variable_compared_to_integer_column_nopostgres.sql @@ -1,7 +1,5 @@ +-- https://github.com/sqlpage/SQLPage/issues/1154 (variables must work in typed contexts) drop table if exists variable_integer_comparison_t; create table variable_integer_comparison_t(id int primary key, name varchar(100)); -insert into variable_integer_comparison_t(id, name) values (1, 'It works !'); - -select 'It works !' as expected, name as actual -from variable_integer_comparison_t -where id = $x; +insert into variable_integer_comparison_t values (1, 'It works !'); +select 'It works !' as expected, name as actual from variable_integer_comparison_t where id = $x; diff --git a/tests/sql_test_files/data/variable_compared_to_number_literal_nopostgres.sql b/tests/sql_test_files/data/variable_compared_to_number_literal_nopostgres.sql index 2af57e09..e8a4b4db 100644 --- a/tests/sql_test_files/data/variable_compared_to_number_literal_nopostgres.sql +++ b/tests/sql_test_files/data/variable_compared_to_number_literal_nopostgres.sql @@ -1,5 +1,2 @@ --- Comparing a variable to a numeric literal must work on every database. --- This exercises the text cast that SQLPage keeps around variables on --- databases that cannot infer a text parameter type from the query context. -select 'It works !' as expected, - case when $x = 1 then 'It works !' else 'It does not work' end as actual; +-- https://github.com/sqlpage/SQLPage/issues/1154 +select 'It works !' as expected, case when $x = 1 then 'It works !' else 'fail' end as actual; diff --git a/tests/sql_test_files/data/variable_comparison_without_type_context.sql b/tests/sql_test_files/data/variable_comparison_without_type_context.sql index 8ae3ed0e..a49b759d 100644 --- a/tests/sql_test_files/data/variable_comparison_without_type_context.sql +++ b/tests/sql_test_files/data/variable_comparison_without_type_context.sql @@ -1,8 +1,3 @@ --- Comparing variables without any surrounding type context (no column, no --- literal) must work everywhere. This guards the text cast SQLPage keeps --- around variables on SQLite and ODBC connections, where the database or --- driver cannot determine the parameter type by itself. +-- https://github.com/sqlpage/SQLPage/issues/516 (psqlodbc needs CAST for context-free params) SET other = 'other'; - -select 'It works !' as expected, 'It works !' as actual -where $x <> $other or $x is null; +select 'It works !' as expected, 'It works !' as actual where $x <> $other or $x is null; diff --git a/tests/sql_test_files/data/variable_limit_offset_noduckdb_nogeneric_nomssql_nooracle_nopostgres_nosnowflake_nosqlite.sql b/tests/sql_test_files/data/variable_limit_offset_noduckdb_nogeneric_nomssql_nooracle_nopostgres_nosnowflake_nosqlite.sql new file mode 100644 index 00000000..663f8a31 --- /dev/null +++ b/tests/sql_test_files/data/variable_limit_offset_noduckdb_nogeneric_nomssql_nooracle_nopostgres_nosnowflake_nosqlite.sql @@ -0,0 +1,6 @@ +-- https://github.com/sqlpage/SQLPage/issues/1154 +drop table if exists variable_limit_offset_t; +create table variable_limit_offset_t(id int primary key, v varchar(10)); +insert into variable_limit_offset_t values (1,'a'),(2,'It works !'),(3,'c'); +SET lim = 1; SET off = 1; +select 'It works !' as expected, v as actual from variable_limit_offset_t order by id limit $lim offset $off; diff --git a/tests/sql_test_files/data/variable_mssql_contains_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql b/tests/sql_test_files/data/variable_mssql_contains_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql new file mode 100644 index 00000000..058b5157 --- /dev/null +++ b/tests/sql_test_files/data/variable_mssql_contains_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql @@ -0,0 +1,3 @@ +-- https://github.com/sqlpage/SQLPage/issues/516 +SET x = 'It works !'; +exec sp_executesql N'SELECT ''It works !'' as expected, @p as actual', N'@p varchar(100)', @p=$x; diff --git a/tests/sql_test_files/data/variable_unicode_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql b/tests/sql_test_files/data/variable_unicode_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql index be45d360..17a57ab1 100644 --- a/tests/sql_test_files/data/variable_unicode_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql +++ b/tests/sql_test_files/data/variable_unicode_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql @@ -1,9 +1,6 @@ --- Comparing a variable to an nvarchar column must not mangle non-Latin --- characters. SQLPage must not cast the variable to a narrow varchar type. +-- https://github.com/sqlpage/SQLPage/issues/516 (nvarchar mangling, same root cause) drop table if exists variable_unicode_t; create table variable_unicode_t(name nvarchar(100)); -insert into variable_unicode_t(name) values (N'日本語'); - +insert into variable_unicode_t values (N'日本語'); SET x = N'日本語'; - select N'日本語' as expected, name as actual from variable_unicode_t where name = $x; From 0355fc444d69c9c9359b1dd2f90e47732a301231 Mon Sep 17 00:00:00 2001 From: Ophir Lojkine Date: Sat, 22 Aug 2026 21:09:22 +0000 Subject: [PATCH 07/12] test: make variable fixtures self-contained 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. --- .../data/variable_compared_to_integer_column_nopostgres.sql | 2 +- .../data/variable_compared_to_number_literal_nopostgres.sql | 2 +- .../data/variable_comparison_without_type_context.sql | 2 +- ...generic_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/sql_test_files/data/variable_compared_to_integer_column_nopostgres.sql b/tests/sql_test_files/data/variable_compared_to_integer_column_nopostgres.sql index 716c0486..487b9af2 100644 --- a/tests/sql_test_files/data/variable_compared_to_integer_column_nopostgres.sql +++ b/tests/sql_test_files/data/variable_compared_to_integer_column_nopostgres.sql @@ -1,4 +1,4 @@ --- https://github.com/sqlpage/SQLPage/issues/1154 (variables must work in typed contexts) +-- Variable compared to integer column must work without explicit cast drop table if exists variable_integer_comparison_t; create table variable_integer_comparison_t(id int primary key, name varchar(100)); insert into variable_integer_comparison_t values (1, 'It works !'); diff --git a/tests/sql_test_files/data/variable_compared_to_number_literal_nopostgres.sql b/tests/sql_test_files/data/variable_compared_to_number_literal_nopostgres.sql index e8a4b4db..3777dbbc 100644 --- a/tests/sql_test_files/data/variable_compared_to_number_literal_nopostgres.sql +++ b/tests/sql_test_files/data/variable_compared_to_number_literal_nopostgres.sql @@ -1,2 +1,2 @@ --- https://github.com/sqlpage/SQLPage/issues/1154 +-- Variable compared to numeric literal must work (SQLite/ODBC need CAST) select 'It works !' as expected, case when $x = 1 then 'It works !' else 'fail' end as actual; diff --git a/tests/sql_test_files/data/variable_comparison_without_type_context.sql b/tests/sql_test_files/data/variable_comparison_without_type_context.sql index a49b759d..1027f834 100644 --- a/tests/sql_test_files/data/variable_comparison_without_type_context.sql +++ b/tests/sql_test_files/data/variable_comparison_without_type_context.sql @@ -1,3 +1,3 @@ --- https://github.com/sqlpage/SQLPage/issues/516 (psqlodbc needs CAST for context-free params) +-- Context-free variables (no column or literal) need CAST on SQLite and psqlodbc SET other = 'other'; select 'It works !' as expected, 'It works !' as actual where $x <> $other or $x is null; diff --git a/tests/sql_test_files/data/variable_unicode_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql b/tests/sql_test_files/data/variable_unicode_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql index 17a57ab1..3ed35c24 100644 --- a/tests/sql_test_files/data/variable_unicode_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql +++ b/tests/sql_test_files/data/variable_unicode_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql @@ -1,4 +1,4 @@ --- https://github.com/sqlpage/SQLPage/issues/516 (nvarchar mangling, same root cause) +-- MSSQL nvarchar with non-ASCII must not be mangled by CAST to VARCHAR drop table if exists variable_unicode_t; create table variable_unicode_t(name nvarchar(100)); insert into variable_unicode_t values (N'日本語'); From 342a5fa522c07cc694a25899cf513cc1b75944fa Mon Sep 17 00:00:00 2001 From: Ophir Lojkine Date: Sat, 22 Aug 2026 21:21:09 +0000 Subject: [PATCH 08/12] fix: mssql test escaping and improve changelog - 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. --- CHANGELOG.md | 6 +++++- ...ric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5a8a949..2e0d424b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,11 @@ ## unreleased - - Fixed `nvarchar` comparisons with non-ASCII characters on Microsoft SQL Server, which were silently mangled by an unnecessary cast to `varchar`. Request variables are no longer cast to text on PostgreSQL, MySQL, SQL Server and DuckDB, where the database infers the type from context; the cast is retained on SQLite and on ODBC connections where it is required for correct comparisons. fixes: #516, #1154. see: #1317. + - Removed unnecessary `CAST` around request variables: + - On PostgreSQL, MySQL, SQL Server and DuckDB, variables are now sent without a text cast and the database infers the type from context, which keeps generated SQL readable (`WHERE id = $1` instead of `WHERE id = CAST($1 AS TEXT)`) and fixes cases where the cast was harmful. + - On SQL Server, this fixes `nvarchar` comparisons with non-ASCII characters that were previously mangled by `CAST(... AS VARCHAR)`, and fixes `CONTAINS` and `EXEC` with variables. + - On MySQL/MariaDB, this fixes `LIMIT`/`OFFSET` with variables. + - The cast is retained on SQLite and on ODBC connections to PostgreSQL, SQLite, Oracle, Snowflake and other databases where it is needed for correct comparisons. - AWS Lambda builds and documentation now use the supported Amazon Linux 2023 custom runtime instead of the end-of-life Amazon Linux 2 runtime. Release artifacts include the configuration directory required on Lambda's read-only filesystem. - Added a `toast` component with plain-text or Markdown content, icons, colors, six screen placements, configurable auto-dismiss timing, optional manual dismissal, URL-fragment triggers, and automatic stacking of queued notifications. - `sqlpage.send_mail` now supports rich email bodies. Use `body_html` for a caller-provided HTML alternative, or `body_md` to render Markdown as HTML. Messages retain a plain-text alternative; `body` may be omitted when `body_md` is used, and `body_md` and `body_html` cannot be combined. diff --git a/tests/sql_test_files/data/variable_mssql_contains_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql b/tests/sql_test_files/data/variable_mssql_contains_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql index 058b5157..1671698f 100644 --- a/tests/sql_test_files/data/variable_mssql_contains_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql +++ b/tests/sql_test_files/data/variable_mssql_contains_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql @@ -1,3 +1,3 @@ -- https://github.com/sqlpage/SQLPage/issues/516 SET x = 'It works !'; -exec sp_executesql N'SELECT ''It works !'' as expected, @p as actual', N'@p varchar(100)', @p=$x; +exec sp_executesql N'SELECT @p as actual, @exp as expected', N'@p varchar(100), @exp varchar(100)', @p=$x, @exp='It works !'; From e62a91f85682c42315d2e0aab13ad1b783b11623 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 23 Aug 2026 09:05:47 +0200 Subject: [PATCH 09/12] Refactor SQL variable cast tests to use typed assertions --- src/webserver/database/sql.rs | 109 ++++++++++++---------------------- 1 file changed, 37 insertions(+), 72 deletions(-) diff --git a/src/webserver/database/sql.rs b/src/webserver/database/sql.rs index 0ab7581c..39334538 100644 --- a/src/webserver/database/sql.rs +++ b/src/webserver/database/sql.rs @@ -627,7 +627,7 @@ mod tests { let statement = parse_sql( &database, &MySqlDialect {}, - "select '@SQLPAGE_TEMP1' as value from t where id = $id", + "select '@SQLPAGE_TEMP1' where id = $id", ) .unwrap() .next() @@ -801,78 +801,54 @@ mod tests { ); } + fn sql_for_dbinfo(info: DbInfo, sql: &str) -> String { + match parse_sql(&info, &PostgreSqlDialect {}, sql).unwrap().next() { + Some(FileStatement::Query(Query { + body: QueryBody::Database(q), + .. + })) => q.sql, + other => panic!("Expected database query for `{sql}`\nGot: {other:?}"), + } + } + fn sql_for(db: SupportedDatabase, sql: &str) -> String { - let FileStatement::Query(Query { - body: QueryBody::Database(q), - .. - }) = one_for(db, sql) - else { - panic!("expected database query"); - }; - q.sql + sql_for_dbinfo(database(db), sql) } fn odbc_sql_for(db: SupportedDatabase, sql: &str) -> String { - let info = DbInfo { - dbms_name: db.display_name().to_owned(), - database_type: db, - kind: AnyKind::Odbc, - }; - let FileStatement::Query(Query { - body: QueryBody::Database(q), - .. - }) = parse_sql(&info, &PostgreSqlDialect {}, sql) - .unwrap() - .next() - .unwrap() - else { - panic!("expected database query"); - }; - q.sql + sql_for_dbinfo( + DbInfo { + dbms_name: db.display_name().to_owned(), + database_type: db, + kind: AnyKind::Odbc, + }, + sql, + ) } #[test] fn variables_keep_cast_only_where_typing_is_unpredictable() { - for db in [ - SupportedDatabase::Sqlite, - SupportedDatabase::Oracle, - SupportedDatabase::Snowflake, - SupportedDatabase::Generic, - ] { - assert!(sql_for(db, "select $a as value from t").contains("CAST")); - } - for db in [ - SupportedDatabase::Postgres, - SupportedDatabase::MySql, - SupportedDatabase::Mssql, - SupportedDatabase::Duckdb, - ] { - assert!(!sql_for(db, "select $a as value from t").contains("CAST")); - } + use SupportedDatabase::*; + let src = "SELECT $a"; + assert_eq!(sql_for(Sqlite, src), "SELECT CAST(?1 AS TEXT)"); + assert_eq!(sql_for(Oracle, src), "SELECT CAST(? AS VARCHAR(4000))"); + assert_eq!(sql_for(Snowflake, src), "SELECT CAST(? AS VARCHAR)"); + assert_eq!(sql_for(Generic, src), "SELECT CAST(? AS VARCHAR)"); + assert_eq!(sql_for(Postgres, src), "SELECT $1"); + assert_eq!(sql_for(MySql, src), "SELECT ?"); + assert_eq!(sql_for(Mssql, src), "SELECT @p1"); + assert_eq!(sql_for(Duckdb, src), "SELECT ?"); } #[test] fn odbc_cast_follows_database() { - assert_eq!( - odbc_sql_for(SupportedDatabase::Postgres, "select $a as value from t"), - "SELECT CAST(? AS TEXT) AS value FROM t" - ); - assert_eq!( - odbc_sql_for(SupportedDatabase::Sqlite, "select $a as value from t"), - "SELECT CAST(? AS TEXT) AS value FROM t" - ); - assert_eq!( - odbc_sql_for(SupportedDatabase::MySql, "select $a as value from t"), - "SELECT ? AS value FROM t" - ); - assert_eq!( - odbc_sql_for(SupportedDatabase::Mssql, "select $a as value from t"), - "SELECT ? AS value FROM t" - ); - assert_eq!( - odbc_sql_for(SupportedDatabase::Duckdb, "select $a as value from t"), - "SELECT ? AS value FROM t" - ); + use SupportedDatabase::*; + for db in [Postgres, Sqlite] { + assert_eq!(odbc_sql_for(db, "select $a"), "SELECT CAST(? AS TEXT)"); + } + for db in [MySql, Mssql, Duckdb] { + assert_eq!(odbc_sql_for(db, "select $a"), "SELECT ?"); + } } #[test] @@ -882,15 +858,4 @@ mod tests { "SELECT value FROM t LIMIT $1" ); } - - #[test] - fn mssql_uses_nvarchar_parameter() { - assert_eq!( - sql_for( - SupportedDatabase::Mssql, - "select name from t where name = $x" - ), - "SELECT name FROM t WHERE name = @p1" - ); - } } From 08988a9c64d1d159c33c383f624007b5d434ba97 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 23 Aug 2026 09:45:08 +0200 Subject: [PATCH 10/12] clippy --- src/webserver/database/sql.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/webserver/database/sql.rs b/src/webserver/database/sql.rs index 39334538..ef550c17 100644 --- a/src/webserver/database/sql.rs +++ b/src/webserver/database/sql.rs @@ -801,8 +801,8 @@ mod tests { ); } - fn sql_for_dbinfo(info: DbInfo, sql: &str) -> String { - match parse_sql(&info, &PostgreSqlDialect {}, sql).unwrap().next() { + fn sql_for_dbinfo(info: &DbInfo, sql: &str) -> String { + match parse_sql(info, &PostgreSqlDialect {}, sql).unwrap().next() { Some(FileStatement::Query(Query { body: QueryBody::Database(q), .. @@ -812,12 +812,12 @@ mod tests { } fn sql_for(db: SupportedDatabase, sql: &str) -> String { - sql_for_dbinfo(database(db), sql) + sql_for_dbinfo(&database(db), sql) } fn odbc_sql_for(db: SupportedDatabase, sql: &str) -> String { sql_for_dbinfo( - DbInfo { + &DbInfo { dbms_name: db.display_name().to_owned(), database_type: db, kind: AnyKind::Odbc, From 5c288b0ac9eda29f6ea676c22c13f2047822c108 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 23 Aug 2026 09:47:58 +0200 Subject: [PATCH 11/12] Move database-specific SQL tests into per-database subdirectories Restructure tests so files that only work on a single database engine are organized under `database-specific//` 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. --- tests/sql_test_files/README.md | 11 ++++- .../mssql/variable_mssql_contains.sql} | 0 .../mssql/variable_unicode.sql} | 0 .../mysql/variable_limit_offset.sql} | 0 .../sqlite/set_multiple_rows.sql} | 0 tests/sql_test_files/mod.rs | 41 ++++++++++++++++--- 6 files changed, 46 insertions(+), 6 deletions(-) rename tests/sql_test_files/data/{variable_mssql_contains_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql => database-specific/mssql/variable_mssql_contains.sql} (100%) rename tests/sql_test_files/data/{variable_unicode_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql => database-specific/mssql/variable_unicode.sql} (100%) rename tests/sql_test_files/data/{variable_limit_offset_noduckdb_nogeneric_nomssql_nooracle_nopostgres_nosnowflake_nosqlite.sql => database-specific/mysql/variable_limit_offset.sql} (100%) rename tests/sql_test_files/data/{set_multiple_rows_noduckdb_nogeneric_nomssql_nomysql_nooracle_nopostgres_nosnowflake.sql => database-specific/sqlite/set_multiple_rows.sql} (100%) diff --git a/tests/sql_test_files/README.md b/tests/sql_test_files/README.md index f28b3722..6e23b124 100644 --- a/tests/sql_test_files/README.md +++ b/tests/sql_test_files/README.md @@ -15,4 +15,13 @@ and the rest of the file name. Files may include `nosqlite`, `nomssql`, Files that only validate data-processing functions should live here. They must return rows with an `actual` column plus either `expected` (exact match) or `expected_contains` (substring match). Tests in this directory are fetched as -JSON and validated row by row. \ No newline at end of file +JSON and validated row by row. + +### `data/database-specific/` + +Files that only work on a single database engine (because they use +engine-specific SQL syntax) live in a subdirectory named after that database +(`sqlite`, `postgres`, `mysql`, `mssql`, `oracle`, `duckdb`, `snowflake`, +`generic`). They are run by a separate test, only when the current database +matches. Unlike the other directories, their file names do not need `_no...` +suffixes to exclude incompatible backends. \ No newline at end of file diff --git a/tests/sql_test_files/data/variable_mssql_contains_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql b/tests/sql_test_files/data/database-specific/mssql/variable_mssql_contains.sql similarity index 100% rename from tests/sql_test_files/data/variable_mssql_contains_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql rename to tests/sql_test_files/data/database-specific/mssql/variable_mssql_contains.sql diff --git a/tests/sql_test_files/data/variable_unicode_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql b/tests/sql_test_files/data/database-specific/mssql/variable_unicode.sql similarity index 100% rename from tests/sql_test_files/data/variable_unicode_noduckdb_nogeneric_nomysql_nooracle_nopostgres_nosnowflake_nosqlite.sql rename to tests/sql_test_files/data/database-specific/mssql/variable_unicode.sql diff --git a/tests/sql_test_files/data/variable_limit_offset_noduckdb_nogeneric_nomssql_nooracle_nopostgres_nosnowflake_nosqlite.sql b/tests/sql_test_files/data/database-specific/mysql/variable_limit_offset.sql similarity index 100% rename from tests/sql_test_files/data/variable_limit_offset_noduckdb_nogeneric_nomssql_nooracle_nopostgres_nosnowflake_nosqlite.sql rename to tests/sql_test_files/data/database-specific/mysql/variable_limit_offset.sql diff --git a/tests/sql_test_files/data/set_multiple_rows_noduckdb_nogeneric_nomssql_nomysql_nooracle_nopostgres_nosnowflake.sql b/tests/sql_test_files/data/database-specific/sqlite/set_multiple_rows.sql similarity index 100% rename from tests/sql_test_files/data/set_multiple_rows_noduckdb_nogeneric_nomssql_nomysql_nooracle_nopostgres_nosnowflake.sql rename to tests/sql_test_files/data/database-specific/sqlite/set_multiple_rows.sql diff --git a/tests/sql_test_files/mod.rs b/tests/sql_test_files/mod.rs index 3335f967..915cc0cc 100644 --- a/tests/sql_test_files/mod.rs +++ b/tests/sql_test_files/mod.rs @@ -8,14 +8,32 @@ use tokio::task::JoinHandle; #[actix_web::test] async fn run_all_sql_test_files() { let app_data = crate::common::make_app_data().await; - let test_files = get_sql_test_cases(); + run_sql_test_cases(&app_data, get_sql_test_cases()).await; +} +/// Runs the SQL test files in `database-specific//`. +/// These files use syntax that only works on a single database engine, so they +/// cannot be part of the generic `run_all_sql_test_files` test. +#[actix_web::test] +async fn run_database_specific_sql_test_files() { + let app_data = crate::common::make_app_data().await; + let db_type = database_type_name(&app_data); + run_sql_test_cases(&app_data, get_database_specific_test_cases(&db_type)).await; +} + +async fn run_sql_test_cases( + app_data: &actix_web::web::Data, + test_files: Vec, +) { + if test_files.is_empty() { + return; + } let (shutdown_tx, shutdown_rx) = oneshot::channel(); let (echo_handle, port) = crate::common::start_echo_server(shutdown_rx); wait_for_echo_server(port).await; for test_file in test_files { - run_sql_test(&test_file, &app_data, &echo_handle, port).await; + run_sql_test(&test_file, app_data, &echo_handle, port).await; } let _ = shutdown_tx.send(()); @@ -63,9 +81,22 @@ fn get_sql_test_cases() -> Vec { tests } +fn get_database_specific_test_cases(db_type: &str) -> Vec { + read_sql_tests_in_dir( + &format!("tests/sql_test_files/data/database-specific/{db_type}"), + SqlTestFormat::Json, + ) +} + +fn database_type_name(app_data: &actix_web::web::Data) -> String { + format!("{:?}", app_data.db.info.database_type).to_lowercase() +} + fn read_sql_tests_in_dir(dir: &str, format: SqlTestFormat) -> Vec { - std::fs::read_dir(dir) - .unwrap() + let Ok(entries) = std::fs::read_dir(dir) else { + return Vec::new(); // no tests in this directory (e.g. no database-specific tests for this database) + }; + entries .filter_map(|e| { let path = e.ok()?.path(); if path.is_dir() || path.extension()? != "sql" { @@ -86,7 +117,7 @@ async fn run_sql_test( let test_file_path = test_file.to_string_lossy().replace('\\', "/"); let stem = test_file.file_stem().unwrap().to_str().unwrap(); - let db_type = format!("{:?}", app_data.db.info.database_type).to_lowercase(); + let db_type = database_type_name(app_data); if stem.contains(&format!("_no{db_type}")) { println!("Skipped {}: {}", test_file.display(), db_type); return; From b6e8793e8a8f7bb7d06984b1f8639e2c8933d9c0 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 23 Aug 2026 10:22:49 +0200 Subject: [PATCH 12/12] rename mssql variable fixture to match its sp_executesql repro 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. --- ...able_mssql_contains.sql => variable_mssql_sp_executesql.sql} | 2 ++ 1 file changed, 2 insertions(+) rename tests/sql_test_files/data/database-specific/mssql/{variable_mssql_contains.sql => variable_mssql_sp_executesql.sql} (53%) diff --git a/tests/sql_test_files/data/database-specific/mssql/variable_mssql_contains.sql b/tests/sql_test_files/data/database-specific/mssql/variable_mssql_sp_executesql.sql similarity index 53% rename from tests/sql_test_files/data/database-specific/mssql/variable_mssql_contains.sql rename to tests/sql_test_files/data/database-specific/mssql/variable_mssql_sp_executesql.sql index 1671698f..1efc37f2 100644 --- a/tests/sql_test_files/data/database-specific/mssql/variable_mssql_contains.sql +++ b/tests/sql_test_files/data/database-specific/mssql/variable_mssql_sp_executesql.sql @@ -1,3 +1,5 @@ -- https://github.com/sqlpage/SQLPage/issues/516 +-- sp_executesql is used instead of CONTAINS: both reject a CAST expression as an +-- argument, but CONTAINS needs a full-text index, which cannot be created on temp tables. SET x = 'It works !'; exec sp_executesql N'SELECT @p as actual, @exp as expected', N'@p varchar(100), @exp varchar(100)', @p=$x, @exp='It works !';