fix: one identifier quoting style, and the unquoted identifiers underneath it (#785, #409)

CHANGELOG.md

@@ -10,6 +10,16 @@
 - **Behavior change:** `dbt_sqlserver_use_native_string_types` now defaults to `True`: `STRING` maps to `VARCHAR(MAX)`, `NCHAR` to `NCHAR(1)`, and `NVARCHAR` to `NVARCHAR(4000)`, instead of the legacy `VARCHAR(8000)`/`CHAR(1)` mappings. The `False` (legacy) behavior is deprecated and will be removed in a future release.
 - Add an experimental `adbc` backend (`backend: adbc`), an alternative to `pyodbc`/`mssql-python` built on [ADBC](https://arrow.apache.org/adbc/) that talks to SQL Server via `go-mssqldb` instead of an ODBC/DB-API bridge. Install with `dbt-sqlserver[adbc]` plus the separate `dbc` CLI-installed driver binary; supports SQL Server (user/password) authentication only for now. See [docs/adbc_backend.md](docs/adbc_backend.md). [#771](https://github.com/dbt-msft/dbt-sqlserver/issues/771)
 
+#### Bugfixes
+
+- Fix models failing with `Incorrect syntax near '\'` when the schema name needs delimiters, such as a domain-qualified `domain\user`. The clustered columnstore index name embeds the schema and was emitted as a bare identifier, so the generated DDL did not parse. [#409](https://github.com/dbt-msft/dbt-sqlserver/issues/409)
+- Fix identifiers built inside string literals not being quoted, which broke schema names containing a `.` or a `"`. `OBJECT_ID('schema.table')` returns `NULL` rather than erroring for such a name, so the failures were silent: the drop-before-create guards in `create_table_as` treated an existing table as absent (then hit `Msg 2714`), and the mask introspection in `apply_masks` found no columns, so configured masks were never applied. `sp_rename` was affected too, failing the table rename-swap with `No item by the name of ...`. All now pass quoted, qualified names. [#785](https://github.com/dbt-msft/dbt-sqlserver/issues/785)
+
+#### Under the hood
+
+- **Behavior change (generated SQL only):** the few macros that hand-formatted `[bracket]` identifiers now use `adapter.quote()`, matching the `"double quoted"` identifiers `{{ relation }}` already rendered, so one statement no longer mixes both styles. Affects `USE` (now via the existing `get_use_database_sql()` helper), `CREATE SCHEMA`, contract column lists, grantees, index/constraint names and generated test view names. Server-side `QUOTENAME()` keeps brackets by design. Visible only if you parse dbt's generated SQL or have custom macros assuming brackets. [#785](https://github.com/dbt-msft/dbt-sqlserver/issues/785)
+- Identifier quoting escapes an embedded `"` by doubling it (`ab"cd` → `"ab""cd"`), in both `adapter.quote()` and relation rendering (`SQLServerRelation.quoted`). Needed twice over: a `"` requires no escaping inside `[brackets]` but does inside double quotes, so the delimiter change above would otherwise have rejected names the adapter previously accepted; and the string-literal fix above renders identifiers through the relation, so a schema containing a `"` depends on it. [#785](https://github.com/dbt-msft/dbt-sqlserver/issues/785)
+
 ### v1.11.0
 
 #### Features

dbt/adapters/sqlserver/sqlserver_adapter.py

@@ -188,6 +188,22 @@ def get_column_schema_from_query(self, sql: str) -> List[BaseColumn]:
         ]
         return columns
 
+    @classmethod
+    def quote(cls, identifier: str) -> str:
+        """Double-quote an identifier, doubling any embedded double quote.
+
+        ``SQLAdapter.quote`` interpolates the identifier verbatim, so a name
+        containing a ``"`` would close the quoted identifier early and the
+        remainder would parse as SQL. T-SQL escapes a delimiter by doubling
+        it -- the same rule ``QUOTENAME()`` applies to brackets -- so
+        ``ab"cd`` must render as ``"ab""cd"``.
+
+        This is the quoting used by every macro that formats an identifier
+        (see #785). Relation rendering escapes nothing, since it goes through
+        ``BaseRelation.quote_character`` upstream rather than this method.
+        """
+        return '"{}"'.format(str(identifier).replace('"', '""'))
+
     @classmethod
     def convert_boolean_type(cls, agate_table, col_idx):
         return "bit"

dbt/adapters/sqlserver/sqlserver_relation.py

@@ -26,6 +26,15 @@ class SQLServerRelation(BaseRelation):
     def get_relation_type(cls) -> Type[SQLServerRelationType]:
         return SQLServerRelationType
 
+    def quoted(self, identifier):
+        """Quote a path part, doubling any embedded quote character (#785).
+
+        Not delegated to super(): that wraps verbatim, so pre-escaping the
+        input would double-escape if upstream ever starts escaping too.
+        """
+        quote_char = self.quote_character
+        return f"{quote_char}{str(identifier).replace(quote_char, quote_char * 2)}{quote_char}"
+
     def _render_limited_alias(self) -> str:
         if self.disable_empty_relation_aliases:
             return ""

dbt/include/sqlserver/macros/adapters/apply_grants.sql

@@ -48,7 +48,7 @@
 {%- macro sqlserver__get_grant_sql(relation, privilege, grantees) -%}
     {%- set grantees_safe = [] -%}
     {%- for grantee in grantees -%}
-        {%- set grantee_safe = "[" ~ grantee ~ "]" -%}
+        {%- set grantee_safe = adapter.quote(grantee) -%}
         {%- do grantees_safe.append(grantee_safe) -%}
     {%- endfor -%}
     grant {{ privilege }} on {{ relation }} to {{ grantees_safe | join(', ') }}
@@ -57,7 +57,7 @@
 {%- macro sqlserver__get_revoke_sql(relation, privilege, grantees) -%}
     {%- set grantees_safe = [] -%}
     {%- for grantee in grantees -%}
-        {%- set grantee_safe = "[" ~ grantee ~ "]" -%}
+        {%- set grantee_safe = adapter.quote(grantee) -%}
         {%- do grantees_safe.append(grantee_safe) -%}
     {%- endfor -%}
     revoke {{ privilege }} on {{ relation }} from {{ grantees_safe | join(', ') }}
@@ -66,6 +66,6 @@
 {% macro get_provision_sql(relation, privilege, grantees) %}
     {% for grantee in grantees %}
         if not exists(select name from sys.database_principals where name = '{{ grantee }}')
-        create user [{{ grantee }}] from external provider;
+        create user {{ adapter.quote(grantee) }} from external provider;
     {% endfor %}
 {% endmacro %}

dbt/include/sqlserver/macros/adapters/apply_masks.sql

@@ -33,7 +33,7 @@
         c.name as name,
         c.masking_function as masking_function
     from sys.masked_columns c {{ information_schema_hints() }}
-    where c.object_id = OBJECT_ID('{{ relation.schema }}.{{ relation.identifier }}')
+    where c.object_id = OBJECT_ID('{{ escape_single_quotes(relation.include(database=False)) }}')
 {% endmacro %}
 
 
@@ -47,7 +47,7 @@
         from sys.index_columns ic {{ information_schema_hints() }}
         inner join sys.columns col {{ information_schema_hints() }}
             on col.object_id = ic.object_id and col.column_id = ic.column_id
-        where ic.object_id = OBJECT_ID('{{ relation.schema }}.{{ relation.identifier }}')
+        where ic.object_id = OBJECT_ID('{{ escape_single_quotes(relation.include(database=False)) }}')
           and ic.is_included_column = 0
     {% endcall %}
     {% set result = [] %}
@@ -64,7 +64,7 @@
     {% call statement('get_unmaskable_columns', fetch_result=True) %}
         select col.name as name
         from sys.columns col {{ information_schema_hints() }}
-        where col.object_id = OBJECT_ID('{{ relation.schema }}.{{ relation.identifier }}')
+        where col.object_id = OBJECT_ID('{{ escape_single_quotes(relation.include(database=False)) }}')
           and (
                 col.is_computed = 1
              or col.is_filestream = 1

dbt/include/sqlserver/macros/adapters/columns.sql

@@ -62,7 +62,7 @@
             drop column "{{ column_name }}";
         {%- endset %}
         {% set rename_column %}
-            exec sp_rename '{{ relation | replace('"', '') }}.{{ tmp_column }}', '{{ column_name }}', 'column'
+            exec sp_rename '{{ escape_single_quotes(relation.include(database=False)) }}.{{ escape_single_quotes(adapter.quote(tmp_column)) }}', '{{ escape_single_quotes(column_name) }}', 'column'
         {%- endset %}
 
         {% do run_query(add_column) %}

dbt/include/sqlserver/macros/adapters/indexes.sql

@@ -1,17 +1,28 @@
+{#- Quoted, comma-separated column list for index DDL. -#}
+{% macro quote_column_list(columns) -%}
+    {%- set quoted = [] -%}
+    {%- for column in columns -%}
+        {%- do quoted.append(adapter.quote(column)) -%}
+    {%- endfor -%}
+    {{- quoted | join(', ') -}}
+{%- endmacro %}
+
+
 {% macro sqlserver__create_clustered_columnstore_index(relation) -%}
+    {#- cci_name embeds the schema, so it must be quoted as an identifier
+        (raw only in the string comparison below) -- issue #409 -#}
     {%- set cci_name = (relation.schema ~ '_' ~ relation.identifier ~ '_cci') | replace(".", "") | replace(" ", "") -%}
     {%- set relation_name = relation.include(database=False) -%}
-    {%- set full_relation = '"' ~ relation.schema ~ '"."' ~ relation.identifier ~ '"' -%}
-    use [{{ relation.database }}];
+    {{ get_use_database_sql(relation.database) }}
     if EXISTS (
         SELECT *
         FROM sys.indexes {{ information_schema_hints() }}
-        WHERE name = '{{cci_name}}'
-        AND object_id=object_id('{{relation_name}}')
+        WHERE name = '{{ escape_single_quotes(cci_name) }}'
+        AND object_id=object_id('{{ escape_single_quotes(relation_name) }}')
     )
-    DROP index {{full_relation}}.{{cci_name}}
-    CREATE CLUSTERED COLUMNSTORE INDEX {{cci_name}}
-    ON {{full_relation}}
+    DROP index {{ relation_name }}.{{ adapter.quote(cci_name) }}
+    CREATE CLUSTERED COLUMNSTORE INDEX {{ adapter.quote(cci_name) }}
+    ON {{ relation_name }}
 {% endmacro %}
 
 {% macro drop_xml_indexes() -%}
@@ -152,7 +163,7 @@
     {% endif %}
     clustered index
         {{ idx_name }}
-        on {{ this }} ({{ '[' + columns|join("], [") + ']' }})
+        on {{ this }} ({{ quote_column_list(columns) }})
     end
 {%- endmacro %}
 
@@ -180,17 +191,17 @@
     begin
     create nonclustered index
         {{ idx_name }}
-        on {{ this }} ({{ '[' + columns|join("], [") + ']' }})
+        on {{ this }} ({{ quote_column_list(columns) }})
         {% if includes -%}
-            include ({{ '[' + includes|join("], [") + ']' }})
+            include ({{ quote_column_list(includes) }})
         {% endif %}
     end
 {% endmacro %}
 
 
 {% macro drop_fk_indexes_on_table(relation) -%}
   {% call statement('find_references', fetch_result=true) %}
-      USE [{{ relation.database }}];
+      {{ get_use_database_sql(relation.database) }}
       SELECT  obj.name AS FK_NAME,
       sch.name AS [schema_name],
       tab1.name AS [table],
@@ -215,7 +226,7 @@
       {% set references = load_result('find_references')['data'] %}
       {% for reference in references -%}
         {% call statement('main') -%}
-           alter table [{{reference[1]}}].[{{reference[2]}}] drop constraint [{{reference[0]}}]
+           alter table {{ adapter.quote(reference[1]) }}.{{ adapter.quote(reference[2]) }} drop constraint {{ adapter.quote(reference[0]) }}
         {%- endcall %}
       {% endfor %}
 {% endmacro %}
@@ -229,7 +240,7 @@
     FROM sys.indexes AS i {{ information_schema_hints() }}
     INNER JOIN sys.index_columns AS ic {{ information_schema_hints() }}
         ON i.object_id = ic.object_id AND i.index_id = ic.index_id and i.type <> 5
-    WHERE i.object_id = OBJECT_ID('{{ relation.schema }}.{{ relation.identifier }}')
+    WHERE i.object_id = OBJECT_ID('{{ escape_single_quotes(relation.include(database=False)) }}')
 
     UNION ALL
 
@@ -283,21 +294,21 @@
                   and object_id = OBJECT_ID('{{ relation }}')
   )
   begin
-  {# key columns: bracket-quoted (with ]] escaping) plus per-column direction #}
+  {# key columns: quoted (adapter.quote escapes embedded quotes) plus per-column direction #}
   {%- set key_columns = [] -%}
   {%- for column in index_config.columns -%}
     {%- do key_columns.append(
-        '[' ~ column | replace(']', ']]') ~ ']'
+        adapter.quote(column)
         ~ (' desc' if column in index_config.descending_columns else '')
     ) -%}
   {%- endfor -%}
   {%- set include_columns = [] -%}
   {%- for column in index_config.included_columns -%}
-    {%- do include_columns.append('[' ~ column | replace(']', ']]') ~ ']') -%}
+    {%- do include_columns.append(adapter.quote(column)) -%}
   {%- endfor %}
   create
   {% if index_config.unique -%} unique {% endif %}{{ index_config.type }}
-  index [{{ index_name }}]
+  index {{ adapter.quote(index_name) }}
   on {{ relation }}
   ({{ key_columns | join(', ') }})
     {% if include_columns -%}
@@ -436,7 +447,7 @@
         from sys.partitions p {{ information_schema_hints() }}
         where p.object_id = i.object_id and p.index_id = i.index_id
     ) part
-    where i.object_id = OBJECT_ID('{{ relation.schema }}.{{ relation.identifier }}')
+    where i.object_id = OBJECT_ID('{{ escape_single_quotes(relation.include(database=False)) }}')
       and i.index_id > 0
       and i.[type] not in (3, 4, 7)  /* xml, spatial, memory-optimized hash */
   {%- endcall %}
@@ -445,7 +456,7 @@
 
 
 {% macro sqlserver__get_drop_index_sql(relation, index_name) -%}
-    drop index [{{ index_name }}] on {{ relation }}
+    drop index {{ adapter.quote(index_name) }} on {{ relation }}
 {%- endmacro %}
 
 

dbt/include/sqlserver/macros/adapters/metadata.sql

@@ -109,7 +109,7 @@
 {% endmacro %}
 
 {%- macro sqlserver__get_use_database_sql(database) -%}
-  USE [{{database | replace('"', '')}}];
+  USE {{ adapter.quote(database | replace('"', '')) }};
 {%- endmacro -%}
 
 {% macro sqlserver__list_schemas(database) %}

dbt/include/sqlserver/macros/adapters/relation.sql

@@ -50,7 +50,8 @@
 {% macro sqlserver__rename_relation(from_relation, to_relation) -%}
   {% call statement('rename_relation') -%}
      {{ get_use_database_sql(from_relation.database) }}
-      EXEC sp_rename '{{ from_relation.schema }}.{{ from_relation.identifier }}', '{{ to_relation.identifier }}'
+      {#- @objname takes a quoted, qualified name; @newname must stay bare -#}
+      EXEC sp_rename '{{ escape_single_quotes(from_relation.include(database=False)) }}', '{{ escape_single_quotes(to_relation.identifier) }}'
   {%- endcall %}
 {% endmacro %}
 

dbt/include/sqlserver/macros/adapters/schema.sql

@@ -1,9 +1,9 @@
 {% macro sqlserver__create_schema(relation) -%}
   {% call statement('create_schema') -%}
-    USE [{{ relation.database }}];
+    {{ get_use_database_sql(relation.database) }}
     IF NOT EXISTS (SELECT * FROM sys.schemas WHERE name = '{{ relation.schema }}')
     BEGIN
-    EXEC('CREATE SCHEMA [{{ relation.schema }}]')
+    EXEC('CREATE SCHEMA {{ adapter.quote(relation.schema) }}')
     END
   {% endcall %}
 {% endmacro %}
@@ -13,7 +13,7 @@
     {{ get_use_database_sql(relation.database) }}
     IF NOT EXISTS (SELECT * FROM sys.schemas WHERE name = '{{ relation.schema }}')
     BEGIN
-    EXEC('CREATE SCHEMA [{{ relation.schema }}] AUTHORIZATION [{{ schema_authorization }}]')
+    EXEC('CREATE SCHEMA {{ adapter.quote(relation.schema) }} AUTHORIZATION {{ adapter.quote(schema_authorization) }}')
     END
   {% endcall %}
 {% endmacro %}

dbt/include/sqlserver/macros/materializations/models/unit_test/unit_test_create_table_as.sql

@@ -37,7 +37,7 @@
 
         {% set listColumns %}
             {% for column in model['columns'] %}
-                {{ "["~column~"]" }}{{ ", " if not loop.last }}
+                {{ adapter.quote(column) }}{{ ", " if not loop.last }}
             {% endfor %}
         {%endset%}
 

dbt/include/sqlserver/macros/materializations/tests.sql

@@ -1,14 +1,15 @@
 {% macro sqlserver__get_test_sql(main_sql, fail_calc, warn_if, error_if, limit) -%}
 
   -- Create target schema if it does not
-  USE [{{ target.database }}];
+  {{ get_use_database_sql(target.database) }}
   IF NOT EXISTS (SELECT * FROM sys.schemas WHERE name = '{{ target.schema }}')
   BEGIN
-    EXEC('CREATE SCHEMA [{{ target.schema }}]')
+    EXEC('CREATE SCHEMA {{ adapter.quote(target.schema) }}')
   END
 
+  {% set testview_name = "testview_" ~ local_md5(main_sql) ~ "_" ~ (range(1300, 19000) | random) %}
   {% set testview %}
-    [{{ target.schema }}].[testview_{{ local_md5(main_sql) }}_{{ range(1300, 19000) | random }}]
+    {{ adapter.quote(target.schema) }}.{{ adapter.quote(testview_name) }}
   {% endset %}
 
   {% set sql = main_sql.replace("'", "''")%}

dbt/include/sqlserver/macros/materializations/unit_tests.sql

@@ -1,19 +1,21 @@
 {% macro sqlserver__get_unit_test_sql(main_sql, expected_fixture_sql, expected_column_names) -%}
 
-  USE [{{ target.database }}];
+  {{ get_use_database_sql(target.database) }}
   IF NOT EXISTS (SELECT * FROM sys.schemas WHERE name = '{{ target.schema }}')
   BEGIN
-  EXEC('CREATE SCHEMA [{{ target.schema }}]')
+  EXEC('CREATE SCHEMA {{ adapter.quote(target.schema) }}')
   END
 
+  {% set test_view_name = "testview_" ~ local_md5(main_sql) ~ "_" ~ (range(1300, 19000) | random) %}
   {% set test_view %}
-      [{{ target.schema }}].[testview_{{ local_md5(main_sql) }}_{{ range(1300, 19000) | random }}]
+      {{ adapter.quote(target.schema) }}.{{ adapter.quote(test_view_name) }}
   {% endset %}
   {% set test_sql = main_sql.replace("'", "''")%}
   EXEC('create view {{test_view}} as {{ test_sql }};')
 
+  {% set expected_view_name = "expectedview_" ~ local_md5(expected_fixture_sql) ~ "_" ~ (range(1300, 19000) | random) %}
   {% set expected_view %}
-      [{{ target.schema }}].[expectedview_{{ local_md5(expected_fixture_sql) }}_{{ range(1300, 19000) | random }}]
+      {{ adapter.quote(target.schema) }}.{{ adapter.quote(expected_view_name) }}
   {% endset %}
   {% set expected_sql = expected_fixture_sql.replace("'", "''")%}
   EXEC('create view {{expected_view}} as {{ expected_sql }};')

dbt/include/sqlserver/macros/relations/table/create.sql

@@ -23,7 +23,7 @@
     {%- set build_into_temp = temporary or _ident.endswith('__dbt_tmp') or _ident.endswith('__dbt_tmp_vw') -%}
 
     {%- do adapter.drop_relation(tmp_relation) -%}
-    USE [{{ relation.database }}];
+    {{ get_use_database_sql(relation.database) }}
     {{ get_create_view_as_sql(tmp_relation, sql) }}
 
     {%- set table_name -%}
@@ -39,15 +39,15 @@
             {{ build_columns_constraints(relation) }}
             {% set listColumns %}
                 {% for column in model['columns'] %}
-                    {{ "["~column~"]" }}{{ ", " if not loop.last }}
+                    {{ adapter.quote(column) }}{{ ", " if not loop.last }}
                 {% endfor %}
             {%endset%}
             INSERT INTO {{relation}} WITH (TABLOCK) ({{listColumns}})
             SELECT {{listColumns}} FROM {{tmp_relation}} {{ query_label }}
 
         {% else %}
             {%- if build_into_temp -%}
-            IF OBJECT_ID('{{ escape_single_quotes(relation.schema) }}.{{ escape_single_quotes(relation.identifier) }}', 'U') IS NOT NULL
+            IF OBJECT_ID('{{ escape_single_quotes(relation.include(database=False)) }}', 'U') IS NOT NULL
                 EXEC('DROP TABLE {{ relation }}');
             {%- endif -%}
             SELECT * INTO {{ table_name }} FROM {{ tmp_relation }} {{ query_label }}
@@ -120,15 +120,15 @@
         later failure during the load would roll the marker back right along
         with it, defeating the point. -#}
     {%- set setup_sql -%}
-        USE [{{ relation.database }}];
+        {{ get_use_database_sql(relation.database) }}
         {{ get_create_view_as_sql(tmp_relation, sql) }}
 
         {#- drop any physical target before the bare CREATE / SELECT ... INTO
             below. OBJECT_ID reads the database directly, so this is robust to
             dbt's relation cache being stale (reporting none while the table
             exists) - which would otherwise collide with Msg 2714 (object
             already exists) -#}
-        IF OBJECT_ID('{{ escape_single_quotes(relation.schema) }}.{{ escape_single_quotes(relation.identifier) }}', 'U') IS NOT NULL
+        IF OBJECT_ID('{{ escape_single_quotes(relation.include(database=False)) }}', 'U') IS NOT NULL
             EXEC('DROP TABLE {{ relation }}');
 
         {% if contract_enforced %}
@@ -169,7 +169,7 @@
     {%- if contract_enforced -%}
         {%- set listColumns -%}
             {%- for column in model['columns'] -%}
-                {{ "["~column~"]" }}{{ ", " if not loop.last }}
+                {{ adapter.quote(column) }}{{ ", " if not loop.last }}
             {%- endfor -%}
         {%- endset -%}
         {%- set insert_statement -%}