dbt-labs / dbt-snowflake

dbt-snowflake contains all of the code enabling dbt to work with Snowflake
https://getdbt.com
Apache License 2.0
296 stars 176 forks source link

Normalize is_iceberg Field for Consistency in Snowflake with QUOTED_IDENTIFIERS_IGNORE_CASE #1229

Closed VersusFacit closed 1 week ago

VersusFacit commented 2 weeks ago

resolves #1227

Problem

In Snowflake, the QUOTED_IDENTIFIERS_IGNORE_CASE setting influences column names created by users, such as is_iceberg. However, this setting does not affect the column values returned by Snowflake’s SHOW OBJECTS query, creating inconsistency in how the is_iceberg field is referenced across different parts of the code.

When QUOTED_IDENTIFIERS_IGNORE_CASE is enabled, the is_iceberg column may appear in uppercase (IS_ICEBERG) in our homemade SHOW OBJECTS query, causing potential misalignment with the system table's always lowercase column names. This mismatch requires normalization to consistently refer to is_iceberg as a lowercase field name across Snowflake queries and Python processing, especially when working with adapters that handle Iceberg materializations.

Solution

Take 2: Removing quoting from the problem entirely and prove by hand that quoting in this metadata query doesn't matter one way or another.

I did so by hand and verified the test fails before the two fix lines are inserted.

just for fun, I did this with tdd: reproduce the failure/error, add the fix, go green!~~

Checklist

github-actions[bot] commented 2 weeks ago

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the dbt-snowflake contributing guide.

adrianburusdbt commented 2 weeks ago

@VersusFacit, there are a few other scenarios we need to consider as well, like this one: https://github.com/dbt-labs/dbt-core/issues/4422 The way we handle QUOTED_IDENTIFIERS_IGNORE_CASE is again the culprit, so I think we should fix this issue throughout the adapter, not just for is_iceberg, i.e. normalize all column names.