Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## fix/mssql-case-sensitive-collation #2676 +/- ##
===================================================================
Coverage 70.90% 70.90%
===================================================================
Files 109 109
Lines 9979 9979
Branches 1088 1088
===================================================================
Hits 7076 7076
Misses 2692 2692
Partials 211 211 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
❌ 194/197 passed, 13 flaky, 3 failed, 2 skipped, 3h14m35s total ❌ test_redshift_connector_execute_query: redshift_connector.error.OperationalError: ('connection time out', TimeoutError(110, 'Connection timed out')) (2m15.691s)❌ test_redshift_connector_connection: redshift_connector.error.OperationalError: ('connection time out', TimeoutError(110, 'Connection timed out')) (2m14.97s)❌ test_redshift_connection_check: redshift_connector.error.OperationalError: ('connection time out', TimeoutError(110, 'Connection timed out')) (2m14.853s)Flaky tests:
Running from acceptance #5497 |
…ks under case-sensitive collations Under a case-sensitive database collation SQL Server matches schema and object names, and the column names of our own CTEs and aliases, exactly as written. The profiler queried the INFORMATION_SCHEMA views as information_schema.tables etc., and cpu_utilization declared CTE aliases (SystemIdle, SQLProcessUtilization, EventTime) but referenced them in lowercase. On a case-sensitive database the tables, columns, views, routines and cpu_utilization steps failed and took the whole run down. Write INFORMATION_SCHEMA view references in uppercase in the mssql single_db and multi_db queries, the legacy_synapse queries, and the one lowercase view name in the Synapse routines query, and reference the cpu_utilization aliases as declared. The INFORMATION_SCHEMA column references are uppercased too, to match their documented case; SQL Server resolves system-view column names case-insensitively, so this is for consistency rather than required. sys.* catalog views are defined in lowercase and are left untouched. Results are written positionally and the DDLs already declare uppercase columns, so the DuckDB tables are unchanged. Co-authored-by: Isaac <no-reply@databricks.com>
…e default The multi_db queries UNION ALL catalog metadata from every database on the instance. Name columns carry each source database's collation, so an instance whose databases use different collations (even just SQL_Latin1_General_CP1_CI_AI vs SQL_Latin1_General_CP1_CI_AS) failed the tables, views, columns, indexed_views and routines steps with "Cannot resolve collation conflict ... in UNION ALL operator". Add COLLATE DATABASE_DEFAULT to every text column in each UNION branch so the combined result takes the collation of the database the query runs in. Numeric and datetime columns are left as they are, since COLLATE is not valid on them. Co-authored-by: Isaac <no-reply@databricks.com>
The multi_db queries tag each row with its database name by splicing QUOTENAME([name], '''') into the dynamic SQL as a plain '...' literal. Without the N prefix that literal is varchar in the server's code page, so characters outside it were replaced: a database named db_ünï_日本 came back as db_ünï_??. Prefix the literal with N so the name stays Unicode. Co-authored-by: Isaac <no-reply@databricks.com>
…columns sys.dm_os_sys_info gained columns over SQL Server versions; process_physical_affinity, for one, only exists from 2017, so on 2016 the whole sys_info query failed to compile with "Invalid column name" (#2667). Give each column added in later versions a NULL default in an outer query and CROSS APPLY the DMV: an unqualified reference binds to the DMV column where it exists and falls back to the default on older versions. Same columns, same order, same DDL. Co-authored-by: Isaac <no-reply@databricks.com>
AFFINITY_TYPE and SOFTNUMA_CONFIGURATION were declared SMALLINT although sys.dm_os_sys_info returns them as int. Declare them INTEGER so the DuckDB table can hold every value the DMV can return. Co-authored-by: Isaac <no-reply@databricks.com>
State that the SQL Server profiler supports SQL Server 2012 and later (plus Azure SQL Database and Managed Instance): sessions.database_id and the SHA2_256 hashing it relies on start in 2012. Note that on versions before 2017 a few sys_info fields introduced later are recorded as empty, and that case-sensitive and mixed database collations are supported. Co-authored-by: Isaac <no-reply@databricks.com>
Several Synapse queries referenced catalog views and DMVs in uppercase (SYS.DATABASES, SYS.INDEXES, SYS.DM_PDW_EXEC_REQUESTS, ...). sys.* objects are defined in lowercase, and under a case-sensitive database collation SQL Server matches schema and object names exactly, so these would fail on a case-sensitive pool. Lowercase the schema and object names; column names are matched case-insensitively and stay as they are. Co-authored-by: Isaac <no-reply@databricks.com>
…SQL Server queries Alias the database column as DATABASE_NAME in every SQL Server query, matching the DDLs (the multi_db queries, and the db_sizes, table_sizes and indexed_views queries, still used database_name). In multi_db db_sizes and table_sizes the table-variable column and the final SELECT are renamed together, since a case-sensitive server matches them exactly. Write the [REDACTED] placeholders for VIEW_DEFINITION and ROUTINE_DEFINITION as N'...' literals, and collate them to the database default in the multi_db UNIONs, so their type and collation match the catalog text around them. Results are written positionally, so the DuckDB tables are unchanged. Co-authored-by: Isaac <no-reply@databricks.com>
Guard each fix with a static check over the query files, so a regression fails in the unit tests instead of on a customer's server: * system schema/object names in their defined case (mssql, legacy_synapse, Synapse queries); * our own CTE/alias names referenced in their declared case; * COLLATE DATABASE_DEFAULT on every text column of a multi_db UNION, and on no other type (which column is text comes from the step's DDL); * N'' prefix on multi_db database-name and text literals; * sys_info: every version-dependent column has a NULL default, is referenced unqualified, and the output columns match sys_info_ddl.sql. Against main these tests fail 21 times across every one of those rules. Co-authored-by: Isaac <no-reply@databricks.com>
Replace the parsing-heavy checks (sqlglot alias analysis, @cols splitting, per-query regexes) with four plain text checks that are robust to reformatting: * system schema/object names are written in their defined case (SQL Server, legacy_synapse and Synapse queries; docstrings ignored); * database names spliced into multi_db SQL are N'' literals; * multi_db queries that UNION across databases use COLLATE DATABASE_DEFAULT; * sys_info has a NULL default for each column added in SQL Server 2016 or later. The integration tests prove the behaviour by running the queries; these only catch an obvious regression early. Against main they still fail for every rule. Co-authored-by: Isaac <no-reply@databricks.com>
m-abulazm
force-pushed
the
fix/mssql-case-sensitive-collation
branch
from
October 13, 2026 08:48
670548a to
e75940d
Compare
… the sandbox database Run the real single_db profiler pipeline against the sandbox SQL Server (sandbox_sqlserver_config, an Azure SQL Database in the acceptance job) and check every step's output, not just that it ran: * every step finishes COMPLETE, and each query's live result columns match its DDL by name and order (the pipeline writes by position, so a mismatch would otherwise load silently); * the test creates its own objects in a uniquely named schema with non-ASCII names (a table with three rows, a view, an indexed view with its index, a procedure run twice) and asserts the exact rows tables, views, columns, indexed_views, routines, table_sizes and proc_stats report for them; * view and routine definitions come back redacted and session login names hashed, for every row; * sys_info, databases, db_sizes, query_stats, sessions and cpu_utilization values are sane. The objects are dropped afterwards, since the sandbox database is shared. Co-authored-by: Isaac <no-reply@databricks.com>
m-abulazm
force-pushed
the
test/mssql-azure-sql-database
branch
from
October 13, 2026 08:48
c0d94cb to
12e3e49
Compare
This branch has not been deployed
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
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
An integration test that runs the real SQL Server profiler (
single_dbpipeline) against the database from thesandbox_sqlserver_configfixture and checks every step's output, not just that it ran. It exercises the collation and version fixes merged in #2675.How it asserts
The test creates its own objects in a uniquely named schema with non-ASCII names — a table with three rows, a view, an indexed view with its index, and a procedure run twice — profiles the database, then checks each extracted table against those known objects. The objects are dropped afterwards; the sandbox database is shared.
COMPLETE; each query's live result columns match its DDL by name and ordernvarchar(20)length,decimal(10,2)precision[REDACTED]Testing
tablesquery each made exactly the intended test fail.This pull request and its description were written by Isaac.