fix: auto-quote backtick-rendered refs in metric_view YAML body (#1361)#1430
fix: auto-quote backtick-rendered refs in metric_view YAML body (#1361)#1430sd-db wants to merge 4 commits into1.12.latestfrom
Conversation
Bare `{{ ref(...) }}` in a metric_view source: line rendered to
`db`.`schema`.`name` and broke the YAML scanner with
METRIC_VIEW_INVALID_VIEW_DEFINITION. Wrap such values in YAML double
quotes before embedding the body, preserving SQL identifier escaping.
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||
|
/integration-test |
|
Integration tests dispatched for PR #1430 by @sd-db. Track progress in the Actions tab. |
|
Integration results for PR #1430 — UC cluster ✅ success · SQL warehouse ✅ success · All-purpose cluster ✅ success |
There was a problem hiding this comment.
Does this parse the whole YAML file or just the source field? If we intend to only change source field can we make it more specific and not whole YAML wide.
Also the regex might need tightening. Valid identifiers as mentioned in this identifier syntax doc. Following samples break in the current implementation
pytest.param(
"source: `schema`.`name` # a comment",
'source: "`schema`.`name` # a comment',
id="two_part_with_comment",
),
# Very rare case, but it is valid syntax. I don't know if we should solve for this?
pytest.param(
"source: `a``b`.`schema`.`table`",
'source: "`a``b`.`schema`.`table`"',
id="escaped_backtick_in_identifier",
),
Good point, will limit this to only source field
Yes this is something we should look to solve for
Here pragmatically I think we should not solve for such cases and it is fine if this fails. We are specifically looking for well intentioned patterns for source ref that we want to parse for |
Address #1430 review: - Scope regex to source: key only (top-level + joins) instead of any \w+: - Preserve trailing YAML # comment - Allow space before colon (source :) - Use space-only whitespace (YAML rejects tabs) - Switch to named groups
Closes #1361.
Bug
Bare
{{ ref('x') }}in ametric_viewsource:field renders to`db`.`schema`.`x`, which the YAML scanner rejects:Fix
Add
SqlUtils.yaml_quote_backtick_values— wraps backtick-rendered identifiers in mapping values with"..."before the body is embedded in$$..$$. Preserves SQL identifier escaping for non-trivial names (hyphens, reserved words). Called from the metric_view create and alter macros.Same shape as
SqlUtils.clean_sqland theSTREAMstrip instreaming_table/create.sql.Test plan
tests/unit/test_handle.py) — idempotence, multi-line, indented, inline backticks left alone, etc.TestMetricViewBareRefruns a bare-ref metric_view + round-trips aMEASURE()querytests/functional/adapter/metric_views/suite (18 tests) — no regressionspre-commit run --all-filesclean