(superset) 01/02: test(snowflake): reproduce case-sensitive column quoting

[jl...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

sadpandajoe pushed a commit to branch fix-snowflake-case-sensitive-column-quoting
in repository https://gitbox.apache.org/repos/asf/superset.git

commit d5f5c40817c71e346b20098ec62dcac720e9114f
Author: sadpandajoe <jc...@gmail.com>
AuthorDate: Tue Aug 18 22:14:42 2026 +0000

    test(snowflake): reproduce case-sensitive column quoting
---
 RCA.md                                             | 64 ++++++++++++++++++++++
 tests/unit_tests/connectors/sqla/models_test.py    | 30 ++++++++++
 tests/unit_tests/db_engine_specs/test_base.py      |  7 +++
 tests/unit_tests/db_engine_specs/test_snowflake.py | 27 +++++++++
 tests/unit_tests/models/helpers_test.py            | 34 ++++++++++++
 5 files changed, 162 insertions(+)

diff --git a/RCA.md b/RCA.md
new file mode 100644
index 00000000000..974f014b110
--- /dev/null
+++ b/RCA.md
@@ -0,0 +1,64 @@
+# Snowflake Case-Sensitive Column Identifier Quoting
+
+## Symptom
+
+A physical Snowflake table can contain explicitly quoted, case-sensitive lowercase
+columns, for example:
+
+```sql
+CREATE TABLE bug_test ("id" INT, "name" VARCHAR);
+```
+
+Charts backed by such a table can generate references such as `SELECT id, name`.
+Snowflake folds those unquoted identifiers to `ID` and `NAME`, which do not match the
+physical lowercase identifiers, so the query fails with a column-not-found error.
+
+## Root Cause
+
+The Snowflake SQLAlchemy dialect preserves the exact case discovered during
+reflection by returning `quoted_name` values from its name-normalization logic.
+Superset persists reflected table and column names in ordinary string ORM fields,
+however, so the `quoted_name` subtype and its explicit-quoting flag are lost after a
+save and reload.
+
+During query generation, Superset consequently passes a plain string to
+`sqlalchemy.column()`. SQLAlchemy's generic identifier heuristic leaves all-lowercase
+names unquoted and quotes names containing uppercase or special characters. That
+heuristic fits engines that fold unquoted identifiers to lowercase, but it is the
+opposite of Snowflake's unquoted-identifier behavior. Superset can no longer infer
+whether a lowercase string was explicitly quoted when its table was created.
+
+For datasets with `normalize_columns` disabled, that history is not needed: the
+stored value is already the exact physical identifier produced during metadata
+synchronization. Explicitly quoting that exact value is safe and idempotent.
+
+## Fix
+
+The engine-spec interface gains an identifier-preparation hook whose default
+implementation returns the supplied name unchanged, preserving every other engine's
+existing SQLAlchemy auto-quoting behavior.
+
+Snowflake overrides the hook to return an explicitly quoted SQLAlchemy identifier
+when `normalize_columns` is disabled. Physical column construction in the modern and
+legacy dataset query paths uses the hook before creating SQLAlchemy column elements.
+Calculated and custom expressions remain on their existing `literal_column()` path.
+
+When `normalize_columns` is enabled, the Snowflake hook leaves the stored name
+unchanged because normalization is intentionally lossy; force-quoting that display
+form could refer to a different physical identifier.
+
+## Out of Scope
+
+A related symptom affects dataset creation when the table name itself is an
+explicitly quoted lowercase Snowflake identifier. Table, schema, and catalog quoting
+in `FROM` clauses and table-existence checks are separate code paths and are not
+changed by this fix. They require a follow-up focused on physical table identifiers.
+
+## Testing
+
+Offline unit tests compile SQLAlchemy expressions with an ANSI double-quote dialect,
+so no Snowflake connection or Snowflake SQLAlchemy package is required. The tests
+cover the physical-column query paths, Snowflake's quoted and normalized identifier
+behavior, and the no-op default engine-spec behavior. The affected connector,
+query-helper, Snowflake engine-spec, and base engine-spec test modules provide
+regression coverage.
diff --git a/tests/unit_tests/connectors/sqla/models_test.py b/tests/unit_tests/connectors/sqla/models_test.py
index e95ff6c3d2e..4240ea40c45 100644
--- a/tests/unit_tests/connectors/sqla/models_test.py
+++ b/tests/unit_tests/connectors/sqla/models_test.py
@@ -21,6 +21,7 @@ import pandas as pd
 import pytest
 from pytest_mock import MockerFixture
 from sqlalchemy import create_engine
+from sqlalchemy.dialects import sqlite
 from sqlalchemy.exc import IntegrityError
 from sqlalchemy.orm.session import Session
 
@@ -49,6 +50,35 @@ from superset.superset_typing import QueryObjectDict
 from superset.utils import json
 
 
+def test_get_sqla_col_quotes_snowflake_case_sensitive_identifier(
+    mocker: MockerFixture,
+) -> None:
+    """Snowflake physical columns retain their exact reflected case in generated SQL."""
+    from superset.db_engine_specs.snowflake import SnowflakeEngineSpec
+
+    database = Database(database_name="db", sqlalchemy_uri="sqlite://")
+    mocker.patch.object(
+        Database,
+        "get_db_engine_spec",
+        return_value=SnowflakeEngineSpec,
+    )
+    table = SqlaTable(
+        table_name="bug_test",
+        database=database,
+        normalize_columns=False,
+    )
+    tbl_column = TableColumn(column_name="id", type="INTEGER", table=table)
+
+    rendered = str(
+        tbl_column.get_sqla_col().compile(
+            dialect=sqlite.dialect(),
+            compile_kwargs={"literal_binds": True},
+        )
+    )
+
+    assert rendered == '"id"'
+
+
 def test_query_bubbles_errors(mocker: MockerFixture) -> None:
     """
     Test that the `query` method bubbles exceptions correctly.
diff --git a/tests/unit_tests/db_engine_specs/test_base.py b/tests/unit_tests/db_engine_specs/test_base.py
index 01f9d46242f..4578ef58d1c 100644
--- a/tests/unit_tests/db_engine_specs/test_base.py
+++ b/tests/unit_tests/db_engine_specs/test_base.py
@@ -291,6 +291,13 @@ def test_get_default_catalog(mocker: MockerFixture) -> None:
     assert BaseEngineSpec.get_default_catalog(database) is None
 
 
+def test_prepare_identifier_returns_name_unchanged() -> None:
+    name = "physical_column"
+
+    assert BaseEngineSpec.prepare_identifier(name, normalize_columns=False) is name
+    assert BaseEngineSpec.prepare_identifier(name, normalize_columns=True) is name
+
+
 def test_quote_table() -> None:
     """
     Test the `quote_table` function.
diff --git a/tests/unit_tests/db_engine_specs/test_snowflake.py b/tests/unit_tests/db_engine_specs/test_snowflake.py
index 3f166b22650..c1d7354e244 100644
--- a/tests/unit_tests/db_engine_specs/test_snowflake.py
+++ b/tests/unit_tests/db_engine_specs/test_snowflake.py
@@ -24,6 +24,7 @@ from unittest import mock
 import pytest
 from pytest_mock import MockerFixture
 from sqlalchemy.engine.url import make_url
+from sqlalchemy.sql import quoted_name
 
 from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
 from superset.utils import json
@@ -31,6 +32,32 @@ from tests.unit_tests.db_engine_specs.utils import assert_convert_dttm
 from tests.unit_tests.fixtures.common import dttm  # noqa: F401
 
 
+@pytest.mark.parametrize("name", ["lowercase", "UPPERCASE"])
+def test_prepare_identifier_quotes_exact_case_names(name: str) -> None:
+    from superset.db_engine_specs.snowflake import SnowflakeEngineSpec
+
+    identifier = SnowflakeEngineSpec.prepare_identifier(
+        name,
+        normalize_columns=False,
+    )
+
+    assert isinstance(identifier, quoted_name)
+    assert str(identifier) == name
+    assert identifier.quote is True
+
+
+def test_prepare_identifier_preserves_normalized_name() -> None:
+    from superset.db_engine_specs.snowflake import SnowflakeEngineSpec
+
+    name = "lowercase"
+    identifier = SnowflakeEngineSpec.prepare_identifier(
+        name,
+        normalize_columns=True,
+    )
+
+    assert identifier is name
+
+
 @pytest.mark.parametrize(
     "target_type,expected_result",
     [
diff --git a/tests/unit_tests/models/helpers_test.py b/tests/unit_tests/models/helpers_test.py
index a7156b795b9..f100c74287d 100644
--- a/tests/unit_tests/models/helpers_test.py
+++ b/tests/unit_tests/models/helpers_test.py
@@ -4552,6 +4552,40 @@ def test_simple_metric_quotes_column_requiring_quoting(database: Database) -> No
     )
 
 
+def test_convert_tbl_column_quotes_snowflake_case_sensitive_identifier(
+    database: Database,
+    mocker: MockerFixture,
+) -> None:
+    """The chart query-object path quotes exact-case Snowflake physical columns."""
+    from superset.connectors.sqla.models import SqlaTable, TableColumn
+    from superset.db_engine_specs.snowflake import SnowflakeEngineSpec
+    from superset.models.core import Database
+
+    mocker.patch.object(
+        Database,
+        "get_db_engine_spec",
+        return_value=SnowflakeEngineSpec,
+    )
+    table = SqlaTable(
+        database=database,
+        table_name="bug_test",
+        normalize_columns=False,
+    )
+    tbl_column = TableColumn(column_name="name", type="VARCHAR", table=table)
+
+    with database.get_sqla_engine() as engine:
+        dialect = engine.dialect
+
+    rendered = str(
+        table.convert_tbl_column_to_sqla_col(tbl_column).compile(
+            dialect=dialect,
+            compile_kwargs={"literal_binds": True},
+        )
+    )
+
+    assert rendered == '"name"'
+
+
 @pytest.mark.parametrize(
     "native_type",
     [