Skip to content

column_descriptions drops identifier quoting, breaking all column comments on case-sensitive columns #5943

Description

@michal-billtech

Summary

column_descriptions keys are upper-cased even when the column name is explicitly quoted in the model. On engines where quoting makes identifiers case-sensitive (Snowflake), the generated ALTER ... COMMENT statement then references a column that does not exist, and all column comments for that table are silently dropped.

The quoting information is discarded before normalization runs, so there is no way to work around this from the model side — the name is already quoted.

Reproduction

models/demo.sql:

MODEL (
  name demo.quoted_columns,
  kind FULL,
  column_descriptions (
    "myColumn" = 'comment for a case-sensitive column'
  )
);

SELECT 1 AS "myColumn"

config.yaml:

gateways:
  duckdb:
    connection:
      type: duckdb
default_gateway: duckdb
model_defaults:
  dialect: snowflake
  start: '2024-01-01'
from sqlmesh.core.context import Context
ctx = Context(paths='.')
m = ctx.get_model('demo.quoted_columns')
print('descriptions:', dict(m.column_descriptions))
print('columns     :', list(m.columns_to_types))

Actual:

descriptions: {'MYCOLUMN': 'comment for a case-sensitive column'}
columns     : ['myColumn']

Expected: the description key should be myColumn, matching the column it describes.

Note that the two disagree within the model itselfcolumns_to_types preserves the quoted casing, column_descriptions does not. No warning is emitted about the mismatch, even though _column_descriptions_validator has a check for descriptions that do not correspond to a column (it compares the already-upper-cased key, so it does not fire here).

Root cause

sqlmesh/core/model/meta.py (line numbers from main):

:327   {".".join([part.this for part in v.this.parts]): v.expression.name for v in vs}
:331   normalize_identifiers(k, dialect=dialect).name

Line 327 extracts part.this, which is the bare identifier text — the quoted flag on the identifier is dropped. Line 331 then normalizes a plain string, so normalize_identifiers treats it as unquoted and upper-cases it for Snowflake.

from sqlglot import exp, parse_one
from sqlglot.optimizer.normalize_identifiers import normalize_identifiers

col = parse_one('"myColumn"', dialect='snowflake', into=exp.Column)
key = '.'.join([p.this for p in col.parts])
print(key, [p.quoted for p in col.parts])                                  # myColumn [True]
print(normalize_identifiers(key, dialect='snowflake').name)                # MYCOLUMN
print(normalize_identifiers(col.copy(), dialect='snowflake').sql('snowflake'))  # "myColumn"

Passing the expression instead of the string makes normalize_identifiers respect the quoting, as the third line shows.

Downstream effect on Snowflake

sqlmesh/core/engine_adapter/snowflake.py:

:647   column_sql = exp.column(column_name).sql(dialect=self.dialect, identify=True)
:654   combined_sql = f"ALTER {table_kind} {table_sql} ALTER {', '.join(list_comment_sql)}"

Line 647 re-quotes the already upper-cased name, producing "MYCOLUMN", which does not match the actual column myColumn:

ALTER VIEW "db"."schema"."table" ALTER COLUMN "MYCOLUMN" COMMENT '...'
-- 000904 (42000): SQL compilation error: invalid identifier 'MYCOLUMN'

Two things make this worse than a single missing comment:

  1. Line 654 batches every column into one statement. One mismatched name fails the whole ALTER, so no column on that table gets its comment — including the columns whose names were lower-case and would have worked. In our case two models ended up with 0/16 and 0/10 columns commented.

  2. The failure is reported as a permissions problem (:658):

    Column comments for table '...' not registered - this may be due to limited permissions.
    

    The actual error is an unknown identifier. This sends people looking at grants instead of at identifier casing; it took reading the query history to find the real cause.

Suggested fix

In _column_descriptions_validator, keep the identifier expression through normalization instead of reducing it to a string, so quoted=True survives.

Independently, it may be worth issuing one ALTER per column (or falling back to per-column statements when the batch fails) so a single bad name cannot discard every comment on the table, and logging the underlying engine error rather than guessing at permissions.

Environment

  • SQLMesh 0.231.1; the relevant code is unchanged on main (verified against 0.236.1 — meta.py:327/331 and snowflake.py:647/654/658 are identical)
  • Engine: Snowflake
  • model_defaults.dialect: snowflake
  • The reproduction above needs only DuckDB, since the mismatch is already visible in the loaded model

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions