From c45473d6c12b2db3b61af9829c2ac4c1bd3d7522 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sat, 7 Nov 2026 16:49:19 +0000 Subject: [PATCH 1/2] fix: render SQL Server table functions as inline TVFs and list warehouse Recall relations portably SQL Server's render_create_function stripped a table function's SELECT and wrapped it in BEGIN RETURN (...) END, which the engine rejects. A TABLE return now renders RETURNS TABLE AS RETURN (). sqb recall schema --backend warehouse matched state relations with LIKE '\_sqlbuild\_%' ESCAPE '\', which ClickHouse cannot parse. A prefix comparison with SUBSTRING needs no escape in any dialect. --- .../src/bindings/_helpers/recall/command.rs | 6 ++-- .../src/bindings/_helpers/recall/constants.rs | 4 +++ .../sqlserver/classes/sqlserver_adapter.py | 13 ++++++-- src/sqlbuild/adapters/sqlserver/constants.py | 1 + .../sqlbuild/cli/recall/test_recall_cli.py | 6 ++++ .../adapters/sqlserver/_test_types.py | 3 ++ .../adapters/sqlserver/test_client.py | 31 ++++++++++++++++--- 7 files changed, 53 insertions(+), 11 deletions(-) diff --git a/crates/sqlbuild-python/src/bindings/_helpers/recall/command.rs b/crates/sqlbuild-python/src/bindings/_helpers/recall/command.rs index 5ba2193fc8..ed4516e2cb 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/recall/command.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/recall/command.rs @@ -33,6 +33,7 @@ use crate::bindings::_helpers::recall::constants::{ DEFAULT_MAX_CELL_BYTES, DEFAULT_QUERY_LIMIT, JSON_FORMAT, LAST_INVOCATION, RECALL_AMBIGUOUS_CODE, RECALL_DISABLED_CODE, RECALL_NOT_FOUND_CODE, RECALL_OPTIONS, RECALL_QUERY_CODE, RECALL_SETTINGS_CODE, RECALL_STORE_CODE, WAREHOUSE_BACKEND, + WAREHOUSE_STATE_RELATIONS_SQL, }; use crate::bindings::_helpers::recall::rendering::{QueryContext, render_result, render_schema}; use crate::bindings::_helpers::records::construct::new_error; @@ -402,10 +403,7 @@ fn run_schema(py: Python<'_>, recall: &RecallRequest) -> PyResult { ORDER BY table_schema, ordinal_position", relation.replace('\'', "''") ), - None => "SELECT table_schema, table_name, table_type FROM information_schema.tables \ - WHERE table_name LIKE '\\_sqlbuild\\_%' ESCAPE '\\' \ - ORDER BY table_schema, table_name" - .to_owned(), + None => WAREHOUSE_STATE_RELATIONS_SQL.to_owned(), }; return warehouse_query(py, recall, &project_dir, (&sql, None)); } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/recall/constants.rs b/crates/sqlbuild-python/src/bindings/_helpers/recall/constants.rs index 2a1af6996c..2a10b7918f 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/recall/constants.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/recall/constants.rs @@ -26,6 +26,10 @@ pub(crate) const RECALL_AMBIGUOUS_CODE: &str = "C973"; pub(crate) const RECALL_QUERY_CODE: &str = "C974"; pub(crate) const RECALL_SETTINGS_CODE: &str = "C975"; pub(crate) const WAREHOUSE_BACKEND: &str = "warehouse"; +/// SQLBuild's state relations in the warehouse; a prefix comparison needs no `LIKE` escape, whose syntax differs by dialect. +pub(crate) const WAREHOUSE_STATE_RELATIONS_SQL: &str = "SELECT table_schema, table_name, table_type \ + FROM information_schema.tables WHERE SUBSTRING(table_name, 1, 10) = '_sqlbuild_' \ + ORDER BY table_schema, table_name"; /// Default cell width in text formats, in bytes. pub(crate) const DEFAULT_MAX_CELL_BYTES: usize = 4096; pub(crate) const JSON_FORMAT: &str = "json"; diff --git a/src/sqlbuild/adapters/sqlserver/classes/sqlserver_adapter.py b/src/sqlbuild/adapters/sqlserver/classes/sqlserver_adapter.py index 2e50a2e916..ee514dd37e 100644 --- a/src/sqlbuild/adapters/sqlserver/classes/sqlserver_adapter.py +++ b/src/sqlbuild/adapters/sqlserver/classes/sqlserver_adapter.py @@ -116,6 +116,7 @@ INFORMATION_SCHEMA_NULLABLE_VALUE, INTEGER_TYPE_TOKEN, ISO8601_CONVERT_STYLE, + TABLE_FUNCTION_RETURN_TYPE, USER_TABLE_OBJECT_TYPE, ) from sqlbuild.compiler.compile.types import FunctionLanguage @@ -831,11 +832,17 @@ def render_create_function( message="Python functions require an engine-specific implementation" ) arg_sql: str = ", ".join(f"@{arg.name} {arg.type}" for arg in arguments) - body_expression: str = body_sql.strip().removeprefix("SELECT").strip().rstrip(";") + body_query: str = body_sql.strip().rstrip(";") for arg in arguments: - body_expression = re.sub( - rf"\b{re.escape(str(arg.name))}\b", f"@{arg.name}", body_expression + body_query = re.sub(rf"\b{re.escape(str(arg.name))}\b", f"@{arg.name}", body_query) + if returns.upper() == TABLE_FUNCTION_RETURN_TYPE: + return ( + f"CREATE OR ALTER FUNCTION {destination}({arg_sql})\n" + "RETURNS TABLE\n" + "AS\n" + f"RETURN (\n{body_query}\n)", ) + body_expression: str = body_query.removeprefix("SELECT").strip() if returns.upper() == BOOLEAN_RETURN_TYPE and not body_expression.upper().startswith( ("CASE ", "CAST(") ): diff --git a/src/sqlbuild/adapters/sqlserver/constants.py b/src/sqlbuild/adapters/sqlserver/constants.py index 611c496afc..bb206e1ed9 100644 --- a/src/sqlbuild/adapters/sqlserver/constants.py +++ b/src/sqlbuild/adapters/sqlserver/constants.py @@ -1,6 +1,7 @@ """SQL Server adapter constants.""" BOOLEAN_RETURN_TYPE: str = "BIT" +TABLE_FUNCTION_RETURN_TYPE: str = "TABLE" DIFF_CHARACTER_LENGTH_TYPES: frozenset[str] = frozenset( {"binary", "char", "nchar", "nvarchar", "varbinary", "varchar"} ) diff --git a/tests/integration/src/sqlbuild/cli/recall/test_recall_cli.py b/tests/integration/src/sqlbuild/cli/recall/test_recall_cli.py index d2def05b76..de785bec9c 100644 --- a/tests/integration/src/sqlbuild/cli/recall/test_recall_cli.py +++ b/tests/integration/src/sqlbuild/cli/recall/test_recall_cli.py @@ -149,6 +149,12 @@ def test_given_a_removed_format_when_querying_then_it_is_a_usage_error_naming_th expected_fragments=("│ order_id │ amount │", "INTEGER", "DECIMAL(10,2)"), unexpected_fragments=("RECORD",), ), + QueryLayoutTestCase( + description="warehouse schema lists SQLBuild's state relations only", + args=("recall", "schema", "--backend", "warehouse"), + expected_fragments=("_sqlbuild_fingerprints", "BASE TABLE"), + unexpected_fragments=("│ orders ",), + ), QueryLayoutTestCase( description="a limited result says how to see more", args=("query", "SELECT range AS order_id FROM range(30)"), diff --git a/tests/unit/src/sqlbuild/adapters/sqlserver/_test_types.py b/tests/unit/src/sqlbuild/adapters/sqlserver/_test_types.py index d4b2389d6f..2d3cd3b2b0 100644 --- a/tests/unit/src/sqlbuild/adapters/sqlserver/_test_types.py +++ b/tests/unit/src/sqlbuild/adapters/sqlserver/_test_types.py @@ -41,6 +41,9 @@ class SqlServerRenderCreateTableAsTestCase: @dataclass(frozen=True) class SqlServerRenderCreateFunctionTestCase: description: str + arguments: tuple[tuple[str, str], ...] + returns: str + body_sql: str expected_statements: tuple[str, ...] diff --git a/tests/unit/src/sqlbuild/adapters/sqlserver/test_client.py b/tests/unit/src/sqlbuild/adapters/sqlserver/test_client.py index a192084a60..ec4b59a935 100644 --- a/tests/unit/src/sqlbuild/adapters/sqlserver/test_client.py +++ b/tests/unit/src/sqlbuild/adapters/sqlserver/test_client.py @@ -419,6 +419,9 @@ def test_given_table_target_when_rendering_create_then_sqlserver_uses_select_int [ SqlServerRenderCreateFunctionTestCase( description="renders scalar SQL function with T-SQL body", + arguments=(("order_status", "NVARCHAR(MAX)"),), + returns="BIT", + body_sql="SELECT order_status = 'completed'", expected_statements=( "CREATE OR ALTER FUNCTION dbo.is_completed_order(@order_status NVARCHAR(MAX))\n" "RETURNS BIT\n" @@ -427,7 +430,25 @@ def test_given_table_target_when_rendering_create_then_sqlserver_uses_select_int " RETURN (CASE WHEN @order_status = 'completed' THEN 1 ELSE 0 END)\n" "END", ), - ) + ), + SqlServerRenderCreateFunctionTestCase( + description="renders a table function as an inline TVF that keeps its SELECT", + arguments=(("p_floor", "FLOAT"),), + returns="table", + body_sql=( + "SELECT id, CASE WHEN amount > p_floor THEN 'high' ELSE 'low' END AS band " + "FROM dbo.orders;" + ), + expected_statements=( + "CREATE OR ALTER FUNCTION dbo.is_completed_order(@p_floor FLOAT)\n" + "RETURNS TABLE\n" + "AS\n" + "RETURN (\n" + "SELECT id, CASE WHEN amount > @p_floor THEN 'high' ELSE 'low' END AS band " + "FROM dbo.orders\n" + ")", + ), + ), ], ids=lambda case: case.description, ) @@ -438,9 +459,11 @@ def test_given_sql_function_when_rendering_create_then_sqlserver_declares_functi statements: tuple[str, ...] = adapter.render_create_function( destination="dbo.is_completed_order", - arguments=(FunctionArgument(name="order_status", type="NVARCHAR(MAX)"),), - returns="BIT", - body_sql="SELECT order_status = 'completed'", + arguments=tuple( + FunctionArgument(name=name, type=type_name) for name, type_name in test_case.arguments + ), + returns=test_case.returns, + body_sql=test_case.body_sql, ) assert statements == test_case.expected_statements From 15b863e92ce686daa2aca7c3a51c94f9886a9707 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sat, 7 Nov 2026 17:15:35 +0000 Subject: [PATCH 2/2] fix(sqlserver): quote schema names in EXEC, end MERGE with a semicolon and refuse WITH source expressions render_create_schema doubled quotes only in the sys.schemas lookup, so a schema name with a quote broke the EXEC string. render_merge now ends with the semicolon SQL Server requires. A source expression starting with WITH cannot sit in a derived table on SQL Server, so it now fails with A715 and a rewrite hint instead of a warehouse syntax error. --- .../sqlserver/classes/sqlserver_adapter.py | 15 ++++- .../adapters/sqlserver/_test_types.py | 8 +++ .../adapters/sqlserver/test_client.py | 55 ++++++++++++++++++- 3 files changed, 73 insertions(+), 5 deletions(-) diff --git a/src/sqlbuild/adapters/sqlserver/classes/sqlserver_adapter.py b/src/sqlbuild/adapters/sqlserver/classes/sqlserver_adapter.py index ee514dd37e..36a8d42f3f 100644 --- a/src/sqlbuild/adapters/sqlserver/classes/sqlserver_adapter.py +++ b/src/sqlbuild/adapters/sqlserver/classes/sqlserver_adapter.py @@ -320,9 +320,10 @@ def query(self, *, connection: Any, sql: str, limit: int | None) -> QueryResult: def render_create_schema(self, *, database: str | None, schema: str) -> tuple[str, ...]: del database escaped_schema: str = schema.replace("'", "''") + escaped_identifier: str = self.render_identifier(schema).replace("'", "''") return ( "IF NOT EXISTS (SELECT 1 FROM sys.schemas WHERE name = " - f"'{escaped_schema}') EXEC('CREATE SCHEMA {self.render_identifier(schema)}')", + f"'{escaped_schema}') EXEC('CREATE SCHEMA {escaped_identifier}')", ) def render_drop_schema(self, *, database: str | None, schema: str) -> tuple[str, ...]: @@ -2406,7 +2407,7 @@ def render_merge( ) if update_assignments: merge_sql += f"WHEN MATCHED THEN UPDATE SET {update_assignments} " - merge_sql += f"WHEN NOT MATCHED THEN INSERT ({insert_columns}) VALUES ({insert_values})" + merge_sql += f"WHEN NOT MATCHED THEN INSERT ({insert_columns}) VALUES ({insert_values});" return (merge_sql,) def render_qualified_name( @@ -2456,7 +2457,15 @@ def render_source_expression_relation(self, *, expression: str) -> str: if stripped_expression.startswith("("): return stripped_expression lowered: str = stripped_expression.lower() - if lowered.startswith(("select", "with", "values")): + if re.match(r"with\b", lowered): + raise AdapterUserError( + message="SQL Server cannot read a source expression that starts with WITH: " + "a common table expression cannot sit inside a derived table", + code="A715", + help="rewrite the source expression as one SELECT, using subqueries instead of " + "WITH, or define it as a view or model and read that", + ) + if lowered.startswith(("select", "values")): return f"({stripped_expression})" return stripped_expression diff --git a/tests/unit/src/sqlbuild/adapters/sqlserver/_test_types.py b/tests/unit/src/sqlbuild/adapters/sqlserver/_test_types.py index 2d3cd3b2b0..3992e76652 100644 --- a/tests/unit/src/sqlbuild/adapters/sqlserver/_test_types.py +++ b/tests/unit/src/sqlbuild/adapters/sqlserver/_test_types.py @@ -167,3 +167,11 @@ class SqlServerMigrationEventReadTestCase: description: str stored_created_at: object expected_read_suffix: str + + +@dataclass(frozen=True) +class SqlServerSourceExpressionRelationTestCase: + description: str + expression: str + expected_relation: str | None + expected_error_code: str | None diff --git a/tests/unit/src/sqlbuild/adapters/sqlserver/test_client.py b/tests/unit/src/sqlbuild/adapters/sqlserver/test_client.py index ec4b59a935..42ba596d2d 100644 --- a/tests/unit/src/sqlbuild/adapters/sqlserver/test_client.py +++ b/tests/unit/src/sqlbuild/adapters/sqlserver/test_client.py @@ -7,6 +7,7 @@ from sqlbuild._native import FunctionArgument from sqlbuild.adapter.contract.classes.statement_recorder import StatementRecorder +from sqlbuild.adapter.contract.exceptions import AdapterUserError from sqlbuild.adapter.contract.models import ( ColumnInfo, CursorValue, @@ -33,6 +34,7 @@ SqlServerRenderRenameTestCase, SqlServerRollbackTestCase, SqlServerRowDiffEqualitySqlTestCase, + SqlServerSourceExpressionRelationTestCase, ) from tests.unit.src.sqlbuild.adapters.sqlserver.helpers import FakeSqlServerConnection @@ -185,7 +187,7 @@ def test_given_cursor_type_when_rendering_delete_insert_then_sqlserver_uses_type expected_insert_clause=( "([tenant_id], [order_id], [updated_at], [status]) VALUES " "(__source.[tenant_id], __source.[order_id], __source.[updated_at], " - "__source.[status])" + "__source.[status]);" ), ) ], @@ -479,7 +481,15 @@ def test_given_sql_function_when_rendering_create_then_sqlserver_declares_functi "IF NOT EXISTS (SELECT 1 FROM sys.schemas WHERE name = 'analytics') " "EXEC('CREATE SCHEMA [analytics]')" ), - ) + ), + SqlServerRenderCreateSchemaTestCase( + description="doubles quotes in both the lookup and the EXEC string", + schema="sqb'meta]x", + expected_statement=( + "IF NOT EXISTS (SELECT 1 FROM sys.schemas WHERE name = 'sqb''meta]x') " + "EXEC('CREATE SCHEMA [sqb''meta]]x]')" + ), + ), ], ids=lambda case: case.description, ) @@ -876,3 +886,44 @@ def test_given_cross_schema_table_move_when_moving_then_sqlserver_uses_native_tr assert tuple(event.content for event in statement_recorder.snapshot()) == ( test_case.expected_statements ) + + +@pytest.mark.parametrize( + "test_case", + [ + SqlServerSourceExpressionRelationTestCase( + description="a SELECT becomes a derived table", + expression="SELECT id FROM raw.orders;", + expected_relation="(SELECT id FROM raw.orders)", + expected_error_code=None, + ), + SqlServerSourceExpressionRelationTestCase( + description="a relation whose name starts with with stays a relation", + expression="raw.with_orders", + expected_relation="raw.with_orders", + expected_error_code=None, + ), + SqlServerSourceExpressionRelationTestCase( + description="a WITH query is refused because SQL Server cannot nest it", + expression="WITH x AS (SELECT 1 AS id) SELECT id FROM x", + expected_relation=None, + expected_error_code="A715", + ), + ], + ids=lambda case: case.description, +) +def test_given_source_expression_when_rendering_relation_then_sqlserver_nests_only_what_it_can( + test_case: SqlServerSourceExpressionRelationTestCase, +) -> None: + adapter: SqlServerAdapter = SqlServerAdapter() + + try: + relation: str | None = adapter.render_source_expression_relation( + expression=test_case.expression + ) + error_code: str | None = None + except AdapterUserError as error: + relation = None + error_code = error.code + + assert (relation, error_code) == (test_case.expected_relation, test_case.expected_error_code)