diff --git a/CHANGELOG.md b/CHANGELOG.md index 96e6c12fe..c7f2ddc36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ - `EXTRACT_CLUSTER_ID_FROM_HTTP_PATH_REGEX` now stops the capture at `?` / `&`, so any trailing query string on `http_path` no longer corrupts the extracted cluster id. Latent issue on legacy hosts; the fix unblocks SPOG cluster paths. - Gate column-level constraints on `contract.enforced` to match the existing model-level gate, ensuring column-level NOT NULL / PK / FK / CHECK constraints are only applied when `contract.enforced: true` under `use_materialization_v2: true` ([#1470](https://github.com/databricks/dbt-databricks/pull/1470) closes [#1381](https://github.com/databricks/dbt-databricks/issues/1381)) - Fix managed Iceberg incremental models configured with `partition_by` silently losing their clustering after the first incremental run. Managed Iceberg stores `partition_by` as liquid clustering server-side, so the reconciler now treats `partition_by` as the desired clustering and no longer issues a spurious `ALTER TABLE ... CLUSTER BY NONE` ([#1496](https://github.com/databricks/dbt-databricks/pull/1496) closes [#1495](https://github.com/databricks/dbt-databricks/issues/1495)) +- Resolve `doc()` (docs blocks) inside `metric_view` model bodies. dbt-core only registers `doc()` in the schema-YAML render context, so `{{ doc(...) }}` in a metric view's `comment:` / `display_name:` fields (YAML spec 1.1+) previously failed to compile with `'doc' is undefined`. A `doc` macro now bridges model-body rendering to the manifest's doc lookup, so a single doc block can document both schema-YAML `description:` and metric-view fields; as a side effect `doc()` now resolves in all model bodies ([#1504](https://github.com/databricks/dbt-databricks/pull/1504) closes [#1501](https://github.com/databricks/dbt-databricks/issues/1501)) ### Under the Hood diff --git a/dbt/adapters/databricks/impl.py b/dbt/adapters/databricks/impl.py index 4f92061fe..55c44ccbd 100644 --- a/dbt/adapters/databricks/impl.py +++ b/dbt/adapters/databricks/impl.py @@ -1095,6 +1095,31 @@ def clean_sql(self, sql: str) -> str: def yaml_quote_backtick_values(self, yaml_body: str) -> str: return SqlUtils.yaml_quote_backtick_values(yaml_body) + @available.parse(lambda *a, **k: None) + def resolve_doc( + self, name: str, package: Optional[str] = None, node_package: Optional[str] = None + ) -> Optional[str]: + """Resolve a docs block by name and return its contents. + + dbt-core only exposes ``doc()`` in the schema-YAML render context, not the + model render context. A metric view's body is YAML (with ``comment:`` / + ``display_name:`` fields) rendered as model SQL, so ``{{ doc(...) }}`` there + is otherwise undefined. The ``doc`` macro bridges to this method so a single + doc block can document both schema-YAML descriptions and metric-view fields. + + Returns ``None`` when the block cannot be resolved -- including during parse, + before the manifest's doc lookup is built -- so the caller can decide how to + handle a miss. The ``@available.parse`` fallback keeps parse rendering a no-op. + """ + resolver = self._macro_resolver + if resolver is None or not hasattr(resolver, "resolve_doc"): + return None + current_project = self.config.project_name + target_doc = resolver.resolve_doc( # type: ignore[attr-defined] + name, package, current_project, node_package or current_project + ) + return target_doc.block_contents if target_doc is not None else None + @available def build_catalog_relation(self, model: RelationConfig) -> Optional[CatalogRelation]: """ diff --git a/dbt/include/databricks/macros/etc/doc.sql b/dbt/include/databricks/macros/etc/doc.sql new file mode 100644 index 000000000..ed1a2811b --- /dev/null +++ b/dbt/include/databricks/macros/etc/doc.sql @@ -0,0 +1,33 @@ +{# + dbt-core only registers `doc()` in the schema-YAML render context, so it is + undefined when rendering a model body. Metric view models carry their docs in + the model body (YAML `comment:` / `display_name:` fields), so `{{ doc(...) }}` + there fails with "'doc' is undefined". Defining `doc` as a macro fills that gap: + the model render context has no `doc` member, so Jinja falls back to this macro, + while the schema-YAML context keeps using dbt-core's built-in `doc()` (it carries + no macro namespace), leaving description rendering untouched. + + Accepts `doc('name')` or `doc('package', 'name')`, matching dbt-core's `doc()`. +#} +{% macro doc() %} + {#- During parse the manifest's doc lookup is not built yet; defer resolution + to execution, mirroring dbt-core where parse-time doc() is a no-op. -#} + {%- if not execute -%} + {{- return('') -}} + {%- endif -%} + {%- if varargs | length == 1 -%} + {%- set doc_package = none -%} + {%- set doc_name = varargs[0] -%} + {%- elif varargs | length == 2 -%} + {%- set doc_package = varargs[0] -%} + {%- set doc_name = varargs[1] -%} + {%- else -%} + {{ exceptions.raise_compiler_error("doc() takes exactly one or two arguments (" ~ (varargs | length) ~ " given)") }} + {%- endif -%} + {%- set node_package = model.package_name if model is defined and model else none -%} + {%- set block_contents = adapter.resolve_doc(doc_name, doc_package, node_package) -%} + {%- if block_contents is none -%} + {{ exceptions.raise_compiler_error("Documentation for '" ~ doc_name ~ "' not found") }} + {%- endif -%} + {{- return(block_contents) -}} +{% endmacro %} diff --git a/tests/functional/adapter/metric_views/fixtures.py b/tests/functional/adapter/metric_views/fixtures.py index 89b330de0..bb9c434a5 100644 --- a/tests/functional/adapter/metric_views/fixtures.py +++ b/tests/functional/adapter/metric_views/fixtures.py @@ -56,6 +56,27 @@ expr: count(1) """ +metric_view_doc_block = """ +{% docs orders_status_doc %} +Order lifecycle status, reused from a doc block. +{% enddocs %} +""" + +metric_view_with_doc = """ +{{ config(materialized='metric_view') }} + +version: 1.1 +source: "{{ ref('source_orders') }}" +dimensions: + - name: status + expr: status + display_name: "Order Status" + comment: "{{ doc('orders_status_doc') }}" +measures: + - name: total_orders + expr: count(1) +""" + metric_view_with_config = """ {{ config( diff --git a/tests/functional/adapter/metric_views/test_metric_view_materialization.py b/tests/functional/adapter/metric_views/test_metric_view_materialization.py index a0c4752da..26e9ae389 100644 --- a/tests/functional/adapter/metric_views/test_metric_view_materialization.py +++ b/tests/functional/adapter/metric_views/test_metric_view_materialization.py @@ -1,10 +1,12 @@ import pytest -from dbt.tests.util import get_manifest, run_dbt +from dbt.tests.util import get_manifest, read_file, run_dbt from tests.functional.adapter.metric_views.fixtures import ( basic_metric_view, metric_view_bare_ref, + metric_view_doc_block, metric_view_with_config, + metric_view_with_doc, metric_view_with_filter, source_table, ) @@ -240,3 +242,42 @@ def test_bare_ref_metric_view_creates_and_queries(self, project): fetch="all", ) assert query_result and query_result[0][0] == 3 + + +@pytest.mark.skip_profile("databricks_cluster") +class TestMetricViewWithDoc: + """Regression for #1501: doc() resolves in a metric_view body. + + dbt-core only exposes doc() in the schema-YAML render context, so a metric + view body (rendered as model SQL) previously failed with "'doc' is undefined". + """ + + @pytest.fixture(scope="class") + def models(self): + return { + "source_orders.sql": source_table, + "metric_view_docs.md": metric_view_doc_block, + "doc_metrics.sql": metric_view_with_doc, + } + + def test_doc_resolves_in_metric_view(self, project): + results = run_dbt(["run"]) + assert len(results) == 2 + assert all(r.status == "success" for r in results), ( + f"Expected all models to succeed, got: " + f"{[(r.node.name, r.status, r.message) for r in results]}" + ) + + # The doc block contents must be substituted into the compiled body, + # with no unresolved doc() call left behind. + compiled = read_file("target", "compiled", "test", "models", "doc_metrics.sql") + assert "Order lifecycle status, reused from a doc block." in compiled + assert "doc(" not in compiled + + # The view must still be functional after the substitution. + metric_view_name = f"{project.database}.{project.test_schema}.doc_metrics" + query_result = project.run_sql( + f"SELECT MEASURE(total_orders) FROM {metric_view_name}", + fetch="all", + ) + assert query_result and query_result[0][0] == 3 diff --git a/tests/unit/macros/etc/test_doc_macro.py b/tests/unit/macros/etc/test_doc_macro.py new file mode 100644 index 000000000..37c3abfb3 --- /dev/null +++ b/tests/unit/macros/etc/test_doc_macro.py @@ -0,0 +1,56 @@ +from unittest.mock import Mock + +import pytest + +from tests.unit.macros.base import MacroTestBase + + +class TestDocMacro(MacroTestBase): + """Unit tests for the ``doc`` macro that bridges doc() into model rendering. + + See #1501: dbt-core only registers doc() in the schema-YAML context, so a + metric_view body (rendered as model SQL) needs this macro fallback. + """ + + @pytest.fixture(scope="class") + def template_name(self) -> str: + return "doc.sql" + + @pytest.fixture(scope="class") + def macro_folders_to_load(self) -> list: + return ["macros/etc"] + + @pytest.fixture(autouse=True) + def setup(self, context): + # execute is False during parse; the macro resolves only at execution time. + context["execute"] = True + context["model"].package_name = "my_pkg" + context["adapter"].resolve_doc = Mock(return_value="DOC CONTENTS") + + def test_single_arg_resolves_block(self, template_bundle): + result = self.run_macro_raw(template_bundle.template, "doc", "my_block") + + assert result.strip() == "DOC CONTENTS" + template_bundle.context["adapter"].resolve_doc.assert_called_once_with( + "my_block", None, "my_pkg" + ) + + def test_two_args_pass_package(self, template_bundle): + result = self.run_macro_raw(template_bundle.template, "doc", "other_pkg", "my_block") + + assert result.strip() == "DOC CONTENTS" + template_bundle.context["adapter"].resolve_doc.assert_called_once_with( + "my_block", "other_pkg", "my_pkg" + ) + + def test_unresolved_block_raises(self, template_bundle): + template_bundle.context["adapter"].resolve_doc = Mock(return_value=None) + + self.run_macro_raw(template_bundle.template, "doc", "missing_block") + + template_bundle.context["exceptions"].raise_compiler_error.assert_called_once() + + # NOTE: the parse-time no-op (execute False) cannot be unit-tested here because + # MacroTestBase mocks return() as a plain identity, so the macro's early + # `{% if not execute %}{{ return('') }}` does not short-circuit rendering. That + # path is exercised by the `dbt parse` functional path instead.