Skip to content

Commit 3f1630f

Browse files
committed
fix(schema_loader): quote case-sensitive column names in external models
create_external_models wrote column names from the adapter verbatim, so on case-folding dialects like postgres a quoted column such as "ID" was serialized as the unquoted key ID. On load, normalize_identifiers folded it to id and the generated external_models.yaml no longer described the table, breaking plan and lint with ambiguousorinvalidcolumn errors. Only quote identifiers whose unquoted form would be rewritten by the dialect's normalization; already-quoted names and identifiers that normalize to themselves (including pseudo columns like _sync_row_hash) are written unchanged. Signed-off-by: devtechedge <devtechedge@gmail.com>
1 parent 80731de commit 3f1630f

2 files changed

Lines changed: 63 additions & 1 deletion

File tree

sqlmesh/core/schema_loader.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
from sqlglot import exp
88
from sqlglot.dialects.dialect import DialectType
9+
from sqlglot.optimizer.normalize_identifiers import normalize_identifiers
910

1011
from sqlmesh.core.console import get_console
1112
from sqlmesh.core.engine_adapter import EngineAdapter
@@ -90,6 +91,16 @@ def create_external_models_file(
9091
yaml.dump(entries_to_keep + schemas, file)
9192

9293

94+
def _serialize_column_name(name: str, dialect: DialectType) -> str:
95+
if name.startswith('"') and name.endswith('"'):
96+
return name
97+
return (
98+
name
99+
if normalize_identifiers(exp.to_column(name), dialect=dialect).name == name
100+
else f'"{name}"'
101+
)
102+
103+
93104
def get_columns(
94105
adapter: EngineAdapter, dialect: DialectType, table: str, strict: bool
95106
) -> t.Optional[t.Dict[str, t.Any]]:
@@ -98,7 +109,10 @@ def get_columns(
98109
"""
99110
try:
100111
columns = adapter.columns(table, include_pseudo_columns=True)
101-
return {c: dtype.sql(dialect=dialect) for c, dtype in columns.items()}
112+
return {
113+
_serialize_column_name(c, dialect): dtype.sql(dialect=dialect)
114+
for c, dtype in columns.items()
115+
}
102116
except Exception as e:
103117
msg = f"Unable to get schema for '{table}': '{e}'."
104118
if strict:

tests/core/test_schema_loader.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,54 @@ def test_no_internal_model_conversion(tmp_path: Path, mocker: MockerFixture):
389389
create_external_model(**row, dialect="bigquery")
390390

391391

392+
def test_create_external_models_quotes_case_sensitive_columns(
393+
tmp_path: Path, mocker: MockerFixture
394+
):
395+
engine_adapter_mock = mocker.Mock()
396+
engine_adapter_mock.columns.return_value = {
397+
"ID": exp.DataType.build("bigint"),
398+
"ORGANID": exp.DataType.build("text"),
399+
"CDATE": exp.DataType.build("date"),
400+
"billno": exp.DataType.build("text"),
401+
"_sync_row_hash": exp.DataType.build("text"),
402+
}
403+
404+
state_reader_mock = mocker.Mock()
405+
state_reader_mock.nodes_exist.return_value = set()
406+
407+
model_a = SqlModel(name="a", query=parse_one("select * FROM raw_fruits"))
408+
409+
filename = tmp_path / c.EXTERNAL_MODELS_YAML
410+
create_external_models_file(
411+
filename,
412+
{"a": model_a}, # type: ignore
413+
engine_adapter_mock,
414+
state_reader_mock,
415+
"postgres",
416+
)
417+
418+
schema = yaml.load(filename)
419+
assert len(schema) == 1
420+
# only identifiers that would be case-folded by the dialect are quoted
421+
assert list(schema[0]["columns"]) == [
422+
'"ID"',
423+
'"ORGANID"',
424+
'"CDATE"',
425+
"billno",
426+
"_sync_row_hash",
427+
]
428+
429+
# the quoted keys must round-trip through the model loader without being folded to lowercase
430+
external_model = create_external_model(**schema[0], dialect="postgres")
431+
assert list(external_model.columns_to_types) == [
432+
"ID",
433+
"ORGANID",
434+
"CDATE",
435+
"billno",
436+
"_sync_row_hash",
437+
]
438+
439+
392440
def test_missing_table(tmp_path: Path):
393441
config = Config(gateways=GatewayConfig(connection=DuckDBConnectionConfig()))
394442
context = Context(paths=[str(tmp_path.absolute())], config=config)

0 commit comments

Comments
 (0)