Skip to content
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- Apply column-level `databricks_tags` for incremental models on the V1 materialization path (`use_materialization_v2: false`, the default). They were silently dropped at create and on subsequent tag changes; the V1 incremental materialization now applies them, matching the `table` materialization and the V2 path. ([#1520](https://github.com/databricks/dbt-databricks/pull/1520) closes [#1307](https://github.com/databricks/dbt-databricks/issues/1307))
- Raise a `DbtRuntimeError` when a Python model job run terminates with a non-success `result_state` (e.g. `FAILED`/`TIMEDOUT`) instead of returning silently ([#1477](https://github.com/databricks/dbt-databricks/pull/1477))
- Fix PK/FK constraints declaring an `expression` (e.g. `RELY`) being dropped and re-added on every incremental run. The `expression` isn't readable from `information_schema`, so reconciliation never converged and issued `DROP CONSTRAINT ... CASCADE` each run — silently dropping dependent FKs, and erroring with `INTERNAL_ERROR` on newer Unity Catalog. PK/FK are now compared on `(name, columns)`. **Regression:** changing the `expression` on an existing PK/FK (`RELY`↔`NORELY`, or an expression-form FK's target) is no longer applied on incremental runs — use `--full-refresh`. ([#1552](https://github.com/databricks/dbt-databricks/pull/1552) closes [#1513](https://github.com/databricks/dbt-databricks/issues/1513))
- Fix materialized views always rebuilding because Databricks-internal `tblproperties` were read as configuration drift; the diff now compares only the configured properties ([#1350](https://github.com/databricks/dbt-databricks/pull/1350) closes [#1314](https://github.com/databricks/dbt-databricks/issues/1314)).

### Under the Hood

Expand Down
52 changes: 7 additions & 45 deletions dbt/adapters/databricks/relation_configs/tblproperties.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Any, ClassVar, Optional
from typing import ClassVar, Optional

from dbt.adapters.contracts.relation import RelationConfig
from dbt.adapters.relation_configs.config_base import RelationResults
Expand All @@ -19,48 +19,11 @@ class TblPropertiesConfig(DatabricksComponentConfig):
tblproperties: dict[str, str]
pipeline_id: Optional[str] = None

# List of tblproperties that should be ignored when comparing configs. These are generally
# set by Databricks and are not user-configurable.
ignore_list: ClassVar[list[str]] = [
"pipelines.pipelineId",
"delta.enableChangeDataFeed",
"delta.minReaderVersion",
"delta.minWriterVersion",
"pipeline_internal.catalogType",
"pipelines.metastore.tableName",
"pipeline_internal.enzymeMode",
"clusterByAuto",
"clusteringColumns",
"delta.enableRowTracking",
"delta.feature.appendOnly",
"delta.feature.changeDataFeed",
"delta.feature.checkConstraints",
"delta.feature.domainMetadata",
"delta.feature.generatedColumns",
"delta.feature.invariants",
"delta.feature.rowTracking",
"delta.rowTracking.materializedRowCommitVersionColumnName",
"delta.rowTracking.materializedRowIdColumnName",
"spark.internal.pipelines.top_level_entry.user_specified_name",
"delta.columnMapping.maxColumnId",
"spark.sql.internal.pipelines.parentTableId",
"delta.enableDeletionVectors",
"delta.feature.deletionVectors",
"delta.parquet.compression.codec",
]

def __eq__(self, __value: Any) -> bool:
"""Override equality check to ignore certain tblproperties."""

if not isinstance(__value, TblPropertiesConfig):
return False

def _without_ignore_list(d: dict[str, str]) -> dict[str, str]:
return {k: v for k, v in d.items() if k not in self.ignore_list}

return _without_ignore_list(self.tblproperties) == _without_ignore_list(
__value.tblproperties
)
def get_diff(self, other: "TblPropertiesConfig") -> Optional["TblPropertiesConfig"]:
# tblproperties are "set only" - we never unset tblproperties, only add or update them
if unapplied_properties := self.tblproperties.items() - other.tblproperties.items():
return TblPropertiesConfig(tblproperties={k: v for k, v in unapplied_properties})
return None


class TblPropertiesProcessor(DatabricksComponentProcessor[TblPropertiesConfig]):
Expand All @@ -71,12 +34,11 @@ def from_relation_results(cls, results: RelationResults) -> TblPropertiesConfig:
table = results.get("show_tblproperties")
tblproperties = dict()
pipeline_id = None

if table:
for row in table.rows:
if str(row[0]) == "pipelines.pipelineId":
pipeline_id = str(row[1])
elif str(row[0]) not in TblPropertiesConfig.ignore_list:
else:
tblproperties[str(row[0])] = str(row[1])

return TblPropertiesConfig(tblproperties=tblproperties, pipeline_id=pipeline_id)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@


def _check_tblproperties(tblproperties: TblPropertiesConfig, expected: dict):
final_tblproperties = {
k: v for k, v in tblproperties.tblproperties.items() if k not in tblproperties.ignore_list
}
# from_relation_results now returns all properties (including server-set ones), so scope the
# comparison to the keys the model configures rather than filtering a Databricks-internal list.
final_tblproperties = {k: v for k, v in tblproperties.tblproperties.items() if k in expected}
assert final_tblproperties == expected


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@


def _check_tblproperties(tblproperties: TblPropertiesConfig, expected: dict):
final_tblproperties = {
k: v for k, v in tblproperties.tblproperties.items() if k not in tblproperties.ignore_list
}
# from_relation_results now returns all properties (including server-set ones), so scope the
# comparison to the keys the model configures rather than filtering a Databricks-internal list.
final_tblproperties = {k: v for k, v in tblproperties.tblproperties.items() if k in expected}
assert final_tblproperties == expected


Expand Down
2 changes: 2 additions & 0 deletions tests/unit/relation_configs/test_incremental_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ def test_from_results(self):
"tblproperties": TblPropertiesConfig(
tblproperties={
"prop": "f1",
"clusterByAuto": "true",
"clusteringColumns": '[["col1"],[""a""]]',
"delta.constraints.check_name_length": "LENGTH (name) >= 1",
}
),
Expand Down
48 changes: 44 additions & 4 deletions tests/unit/relation_configs/test_tblproperties.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,17 @@ def test_from_results__multiple(self):
spec = TblPropertiesProcessor.from_relation_results(results)
assert spec == TblPropertiesConfig(tblproperties={"prop": "1", "other": "other"})

def test_from_results__drops_ignored_properties(self):
# Properties set by Databricks (e.g. the server-default parquet compression codec)
# must be dropped so they don't show up as spurious configuration changes.
def test_from_results__retains_server_set_properties(self):
# Server-set properties are kept here; get_diff filters them out, not from_relation_results.
results = {
"show_tblproperties": fixtures.gen_tblproperties(
[["prop", "1"], ["delta.parquet.compression.codec", "zstd"]]
)
}
spec = TblPropertiesProcessor.from_relation_results(results)
assert spec == TblPropertiesConfig(tblproperties={"prop": "1"})
assert spec == TblPropertiesConfig(
tblproperties={"prop": "1", "delta.parquet.compression.codec": "zstd"}
)

def test_from_model_node__without_tblproperties(self):
model = Mock()
Expand Down Expand Up @@ -108,3 +109,42 @@ def test_from_model_node__with_iceberg_no_flag_no_properties(self):
spec = TblPropertiesProcessor.from_relation_config(model)
# Should not have UniForm properties without explicit use_managed_iceberg=False
assert spec == TblPropertiesConfig(tblproperties={})


class TestTblPropertiesConfig:
def test_get_diff__empty_and_some_exist(self):
# tblproperties are "set only" - when config has no tblproperties and the relation
# has some, we don't unset the existing tblproperties
config_properties = TblPropertiesConfig(tblproperties={})
relation_properties = TblPropertiesConfig(tblproperties={"prop": "1"})
diff = config_properties.get_diff(relation_properties)
assert diff is None # No changes needed since we don't unset tblproperties

def test_get_diff__some_new_and_empty_existing(self):
config_properties = TblPropertiesConfig(tblproperties={"prop": "1"})
relation_properties = TblPropertiesConfig(tblproperties={})
diff = config_properties.get_diff(relation_properties)
assert diff == TblPropertiesConfig(tblproperties={"prop": "1"})

def test_get_diff__mixed_case(self):
# tblproperties are "set only" - only the new/updated tblproperties are included
config_properties = TblPropertiesConfig(tblproperties={"prop": "1", "other": "other"})
relation_properties = TblPropertiesConfig(tblproperties={"prop": "2", "c": "value"})
diff = config_properties.get_diff(relation_properties)
assert diff == TblPropertiesConfig(tblproperties={"prop": "1", "other": "other"})

def test_get_diff__no_changes(self):
config_properties = TblPropertiesConfig(tblproperties={"prop": "1"})
relation_properties = TblPropertiesConfig(tblproperties={"prop": "1"})
diff = config_properties.get_diff(relation_properties)
assert diff is None

def test_get_diff__ignores_server_set_properties(self):
# A server-set property (e.g. the default parquet compression codec) present on the
# relation but absent from the model config must not register as a change.
config_properties = TblPropertiesConfig(tblproperties={"prop": "1"})
relation_properties = TblPropertiesConfig(
tblproperties={"prop": "1", "delta.parquet.compression.codec": "zstd"}
)
diff = config_properties.get_diff(relation_properties)
assert diff is None
Loading