From 7480529192faf5bdd486f3547dc2744e2dcde30b Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Fri, 26 Jun 2026 10:54:58 -0700 Subject: [PATCH 01/44] Stop LookML export from silently turning measures into COUNT The measure export fell back to type_mapping.get(metric.agg, "count"), so any aggregation Looker has no entry for was emitted as type: count -- a silent change to a row count that round-trips back as agg=count: - median / stddev / variance exported as COUNT(col) - complex metric types (cumulative/conversion/retention/cohort, agg=None) exported as COUNT(*) Map median to Looker's native type, emit stddev/variance as type: number with an explicit SQL aggregate, export agg-less opaque measures as type: number, and skip-with-warning anything with no LookML equivalent instead of defaulting to count. --- sidemantic/adapters/lookml.py | 318 +++++- tests/adapters/lookml/test_edge_cases.py | 1178 ++++++++++++++++++++++ 2 files changed, 1477 insertions(+), 19 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 3193a0eb..b507ecfb 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -625,10 +625,26 @@ def _range_sql_negated(bounds, col: str) -> str: return conds[0] if len(conds) == 1 else "(" + " OR ".join(conds) + ")" @staticmethod - def _split_top_level_commas(s: str) -> list[str]: - """Split on commas that are NOT inside ``[...]``/``(...)`` brackets.""" - out, cur, depth = [], "", 0 + def _split_top_level_commas(s: str, quote_aware: bool = False) -> list[str]: + """Split on commas that are NOT inside ``[...]``/``(...)`` brackets. + + With ``quote_aware`` also ignore commas inside SQL string literals + (``'...'`` / ``"..."``) -- needed when splitting SQL expressions to count + aggregate arity (a single-arg ``COUNT(DISTINCT a || ',' || b)`` must not look + multi-column). It is OFF by default because LookML filter VALUES treat an + apostrophe as a literal char (``O'Brien, Smith`` is two values, not one). + """ + out, cur, depth, quote = [], "", 0, None for ch in s: + if quote_aware and quote is not None: + cur += ch + if ch == quote: + quote = None + continue + if quote_aware and ch in "'\"": + quote = ch + cur += ch + continue if ch in "[(": depth += 1 elif ch in ")]": @@ -1233,6 +1249,10 @@ def _folded_measure_filter(m_def): measure_names.add(m_name) m_type = m.get("type", "count") agg_template = self._SQL_AGG_FUNC.get(m_type) + # An approximate count_distinct must aggregate approximately when wrapped + # by a post-SQL measure (percent_of_total/previous), matching the direct metric. + if m_type == "count_distinct" and m.get("approximate") in ("yes", True): + agg_template = "APPROX_COUNT_DISTINCT({0})" if agg_template: measure_agg_lookup[m_name] = agg_template m_sql = m.get("sql") @@ -2140,6 +2160,10 @@ def _parse_measure( agg_type = type_mapping.get(measure_type) + # Looker's `approximate: yes` on a count_distinct -> approximate distinct count. + if agg_type == "count_distinct" and measure_def.get("approximate") in ("yes", True): + agg_type = "approx_count_distinct" + # Parse filters - lkml parses these as filters__all # There are TWO different filter syntaxes in LookML: # 1. Shorthand: filters: [status: "completed"] @@ -2921,6 +2945,147 @@ def export(self, graph: SemanticGraph, output_path: str | Path) -> None: lookml_str = lkml.dump(data) f.write(lookml_str) + @staticmethod + def _fold_filter_conds(filters: list[str], model: Model) -> str: + """Resolve ``metric.filters`` into an AND-joined, ``${TABLE}``-qualified SQL + predicate for folding into an exported aggregate measure. + + Field refs are resolved through the model's dimension SQL so a renamed column is + used, not a bare name. Three forms are handled: ``{model}.col``, the model's own + name ``orders.col``, and an UNqualified dimension name used as a column + (``status = 'done'``, matched only before a comparison operator so string VALUES + aren't rewritten). Each filter is parenthesized so a filter containing ``OR`` is + not broken by ``AND``'s higher precedence. + """ + dim_sql = {d.name: d.sql for d in model.dimensions if d.sql} + dim_names = {d.name for d in model.dimensions} + + def _qualify(val: str) -> str: + # Bare column -> qualify with {model}. so it stays unambiguous in joins; + # any resolved expression is parenthesized to preserve precedence. + if re.fullmatch(r"\w+", val): + return f"({{model}}.{val})" + return f"({val})" + + # Qualified ref (group 1), OR a bare known-dimension name (group 2) used as a + # column anywhere (incl. inside a function like LOWER(status)). Matching is done + # only OUTSIDE single-quoted string literals (see _resolve), so a quoted value + # that happens to equal a dimension name is never rewritten. Both alternatives use a + # negative lookbehind for `.`/word-char: the bare one so it does NOT match the field + # of a foreign qualifier (`status` inside `customers.status`), and the model-name one + # so it does NOT match a schema-qualified ref (`orders.status` inside + # `schema.orders.status`). The bare alt also has a negative lookahead for `(` so it + # does NOT match a function name (e.g. `date(...)`). + names_alt = "|".join(re.escape(n) for n in sorted(dim_names, key=len, reverse=True)) + pattern = rf"(?:\{{model\}}|(? str: + def _one(m): + # The bare-dimension alternative (group 2) only exists when names_alt is + # non-empty; with no declared dimensions the pattern has a single group, so + # read group 2 defensively (m.group(2) would raise IndexError otherwise). + bare = m.group(2) if m.re.groups >= 2 else None + if bare is not None: + # Bare dimension-name alternative: skip when it sits in a SQL TYPE context + # (a cast target), not a column operand -- e.g. CAST(x AS date) or x::date + # with a `date` dimension. Rewriting the type token to a column would emit + # invalid SQL like CAST(x AS (${TABLE}.order_date)). (Typed literals like + # `date '2024-01-01'` are protected earlier, in the split below.) + pre = m.string[: m.start()] + if re.search(r"(?is)\bAS\s+$", pre) or pre.rstrip().endswith("::"): + return m.group(0) + name = m.group(1) or bare + return _qualify(dim_sql.get(name, name)) + + # Split out (and thus protect from rewriting) SQL TYPED LITERALS whose type keyword + # equals a dimension name (`date '2024-01-01'`, `timestamp '...'`, `interval '...'`) + # -- the whole ` '...'` unit is kept intact so the leading `date`/`time`/etc. + # is not mistaken for a column; then single-quoted string literals, double-quoted + # identifiers (doubled-quote escapes), backtick (BigQuery/MySQL) and [bracket] (SQL + # Server) quoted identifiers, AND Liquid/Jinja template segments ({{ }} / {% %}). + # Rewrite refs only in the remaining (even-index) segments so string VALUES, quoted + # identifiers, typed literals, and template variables are untouched. The template + # patterns require DOUBLE braces / brace-percent, so the single-brace {model} is safe. + parts = re.split( + r"""((?i:\b(?:date|time|timestamp|timestamptz|datetime|interval)\s+'(?:[^']|'')*')""" + r"""|'(?:[^']|'')*'|"(?:[^"]|"")*"|`[^`]*`|\[[^\]]*\]|\{\{.*?\}\}|\{%.*?%\})""", + fstr, + ) + for i in range(0, len(parts), 2): + parts[i] = ref_re.sub(_one, parts[i]) + return "".join(parts) + + return " AND ".join("(" + _resolve(f).replace("{model}", "${TABLE}") + ")" for f in filters) + + @classmethod + def _fold_filters_into_aggregate(cls, agg_sql: str, filters: list[str], model: Model) -> str | None: + """Fold ``filters`` into a single-outer-aggregate SQL expression. + + For ``SUM(${TABLE}.amount)`` + ``status='done'`` returns + ``SUM(CASE WHEN (...) THEN ${TABLE}.amount END)``. Returns ``None`` when the + expression is not exactly one outer ``FUNC(arg)`` (so the caller can fall back + rather than mangle a complex expression). + """ + m = re.match(r"^\s*(\w+)\s*\((.*)\)\s*$", agg_sql, re.S) + if not m: + return None + func, arg = m.group(1), m.group(2) + # Confirm the parens wrap the WHOLE expression (no premature close, e.g. + # "SUM(a)/COUNT(b)" must not be treated as one outer SUM(...)). Quote-aware: a paren + # inside a string literal / quoted identifier (e.g. CONCAT(a, ')')) is not syntax. + depth = 0 + quote = None + for ch in arg: + if quote is not None: + if ch == quote: + quote = None + continue + if ch in "'\"`": + quote = ch + elif ch == "(": + depth += 1 + elif ch == ")": + depth -= 1 + if depth < 0: + return None + if depth != 0: + return None + arg = arg.strip() + # The outer FUNC must itself be the aggregate. A scalar wrapper around an aggregate + # (e.g. ABS(SUM(amount))) has the aggregate in `arg`; folding would push CASE around + # the inner aggregate (ABS(CASE WHEN ... THEN SUM(amount) END)) -> wrong. Bail so the + # caller skips rather than emit invalid SQL. + from sidemantic.sql.aggregation_detection import sql_has_aggregate as _has_agg + + if _has_agg(arg): + return None + conds = cls._fold_filter_conds(filters, model) + # COUNT(*) -> COUNT(CASE WHEN ... THEN 1 END): "* " can't live inside CASE. + if arg == "*": + return f"{func}(CASE WHEN {conds} THEN 1 END)" + # COUNT(DISTINCT x) -> COUNT(DISTINCT CASE WHEN ... THEN x END): DISTINCT stays + # outside the CASE (it's part of the aggregate, not the value being filtered). Accept + # the parenthesized spelling COUNT(DISTINCT(x)) too; the lookahead requires a space or + # `(` after DISTINCT so an identifier like `DISTINCTION` is not mistaken for it. + dm = re.match(r"(?is)^DISTINCT(?=[\s(])\s*(.+)$", arg) + if dm: + distinct_arg = dm.group(1).strip() + # A multi-column DISTINCT (COUNT(DISTINCT a, b)) has no single CASE result, so + # bail and let the caller skip rather than emit malformed `THEN a, b END`. + # quote_aware: a delimited composite key COUNT(DISTINCT a || ',' || b) is ONE + # column -- the comma in the string literal must not count as a separator. + if len(cls._split_top_level_commas(distinct_arg, quote_aware=True)) > 1: + return None + return f"{func}(DISTINCT CASE WHEN {conds} THEN {distinct_arg} END)" + # A multi-argument aggregate (WEIGHTED_AVG(price, qty)) has no single CASE result, + # so bail rather than emit malformed `THEN price, qty END`. + if len(cls._split_top_level_commas(arg, quote_aware=True)) > 1: + return None + return f"{func}(CASE WHEN {conds} THEN {arg} END)" + def _export_view(self, model: Model, graph: SemanticGraph) -> dict: """Export model to LookML view definition. @@ -3050,9 +3215,12 @@ def _export_view(self, model: Model, graph: SemanticGraph) -> dict: view["dimension_groups"] = dimension_groups # Export measures + from sidemantic.sql.aggregation_detection import sql_has_aggregate as _sql_has_aggregate + measures = [] for metric in model.metrics: measure_def = {"name": metric.name} + filters_folded = False # set when filters are folded into the measure SQL # Handle different metric types if metric.type == "time_comparison": @@ -3102,23 +3270,135 @@ def _export_view(self, model: Model, graph: SemanticGraph) -> dict: if metric.numerator and metric.denominator: measure_def["sql"] = f"1.0 * ${{{metric.numerator}}} / NULLIF(${{{metric.denominator}}}, 0)" else: - # Regular aggregation measure - type_mapping = { - "count": "count", - "count_distinct": "count_distinct", - "sum": "sum", - "avg": "average", - "min": "min", - "max": "max", - } - measure_def["type"] = type_mapping.get(metric.agg, "count") - - if metric.sql: - sql = metric.sql.replace("{model}", "${TABLE}") - measure_def["sql"] = sql + # Any metric.type that reaches here (time_comparison/derived/ratio + # were handled above) is a complex type. A running_total imported from + # LookML (type=cumulative + table_calculation meta) round-trips back to + # a LookML running_total over its base measure; other complex types + # (cumulative/conversion/retention/cohort) have no LookML equivalent and + # are skipped rather than exported as a misleading plain aggregation. + if metric.type is not None: + rt_sql = (metric.sql or "").strip() + rt_is_running_total = (metric.meta or {}).get("table_calculation") == "running_total" and rt_sql + # A LookML running_total's `sql` is a SINGLE base-measure reference. + # Accept a bare measure name (-> ${name}) or a string that is EXACTLY one + # already-braced ref (an unresolved cross-view ${other.total}, passed + # through). An EXPRESSION (e.g. "${other.total} + tax" -- note the local + # ref also lost its braces) is not a valid single ref, so fall through to + # the skip-with-warning rather than emit malformed `sql: ${other.total} + tax`. + if rt_is_running_total and re.fullmatch(r"\$\{[^{}]+\}", rt_sql): + measure_def["type"] = "running_total" + measure_def["sql"] = rt_sql + elif rt_is_running_total and re.fullmatch(r"\w+", rt_sql): + measure_def["type"] = "running_total" + measure_def["sql"] = f"${{{rt_sql}}}" + else: + logger.warning( + "Metric %r (type=%r) has no LookML equivalent; skipping on export.", + metric.name, + metric.type, + ) + continue + else: + # Regular aggregation measure. + type_mapping = { + "count": "count", + "count_distinct": "count_distinct", + "sum": "sum", + "avg": "average", + "average": "average", + "min": "min", + "max": "max", + "median": "median", + } + # Aggregations Looker has no native measure type for: emit as a + # type: number with an explicit SQL aggregate. + sql_agg_funcs = { + "stddev": "STDDEV", + "stddev_pop": "STDDEV_POP", + "variance": "VAR_SAMP", + "variance_pop": "VAR_POP", + } + col_sql = metric.sql.replace("{model}", "${TABLE}") if metric.sql else None + + if metric.agg == "approx_count_distinct": + # Looker represents this as count_distinct with approximate: yes. + measure_def["type"] = "count_distinct" + measure_def["approximate"] = "yes" + if col_sql: + measure_def["sql"] = col_sql + elif metric.agg in type_mapping: + measure_def["type"] = type_mapping[metric.agg] + if col_sql: + measure_def["sql"] = col_sql + elif metric.agg in sql_agg_funcs and col_sql: + measure_def["type"] = "number" + agg_sql = f"{sql_agg_funcs[metric.agg]}({col_sql})" + if metric.filters: + # type: number re-imports as a derived metric whose generator + # does not apply LookML `filters`, so fold them into the aggregate + # here. Reuse _fold_filters_into_aggregate so DISTINCT stays OUTSIDE + # the CASE (STDDEV(DISTINCT amount) -> STDDEV(DISTINCT CASE WHEN ... + # THEN amount END), not STDDEV(CASE WHEN ... THEN DISTINCT amount END)); + # skip if the form can't be folded rather than emit invalid SQL. + folded = self._fold_filters_into_aggregate(agg_sql, metric.filters, model) + if folded is None: + logger.warning( + "Metric %r (agg=%r) has filters over an aggregate form that " + "cannot be folded for LookML export; skipping to avoid invalid SQL.", + metric.name, + metric.agg, + ) + continue + measure_def["sql"] = folded + filters_folded = True + else: + measure_def["sql"] = agg_sql + elif metric.agg is None and col_sql and _sql_has_aggregate(metric.sql or ""): + # An agg-less measure whose SQL is itself an aggregate (a complete + # SUM({model}.amount) imported from Cube, or an inline aggregate + # expression). Faithfully maps to a LookML type: number with the + # aggregate SQL. type: number re-imports as a derived metric that + # does NOT apply a separate `filters` block, so any filters must be + # folded into the aggregate; if the expression isn't a single + # foldable FUNC(arg), skip rather than emit a silently-unfiltered + # measure. + if not metric.filters and re.fullmatch(r"(?i)count\s*\(\s*(?:\*|\d+)\s*\)", col_sql.strip()): + # A bare row count -- COUNT(*), COUNT(1), COUNT(0), incl. spaced + # COUNT (*) -- references + # no column; a type: number would re-import as a derived metric + # over an empty CTE (SELECT FROM ...), which the compiler rejects. + # LookML's native type: count counts rows and round-trips cleanly. + measure_def["type"] = "count" + else: + measure_def["type"] = "number" + if metric.filters: + folded = self._fold_filters_into_aggregate(col_sql, metric.filters, model) + if folded is None: + logger.warning( + "Metric %r has filters over a complex aggregate SQL expression that " + "cannot be folded for LookML export; skipping to avoid an unfiltered measure.", + metric.name, + ) + continue + measure_def["sql"] = folded + filters_folded = True + else: + measure_def["sql"] = col_sql + else: + # agg=None over a NON-aggregate SQL (a plain row-level column / + # string / yesno measure), an unknown aggregation, or an opaque + # complete *column* expression: Looker measures aggregate, so there + # is no faithful measure form. Skip with a warning rather than + # forcing a misleading type: number that crashes on re-import. + logger.warning( + "Metric %r (agg=%r) has no LookML equivalent; skipping on export.", + metric.name, + metric.agg, + ) + continue - # Add filters (skip for time_comparison as they don't use filters) - if metric.filters and metric.type != "time_comparison": + # Add filters (skip for time_comparison; skip when already folded into SQL) + if metric.filters and metric.type != "time_comparison" and not filters_folded: filters_all = [] for filter_str in metric.filters: # Parse SQL-format filters back to LookML format diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 48639621..4b2e3259 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4009,5 +4009,1183 @@ def test_lookml_self_referential_dimension_terminates(): assert selfd.sql.count("+ 1") <= 2 +def test_lookml_export_unmapped_aggregations_not_count(): + """Unmapped aggregations / complex types must not be silently exported as COUNT.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + model = Model( + name="orders", + table="orders", + primary_key="id", + dimensions=[Dimension(name="id", type="numeric", sql="id")], + metrics=[ + Metric(name="med", agg="median", sql="amount"), + Metric(name="sd", agg="stddev", sql="amount"), + Metric(name="va", agg="variance", sql="amount"), + Metric(name="cnt", agg="count"), + Metric(name="sm", agg="sum", sql="amount"), + Metric(name="cum", type="cumulative", sql="amount"), + ], + ) + graph = SemanticGraph() + graph.add_model(model) + + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + + # median has a native Looker type + assert "type: median" in text + # stddev/variance become type: number with an explicit SQL aggregate + assert "STDDEV(" in text + assert "VAR_SAMP(" in text + # the complex cumulative metric is skipped (no LookML equivalent), not COUNT + assert "measure: cum" not in text + # genuine count/sum unchanged + assert "type: count" in text + assert "type: sum" in text + + # Round-trip: median survives, none of these come back as a plain count corruption + reimported = {m.name: m for m in LookMLAdapter().parse(Path(out)).get_model("orders").metrics} + assert reimported["med"].agg == "median" + assert "cum" not in reimported # skipped on export + + +def test_lookml_export_complex_type_with_agg_skipped(): + """A complex-type metric carrying an agg (e.g. cumulative rolling avg) must be skipped, not exported as a plain measure.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="o", + primary_key="id", + dimensions=[Dimension(name="id", type="numeric", sql="id")], + metrics=[Metric(name="roll", type="cumulative", agg="avg", sql="amount")], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "measure: roll" not in text + assert "type: average" not in text # not silently downgraded to a plain average + + +def test_lookml_export_filtered_distinct_stddev_keeps_distinct_outside_case(): + """A filtered DISTINCT stddev/variance must keep DISTINCT OUTSIDE the folded CASE.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="status"), + Dimension(name="amount", type="numeric", sql="amount"), + ], + metrics=[ + Metric(name="sd", agg="stddev", sql="DISTINCT {model}.amount", filters=["{model}.status = 'done'"]) + ], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "measure: sd" in text + assert "STDDEV(DISTINCT CASE WHEN" in text # DISTINCT outside the CASE + assert "THEN DISTINCT" not in text # never DISTINCT inside the CASE (invalid SQL) + + +def test_lookml_export_filtered_sql_aggregate_folds_filter(): + """A filtered stddev/variance (type: number path) must fold the filter into SQL, not drop it.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="o", + primary_key="id", + dimensions=[Dimension(name="id", type="numeric", sql="id")], + metrics=[Metric(name="sd", agg="stddev", sql="amount", filters=["{model}.status = 'done'"])], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + # The single measure folds its filter into the SQL aggregate (the model-qualified + # ref is resolved to a ${TABLE}-qualified column)... + assert "STDDEV(CASE WHEN" in text + assert "(${TABLE}.status) = 'done'" in text + # ...and is not also emitted as a (non-applied) LookML filters block. + assert "filters" not in text + + +def test_lookml_approximate_distinct_preserved_in_post_sql_measure(): + """A post-SQL measure (percent_of_total) over an approximate count_distinct must stay approximate.""" + graph = _parse_lkml( + """ +view: v { + sql_table_name: t ;; + dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } + measure: uu { type: count_distinct sql: ${TABLE}.user_id ;; approximate: yes } + measure: pct { type: percent_of_total sql: ${uu} ;; } +} +""" + ) + pct = graph.get_model("v").get_metric("pct") + assert "APPROX_COUNT_DISTINCT" in pct.sql + assert "COUNT(DISTINCT" not in pct.sql + + +def test_lookml_export_running_total_roundtrips(): + """An imported running_total (cumulative + table_calculation meta) round-trips, not dropped.""" + import tempfile + + graph = _parse_lkml( + """ +view: v { + sql_table_name: t ;; + dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } + measure: total { type: sum sql: ${TABLE}.amt ;; } + measure: rt { type: running_total sql: ${total} ;; } +} +""" + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + assert "type: running_total" in open(out).read() + reimported = {m.name: m for m in LookMLAdapter().parse(Path(out)).get_model("v").metrics} + assert "rt" in reimported + assert (reimported["rt"].meta or {}).get("table_calculation") == "running_total" + + +def test_lookml_export_approximate_distinct_preserved(): + """approx_count_distinct must export as count_distinct + approximate: yes and round-trip.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="o", + primary_key="id", + dimensions=[Dimension(name="id", type="numeric", sql="id")], + metrics=[Metric(name="uu", agg="approx_count_distinct", sql="user_id")], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "type: count_distinct" in text + assert "approximate: yes" in text + reimported = {m.name: m for m in LookMLAdapter().parse(Path(out)).get_model("o").metrics} + assert reimported["uu"].agg == "approx_count_distinct" + + +def test_lookml_export_opaque_complete_sql_measure_skipped(): + """An agg-less sql_is_complete measure has no faithful LookML form and is skipped, not exported as broken derived.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="o", + primary_key="id", + dimensions=[Dimension(name="id", type="numeric", sql="id")], + metrics=[Metric(name="opaque", agg=None, sql="status_label", sql_is_complete=True)], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + assert "measure: opaque" not in open(out).read() + + +def test_lookml_export_folded_filter_resolves_dimension_sql(): + """A folded filter must reference the dimension's SQL column, not the bare dimension name.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="o", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="order_status"), + ], + metrics=[Metric(name="sd", agg="stddev", sql="amount", filters=["{model}.status = 'done'"])], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + # Resolved dimension column is qualified (${TABLE}.) and parenthesized. + assert "(${TABLE}.order_status) = 'done'" in open(out).read() + + +def test_lookml_export_folded_filter_resolves_model_qualified_ref(): + """A folded filter qualified by the model's own NAME (orders.status) resolves too.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="raw_orders", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="order_status"), + ], + metrics=[Metric(name="sd", agg="stddev", sql="amount", filters=["orders.status = 'done'"])], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "(${TABLE}.order_status) = 'done'" in text + assert "orders.status" not in text # model-name prefix was normalized away + + +def test_lookml_export_complete_aggregate_sql_measure_not_dropped(): + """An opaque COMPLETE aggregate measure (e.g. from Cube) exports as type: number, not dropped.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="t", + primary_key="id", + dimensions=[Dimension(name="id", type="numeric", sql="id")], + metrics=[Metric(name="total_amt", agg=None, sql="SUM({model}.amount)", sql_is_complete=True)], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "measure: total_amt" in text # not dropped + assert "type: number" in text + assert "SUM(${TABLE}.amount)" in text + + +def test_lookml_export_string_measure_not_forced_to_number(): + """A non-aggregate (string/yesno/row-level) agg-less measure must NOT export as number. + + Looker measures aggregate; forcing a raw column measure to type: number re-imports as + a derived metric and crashes ({model}.status read as a metric dep), so it is skipped. + """ + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="t", + primary_key="id", + dimensions=[Dimension(name="id", type="numeric", sql="id")], + metrics=[Metric(name="label", agg=None, sql="{model}.status")], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + assert "measure: label" not in open(out).read() # skipped, not exported as type: number + + +def test_lookml_export_complete_aggregate_with_filters_folds_into_aggregate(): + """A complete aggregate measure WITH filters folds them into the aggregate (not a dropped filter).""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="order_status"), + ], + metrics=[ + Metric( + name="done_amt", + agg=None, + sql="SUM({model}.amount)", + sql_is_complete=True, + filters=["{model}.status = 'done'"], + ) + ], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + # Filter folded INSIDE the aggregate, and no separate (ignored-on-reimport) filters block. + assert "SUM(CASE WHEN" in text + assert "(${TABLE}.order_status) = 'done'" in text + measure_block = text[text.index("measure: done_amt") :] + measure_block = measure_block[: measure_block.index("}")] + assert "filters:" not in measure_block + + +def test_lookml_export_folded_filter_resolves_compact_dimension(): + """A folded filter over a COMPACT (no-sql) dimension resolves to ${TABLE}.col, not .col.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="raw_orders", + primary_key="id", + dimensions=[Dimension(name="id", type="numeric", sql="id"), Dimension(name="status", type="categorical")], + metrics=[Metric(name="sd", agg="stddev", sql="amount", filters=["orders.status = 'done'"])], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "(${TABLE}.status) = 'done'" in text # default column, qualified to ${TABLE} + assert "orders.status" not in text # not left pointing at a literal `orders` table + + +def test_lookml_export_folded_filter_resolves_unqualified_dimension(): + """An UNqualified folded filter field (status = 'done') resolves through dimension SQL.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="raw_orders", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="order_status"), + ], + metrics=[Metric(name="sd", agg="stddev", sql="amount", filters=["status = 'done'"])], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "(${TABLE}.order_status) = 'done'" in text # unqualified dim -> its real column + # 'done' (a string VALUE) must NOT be rewritten even though it's not a dimension. + assert "'done'" in text + + +def test_lookml_export_folded_filter_resolves_dimension_inside_function(): + """An unqualified dimension INSIDE a function (LOWER(status)) resolves; quoted value is safe.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="raw_orders", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="order_status"), + ], + # LOWER(status): dim inside a function; and a quoted value equal to a dim name. + metrics=[Metric(name="sd", agg="stddev", sql="amount", filters=["LOWER(status) = 'status'"])], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "LOWER((${TABLE}.order_status))" in text # dim resolved inside the function + assert "= 'status'" in text # the quoted value was NOT rewritten + + +def test_lookml_export_folded_filter_leaves_quoted_identifier_untouched(): + """A double-quoted identifier in a folded filter must not be substituted inside.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="raw_orders", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="order_status"), + ], + metrics=[Metric(name="sd", agg="stddev", sql="amount", filters=["\"status\" = 'done'"])], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "\"status\" = 'done'" in text # quoted identifier passes through verbatim + assert '"(${TABLE}' not in text # NOT substituted inside the quotes (invalid SQL) + + +def test_lookml_export_folded_filter_leaves_foreign_qualified_field_untouched(): + """A foreign-qualified field (customers.status) must not have its `status` part rewritten.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="raw_orders", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="order_status"), + ], + metrics=[Metric(name="sd", agg="stddev", sql="amount", filters=["customers.status = 'vip'"])], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "customers.status = 'vip'" in text # foreign qualifier left intact + assert "customers.(" not in text # the `status` part is NOT rewritten into a malformed ref + + +def test_lookml_export_folded_filter_does_not_rewrite_function_name(): + """A folded filter's SQL function name equal to a dimension name must not be rewritten.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="raw_orders", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="date", type="time", granularity="day", sql="order_date"), + ], + metrics=[Metric(name="sd", agg="stddev", sql="amount", filters=["date(created_at) = '2024-01-01'"])], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "date(created_at)" in text # function call left intact + assert "order_date)(" not in text # the function name is NOT rewritten to a column + + +def test_lookml_export_folded_filter_does_not_rewrite_template_variable(): + """A folded filter's Liquid/Jinja template variable equal to a dimension name is untouched.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="order_status"), # dim named 'status' + Dimension(name="amount", type="numeric", sql="amount"), + ], + metrics=[Metric(name="sd", agg="stddev", sql="{model}.amount", filters=["{model}.status = {{ status }}"])], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "measure: sd" in text + assert "{{ status }}" in text # template variable left intact + assert "{{ (${TABLE}" not in text # NOT rewritten inside the template + assert "order_status" in text # the real column operand IS resolved + + +def test_lookml_export_folded_filter_no_dimensions_does_not_crash(): + """Folding a qualified filter on a model with NO dimensions must not IndexError. + + With no declared dimensions the bare-name alternative is absent from the regex (one group), + so the callback must read group 2 defensively instead of raising 'no such group'. + """ + import tempfile + + from sidemantic import Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="t", + primary_key="id", + dimensions=[], # no dimensions -> names_alt empty -> single-group regex + metrics=[Metric(name="sd", agg="stddev", sql="{model}.amount", filters=["{model}.status = 'done'"])], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) # must not raise IndexError + text = open(out).read() + assert "measure: sd" in text + assert "${TABLE}.status" in text # qualified filter still folded + + +def test_lookml_export_folded_filter_does_not_rewrite_typed_date_literal(): + """A folded filter's typed date literal (`date '...'`) must not be rewritten as a column.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="raw_orders", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="date", type="time", granularity="day", sql="order_date"), # dim named 'date' + Dimension(name="amount", type="numeric", sql="amount"), + ], + metrics=[ + Metric(name="sd", agg="stddev", sql="{model}.amount", filters=["created_at >= date '2024-01-01'"]) + ], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "measure: sd" in text + assert "date '2024-01-01'" in text # typed literal left intact + assert "order_date) '2024" not in text # NOT rewritten into a column + + +def test_lookml_export_folded_filter_does_not_rewrite_cast_type(): + """A folded filter's SQL CAST type token equal to a dimension name must not be rewritten.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="raw_orders", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="date", type="time", granularity="day", sql="order_date"), # dim named 'date' + Dimension(name="amount", type="numeric", sql="amount"), + ], + metrics=[ + Metric( + name="sd", agg="stddev", sql="{model}.amount", filters=["CAST(created_at AS date) = '2024-01-01'"] + ) + ], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "measure: sd" in text # not skipped + assert "CAST(created_at AS date)" in text # type token intact + assert "AS (${TABLE}" not in text # the cast type is NOT rewritten to a column + + +def test_lookml_export_scalar_wrapped_aggregate_filter_skipped(): + """A scalar-wrapped aggregate with filters (ABS(SUM(x))) can't fold -> skipped, not mangled.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="status"), + ], + metrics=[ + Metric( + name="aw", + agg=None, + sql="ABS(SUM({model}.amount))", + sql_is_complete=True, + filters=["{model}.status = 'x'"], + ) + ], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "measure: aw" not in text # can't fold CASE around the inner aggregate -> skipped + assert "ABS(CASE WHEN" not in text # never push CASE around a nested aggregate + + +def test_lookml_export_multi_column_distinct_filter_skipped(): + """A multi-column COUNT(DISTINCT a, b) with filters can't fold to one CASE -> skipped, not malformed.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="status"), + ], + metrics=[ + Metric( + name="du", + agg=None, + sql="COUNT(DISTINCT {model}.a, {model}.b)", + sql_is_complete=True, + filters=["{model}.status = 'x'"], + ) + ], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + assert "measure: du" not in open(out).read() # skipped, no malformed `THEN a, b END` + + +def test_lookml_export_folded_filter_leaves_backtick_identifier_untouched(): + """A folded filter's backtick/bracket-quoted identifier must not be rewritten inside the quotes.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="order_status"), # dim named 'status' + Dimension(name="amount", type="numeric", sql="amount"), + ], + metrics=[Metric(name="sd", agg="stddev", sql="{model}.amount", filters=["`status` = 'done'"])], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "measure: sd" in text + assert "`status`" in text # backtick identifier intact + assert "`(${TABLE}" not in text # not rewritten inside the backticks + + +def test_lookml_export_string_literal_paren_in_aggregate_arg_folds(): + """A valid aggregate whose arg has a paren inside a STRING LITERAL must fold, not be rejected.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="status"), + ], + metrics=[ + Metric( + name="du", + agg=None, + sql="COUNT(DISTINCT CONCAT({model}.a, ')'))", # literal ')' must not break depth scan + sql_is_complete=True, + filters=["{model}.status = 'x'"], + ) + ], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "measure: du" in text # folded, not skipped + assert "DISTINCT CASE WHEN" in text + + +def test_lookml_export_parenthesized_distinct_filter_folds(): + """COUNT(DISTINCT(x)) (parenthesized, no space) must fold its filter, not emit malformed SQL.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="status"), + ], + metrics=[ + Metric( + name="du", + agg=None, + sql="COUNT(DISTINCT({model}.uid))", + sql_is_complete=True, + filters=["{model}.status = 'x'"], + ) + ], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "measure: du" in text + assert "DISTINCT CASE WHEN" in text # folded as DISTINCT over a CASE + assert "THEN DISTINCT(" not in text # NOT the malformed generic-wrapper output + + +def test_lookml_export_delimited_distinct_filter_folds_not_skipped(): + """A single-arg DISTINCT containing a comma STRING LITERAL must fold, not be mis-rejected. + + COUNT(DISTINCT a || ',' || b) is one column; the arity check must ignore the comma + inside the string literal (quote-aware split) instead of treating it as multi-column. + """ + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="status"), + ], + metrics=[ + Metric( + name="composite_distinct", + agg=None, + sql="COUNT(DISTINCT {model}.a || ',' || {model}.b)", + sql_is_complete=True, + filters=["{model}.status = 'x'"], + ) + ], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + txt = open(out).read() + assert "measure: composite_distinct" in txt # folded, NOT skipped + assert "DISTINCT CASE WHEN" in txt # filter folded inside the single-column DISTINCT + + +def test_lookml_export_multi_arg_aggregate_filter_skipped(): + """A multi-argument aggregate WEIGHTED_AVG(price, qty) with filters skips, not malformed CASE.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="status"), + ], + metrics=[ + Metric( + name="wavg", + agg=None, + sql="WEIGHTED_AVG({model}.price, {model}.qty)", + sql_is_complete=True, + filters=["{model}.status = 'x'"], + ) + ], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + assert "measure: wavg" not in open(out).read() # skipped, no malformed `THEN price, qty END` + + +def test_lookml_export_count_constant_uses_native_count_type(): + """COUNT(1) / COUNT(0) row-count aggregates export as native type: count, like COUNT(*).""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + for expr in ("COUNT(1)", "COUNT(0)"): + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="t", + primary_key="id", + dimensions=[Dimension(name="id", type="numeric", sql="id")], + metrics=[Metric(name="c", agg=None, sql=expr, sql_is_complete=True)], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "type: count" in text and expr not in text + + +def test_lookml_export_spaced_count_star_maps_to_native_count(): + """A spaced `COUNT (*)` complete aggregate must still export as native type: count.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="t", + primary_key="id", + dimensions=[Dimension(name="id", type="numeric", sql="id")], + metrics=[Metric(name="c", agg=None, sql="COUNT (*)", sql_is_complete=True)], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + import re + + block = re.search(r"measure: c \{.*?\}", open(out).read(), re.S) + assert block and "type: count" in block.group(0) # native count, not a number over empty CTE + + +def test_lookml_export_count_star_and_distinct_filters_fold_validly(): + """COUNT(*) / COUNT(DISTINCT x) complete aggregates with filters fold to valid SQL that runs.""" + import tempfile + + import duckdb + + from sidemantic import Dimension, Metric, Model, SemanticLayer + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="orders", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="order_status"), + ], + metrics=[ + Metric(name="cnt", agg=None, sql="COUNT(*)", sql_is_complete=True, filters=["{model}.status = 'done'"]), + Metric( + name="du", + agg=None, + sql="COUNT(DISTINCT {model}.user_id)", + sql_is_complete=True, + filters=["{model}.status = 'done'"], + ), + ], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "COUNT(CASE WHEN" in text and "THEN 1 END)" in text # COUNT(*) -> THEN 1 + assert "COUNT(DISTINCT CASE WHEN" in text # DISTINCT stays outside the CASE + + layer = SemanticLayer(auto_register=False) + for m in LookMLAdapter().parse(Path(out)).models.values(): + layer.add_model(m) + con = duckdb.connect() + con.execute("create table orders(id int, user_id int, order_status text)") + con.execute("insert into orders values (1,7,'done'),(2,7,'open'),(3,8,'done')") + assert con.execute(layer.compile(metrics=["orders.cnt"])).fetchall() == [(2,)] + assert con.execute(layer.compile(metrics=["orders.du"])).fetchall() == [(2,)] + + +def test_lookml_export_bare_count_star_uses_native_count_type(): + """A bare COUNT(*) complete aggregate exports as native type: count (round-trips + runs). + + type: number would re-import as a derived metric over an empty CTE (SELECT FROM ...), + which the compiler rejects; native type: count counts rows and round-trips cleanly. + """ + import tempfile + + import duckdb + + from sidemantic import Dimension, Metric, Model, SemanticLayer + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="orders", + primary_key="id", + dimensions=[Dimension(name="id", type="numeric", sql="id")], + metrics=[Metric(name="cnt", agg=None, sql="COUNT(*)", sql_is_complete=True)], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + assert "type: count" in open(out).read() + reimported = LookMLAdapter().parse(Path(out)) + assert reimported.get_model("orders").get_metric("cnt").agg == "count" + layer = SemanticLayer(auto_register=False) + for m in reimported.models.values(): + layer.add_model(m) + con = duckdb.connect() + con.execute("create table orders as select 1 id union all select 2") + assert con.execute(layer.compile(metrics=["orders.cnt"])).fetchall() == [(2,)] + + +def test_lookml_export_running_total_cross_view_ref_not_double_wrapped(): + """A running_total over an unsupported (already-braced) cross-view ref must not emit ${${...}}.""" + import tempfile + + graph = _parse_lkml( + """ +view: orders { + sql_table_name: t ;; + dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } + measure: rt { type: running_total sql: ${other_view.total} ;; } +} +""" + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "${${" not in text + assert "sql: ${other_view.total}" in text + + +def test_lookml_export_running_total_braced_ref_plus_expression_skipped(): + """A running_total whose sql is a braced cross-view ref PLUS more must be skipped, not exported. + + `${other.total} + tax` contains `${` so a substring check would wrongly accept it and emit + a malformed `sql: ${other.total} + tax` (the local `tax` ref already lost its braces). Only + a string that is EXACTLY one `${...}` reference may pass through. + """ + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="t", + primary_key="id", + dimensions=[Dimension(name="id", type="numeric", sql="id")], + metrics=[ + Metric( + name="rt", + type="cumulative", + sql="${other.total} + tax", + meta={"table_calculation": "running_total"}, + ) + ], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "measure: rt" not in text # not a single ref -> skipped + assert "${other.total} + tax" not in text # never emit the malformed mixed expression + + +def test_lookml_export_folded_filter_does_not_rewrite_schema_qualified_ref(): + """A folded filter's schema-qualified own-model ref must not match the model-name suffix.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="raw_orders", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="order_status"), + Dimension(name="amount", type="numeric", sql="amount"), + ], + metrics=[Metric(name="sd", agg="stddev", sql="{model}.amount", filters=["schema.orders.status = 'done'"])], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "measure: sd" in text + assert "schema.orders.status" in text # schema-qualified ref left intact + assert "schema.(${TABLE}" not in text # not mangled into a column substitution + + +def test_lookml_export_running_total_expression_skipped(): + """A running_total over an EXPRESSION (not a single base measure ref) is skipped, not malformed.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="t", + primary_key="id", + dimensions=[Dimension(name="id", type="numeric", sql="id")], + metrics=[ + Metric(name="total", agg="sum", sql="{model}.amt"), + Metric(name="rt", type="cumulative", sql="total + tax", meta={"table_calculation": "running_total"}), + ], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "measure: rt" not in text # expression isn't a valid running_total base -> skipped + assert "${total + tax}" not in text # never emit a malformed field reference + + +def test_lookml_export_multiple_folded_filters_parenthesized(): + """Each folded filter is parenthesized so a filter containing OR isn't broken by AND precedence.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="o", + primary_key="id", + dimensions=[Dimension(name=n, type="numeric", sql=n) for n in ("id", "a", "b", "c")], + metrics=[ + Metric( + name="sd", agg="stddev", sql="amount", filters=["{model}.a = 1 OR {model}.b = 1", "{model}.c = 1"] + ) + ], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + # The OR filter is grouped before being AND-joined with the second filter. + assert "((${TABLE}.a) = 1 OR (${TABLE}.b) = 1) AND ((${TABLE}.c) = 1)" in text + + +def test_lookml_export_folded_filter_parenthesizes_expression_dimension(): + """A folded filter on a dimension whose SQL is an expression must be parenthesized.""" + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="o", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="eligible", type="categorical", sql="{model}.amount > 10 OR {model}.special"), + ], + metrics=[Metric(name="sd", agg="stddev", sql="amount", filters=["{model}.eligible = false"])], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + assert "(${TABLE}.amount > 10 OR ${TABLE}.special) = false" in open(out).read() + + if __name__ == "__main__": pytest.main([__file__, "-v"]) From 7b52d6485294d3c6a25382d8150b33cc615e37b2 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Fri, 10 Jul 2026 09:21:10 -0700 Subject: [PATCH 02/44] Handle constant-count exports and protect date-part keywords in folded filters Two LookML export bugs producing invalid re-import SQL: - COUNT over a non-null constant other than *//digit (COUNT(TRUE), COUNT('x'), COUNT(1.0)) fell through to type: number and re-imported as a zero-column complete-SQL metric, whose query hits an empty model CTE (SELECT FROM ...). Classify every non-null constant COUNT as a native row count (type: count); COUNT(NULL) is excluded since it is always 0. - A folded-filter bare-name resolver rewrote date-part / interval-unit keywords that match a dimension name -- e.g. EXTRACT(day FROM ...) or INTERVAL 7 day on a model with a 'day' dimension -- into the dimension SQL, emitting invalid LookML like EXTRACT((${TABLE}.order_day) FROM ...). Protect the EXTRACT-part, extract-FROM, and INTERVAL-unit positions. Context checks now use absolute offsets into the full predicate so a split literal (INTERVAL '7' day) is handled. --- sidemantic/adapters/lookml.py | 78 ++++++++++++++++-------- tests/adapters/lookml/test_edge_cases.py | 57 +++++++++++++++-- 2 files changed, 107 insertions(+), 28 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index b507ecfb..9b6ff9fb 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -2983,22 +2983,44 @@ def _qualify(val: str) -> str: ref_re = re.compile(pattern) def _resolve(fstr: str) -> str: - def _one(m): - # The bare-dimension alternative (group 2) only exists when names_alt is - # non-empty; with no declared dimensions the pattern has a single group, so - # read group 2 defensively (m.group(2) would raise IndexError otherwise). - bare = m.group(2) if m.re.groups >= 2 else None - if bare is not None: - # Bare dimension-name alternative: skip when it sits in a SQL TYPE context - # (a cast target), not a column operand -- e.g. CAST(x AS date) or x::date - # with a `date` dimension. Rewriting the type token to a column would emit - # invalid SQL like CAST(x AS (${TABLE}.order_date)). (Typed literals like - # `date '2024-01-01'` are protected earlier, in the split below.) - pre = m.string[: m.start()] - if re.search(r"(?is)\bAS\s+$", pre) or pre.rstrip().endswith("::"): - return m.group(0) - name = m.group(1) or bare - return _qualify(dim_sql.get(name, name)) + def _make_one(base: int): + # `base` is this segment's absolute offset into fstr, so context checks can look + # at the FULL predicate -- not just the current split segment. A quoted number + # (`INTERVAL '7' day`) splits `day` into its own segment, so a segment-local `pre` + # would miss the leading INTERVAL and wrongly rewrite the unit keyword. + def _one(m): + # The bare-dimension alternative (group 2) only exists when names_alt is + # non-empty; with no declared dimensions the pattern has a single group, so + # read group 2 defensively (m.group(2) would raise IndexError otherwise). + bare = m.group(2) if m.re.groups >= 2 else None + if bare is not None: + # Bare dimension-name alternative: skip when it sits in a SQL TYPE context + # (a cast target), not a column operand -- e.g. CAST(x AS date) or x::date + # with a `date` dimension. Rewriting the type token to a column would emit + # invalid SQL like CAST(x AS (${TABLE}.order_date)). (Typed literals like + # `date '2024-01-01'` are protected earlier, in the split below.) + pre = fstr[: base + m.start()] + if re.search(r"(?is)\bAS\s+$", pre) or pre.rstrip().endswith("::"): + return m.group(0) + # Skip a bare token in a DATE-PART / INTERVAL-UNIT keyword position, not a + # column operand -- e.g. EXTRACT(day FROM ...) or `INTERVAL 7 day` on a + # model with a `day` dimension. Rewriting the keyword to the dimension SQL + # emits invalid SQL like EXTRACT((${TABLE}.order_day) FROM ...). Positions: + # right after EXTRACT(, immediately before an extract's FROM, or as the unit + # following an INTERVAL . (Quoted forms like + # DATE_TRUNC('day', ...) and INTERVAL '7 day' are already protected as + # string literals.) + suf = fstr[base + m.end() :] + if ( + re.search(r"(?i)\bextract\s*\(\s*$", pre) + or re.match(r"(?is)\s+from\b", suf) + or re.search(r"(?i)\binterval\s+(?:[+-]?\d+(?:\.\d+)?|'(?:[^']|'')*')\s*$", pre) + ): + return m.group(0) + name = m.group(1) or bare + return _qualify(dim_sql.get(name, name)) + + return _one # Split out (and thus protect from rewriting) SQL TYPED LITERALS whose type keyword # equals a dimension name (`date '2024-01-01'`, `timestamp '...'`, `interval '...'`) @@ -3014,8 +3036,11 @@ def _one(m): r"""|'(?:[^']|'')*'|"(?:[^"]|"")*"|`[^`]*`|\[[^\]]*\]|\{\{.*?\}\}|\{%.*?%\})""", fstr, ) - for i in range(0, len(parts), 2): - parts[i] = ref_re.sub(_one, parts[i]) + offset = 0 + for i, part in enumerate(parts): + if i % 2 == 0: + parts[i] = ref_re.sub(_make_one(offset), part) + offset += len(part) return "".join(parts) return " AND ".join("(" + _resolve(f).replace("{model}", "${TABLE}") + ")" for f in filters) @@ -3362,12 +3387,17 @@ def _export_view(self, model: Model, graph: SemanticGraph) -> dict: # folded into the aggregate; if the expression isn't a single # foldable FUNC(arg), skip rather than emit a silently-unfiltered # measure. - if not metric.filters and re.fullmatch(r"(?i)count\s*\(\s*(?:\*|\d+)\s*\)", col_sql.strip()): - # A bare row count -- COUNT(*), COUNT(1), COUNT(0), incl. spaced - # COUNT (*) -- references - # no column; a type: number would re-import as a derived metric - # over an empty CTE (SELECT FROM ...), which the compiler rejects. - # LookML's native type: count counts rows and round-trips cleanly. + # A COUNT over any NON-NULL constant counts every row -- it is a native + # row count, identical to type: count: `*`, an int/decimal (1, 0, 1.0, + # .5), a boolean (TRUE/FALSE), or a string literal ('x'). COUNT(NULL) is + # deliberately excluded -- it is always 0, not a row count. + _count_const = r"\*|[+-]?(?:\d+\.?\d*|\.\d+)|true|false|'(?:[^']|'')*'" + if not metric.filters and re.fullmatch( + rf"(?i)count\s*\(\s*(?:{_count_const})\s*\)", col_sql.strip() + ): + # These reference no column; a type: number would re-import as a + # derived metric over an empty CTE (SELECT FROM ...), which the + # compiler rejects. Native type: count round-trips cleanly. measure_def["type"] = "count" else: measure_def["type"] = "number" diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 4b2e3259..7086b27c 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4650,6 +4650,41 @@ def test_lookml_export_folded_filter_does_not_rewrite_cast_type(): assert "AS (${TABLE}" not in text # the cast type is NOT rewritten to a column +def test_lookml_export_folded_filter_does_not_rewrite_date_part_keyword(): + """A folded filter's date-part / interval-unit keyword equal to a dimension must not be rewritten. + + With a `day` dimension, EXTRACT(day FROM ...) and INTERVAL 7 day contain the SQL keyword `day` + in a non-column position. Rewriting it to the dimension SQL emits invalid LookML such as + EXTRACT((${TABLE}.order_day) FROM ...); the keyword must be protected while genuine column uses + (and the extract SOURCE column) are still rewritten. + """ + from sidemantic import Dimension, Model + + model = Model( + name="orders", + table="raw_orders", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="day", type="time", granularity="day", sql="order_day"), # dim named 'day' + Dimension(name="created_at", type="time", granularity="day", sql="created_at"), + ], + ) + conds = LookMLAdapter._fold_filter_conds + # EXTRACT part keyword protected; the FROM source column IS rewritten. + assert conds(["EXTRACT(day FROM created_at) = 1"], model) == "(EXTRACT(day FROM (${TABLE}.created_at)) = 1)" + # INTERVAL unit keyword protected -- both bare and quoted-number spellings. + assert conds(["created_at >= CURRENT_DATE - INTERVAL 7 day"], model) == ( + "((${TABLE}.created_at) >= CURRENT_DATE - INTERVAL 7 day)" + ) + assert conds(["created_at >= CURRENT_DATE - INTERVAL '7' day"], model) == ( + "((${TABLE}.created_at) >= CURRENT_DATE - INTERVAL '7' day)" + ) + # A genuine column use of the same name IS still rewritten (not over-protected). + assert conds(["day = '2024-01-01'"], model) == "((${TABLE}.order_day) = '2024-01-01')" + assert conds(["LOWER(day) = 'x'"], model) == "(LOWER((${TABLE}.order_day)) = 'x')" + + def test_lookml_export_scalar_wrapped_aggregate_filter_skipped(): """A scalar-wrapped aggregate with filters (ABS(SUM(x))) can't fold -> skipped, not mangled.""" import tempfile @@ -4891,13 +4926,20 @@ def test_lookml_export_multi_arg_aggregate_filter_skipped(): def test_lookml_export_count_constant_uses_native_count_type(): - """COUNT(1) / COUNT(0) row-count aggregates export as native type: count, like COUNT(*).""" + """COUNT over any NON-NULL constant is a native row count and exports as type: count. + + COUNT(1)/COUNT(0), plus COUNT(TRUE), COUNT('x'), COUNT(1.0), COUNT(.5) all count every row. + Exporting them as type: number would re-import as a zero-column complete-SQL metric whose + query hits an empty model CTE (SELECT FROM ...). COUNT(NULL) is NOT a row count (always 0), so + it must stay a type: number. + """ + import re import tempfile from sidemantic import Dimension, Metric, Model from sidemantic.core.semantic_graph import SemanticGraph - for expr in ("COUNT(1)", "COUNT(0)"): + def export_measure(expr): graph = SemanticGraph() graph.add_model( Model( @@ -4910,8 +4952,15 @@ def test_lookml_export_count_constant_uses_native_count_type(): ) out = tempfile.mktemp(suffix=".lkml") LookMLAdapter().export(graph, out) - text = open(out).read() - assert "type: count" in text and expr not in text + return re.search(r"measure: c \{.*?\n \}", open(out).read(), re.S).group(0) + + for expr in ("COUNT(1)", "COUNT(0)", "COUNT(TRUE)", "COUNT('x')", "COUNT(1.0)", "COUNT(.5)"): + block = export_measure(expr) + assert "type: count" in block and expr not in block, f"{expr} -> {block}" + + # COUNT(NULL) is always 0, not a row count: it must NOT be flattened to type: count. + null_block = export_measure("COUNT(NULL)") + assert "type: number" in null_block and "COUNT(NULL)" in null_block def test_lookml_export_spaced_count_star_maps_to_native_count(): From 4afb5082424d76243035ef0c947e565022b44c46 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Fri, 10 Jul 2026 10:12:39 -0700 Subject: [PATCH 03/44] Skip zero-column aggregates on LookML export instead of emitting broken type:number The constant-count guard only diverted non-null COUNT constants. Other zero-column aggregates -- COUNT(NULL), COUNT(DISTINCT 1), SUM(1), MAX('x') -- still exported as type: number with SQL that references no column, so re-importing built an opaque complete-SQL metric with an empty referenced-column set and compiling it produced an empty model CTE (SELECT ... FROM with no select list). Skip an UNFILTERED aggregate whose SQL references no column, with a warning. A FILTERED zero-column aggregate still folds (COUNT(*) with a filter -> COUNT(CASE WHEN ... THEN 1 END)), which references the filter's columns and runs. --- sidemantic/adapters/lookml.py | 35 ++++++++++++++++++++++++ tests/adapters/lookml/test_edge_cases.py | 16 ++++++----- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 9b6ff9fb..da13fe4d 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -3045,6 +3045,25 @@ def _one(m): return " AND ".join("(" + _resolve(f).replace("{model}", "${TABLE}") + ")" for f in filters) + @staticmethod + def _aggregate_references_column(sql: str) -> bool: + """True if ``sql`` references at least one COLUMN (not just constants/functions). + + A zero-column aggregate (``COUNT(NULL)``, ``COUNT(DISTINCT 1)``, ``SUM(1)``) exported as a + LookML ``type: number`` re-imports as an opaque complete-SQL metric whose referenced-column + set is empty, so compiling it builds an empty model CTE (``SELECT ... FROM`` with no select + list). Callers use this to skip such measures. On a parse failure assume it DOES reference a + column, so a genuine (unparseable) column expression is not wrongly dropped. + """ + import sqlglot + from sqlglot import expressions as exp + + try: + tree = sqlglot.parse_one(sql.replace("{model}", "__m__").replace("${TABLE}", "__m__")) + except Exception: + return True + return any(True for _ in tree.find_all(exp.Column)) + @classmethod def _fold_filters_into_aggregate(cls, agg_sql: str, filters: list[str], model: Model) -> str | None: """Fold ``filters`` into a single-outer-aggregate SQL expression. @@ -3399,6 +3418,22 @@ def _export_view(self, model: Model, graph: SemanticGraph) -> dict: # derived metric over an empty CTE (SELECT FROM ...), which the # compiler rejects. Native type: count round-trips cleanly. measure_def["type"] = "count" + elif not metric.filters and not self._aggregate_references_column(col_sql): + # ANY OTHER zero-column aggregate -- COUNT(NULL), COUNT(DISTINCT 1), + # SUM(1), MAX('x') -- has the same fate as a bare constant count: a + # type: number re-imports as an opaque complete-SQL metric whose + # referenced-column set is empty, so compiling it builds an empty model + # CTE (SELECT ... FROM with no select list). Unlike a plain row count it + # has no faithful native form, so skip it with a warning. (A FILTERED + # one falls through: folding the filter in makes it reference the + # filter's columns, e.g. COUNT(*) -> COUNT(CASE WHEN ... THEN 1 END).) + logger.warning( + "Metric %r has a zero-column aggregate SQL (%r) with no LookML " + "equivalent that round-trips; skipping on export.", + metric.name, + col_sql, + ) + continue else: measure_def["type"] = "number" if metric.filters: diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 7086b27c..06c651a7 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4930,8 +4930,9 @@ def test_lookml_export_count_constant_uses_native_count_type(): COUNT(1)/COUNT(0), plus COUNT(TRUE), COUNT('x'), COUNT(1.0), COUNT(.5) all count every row. Exporting them as type: number would re-import as a zero-column complete-SQL metric whose - query hits an empty model CTE (SELECT FROM ...). COUNT(NULL) is NOT a row count (always 0), so - it must stay a type: number. + query hits an empty model CTE (SELECT FROM ...). Every OTHER zero-column aggregate that is NOT + a plain row count -- COUNT(NULL), COUNT(DISTINCT 1), SUM(1), MAX('x') -- has no faithful native + form, so it is SKIPPED (not emitted as a broken type: number). """ import re import tempfile @@ -4952,15 +4953,16 @@ def export_measure(expr): ) out = tempfile.mktemp(suffix=".lkml") LookMLAdapter().export(graph, out) - return re.search(r"measure: c \{.*?\n \}", open(out).read(), re.S).group(0) + m = re.search(r"measure: c \{.*?\n \}", open(out).read(), re.S) + return m.group(0) if m else None for expr in ("COUNT(1)", "COUNT(0)", "COUNT(TRUE)", "COUNT('x')", "COUNT(1.0)", "COUNT(.5)"): block = export_measure(expr) - assert "type: count" in block and expr not in block, f"{expr} -> {block}" + assert block and "type: count" in block and expr not in block, f"{expr} -> {block}" - # COUNT(NULL) is always 0, not a row count: it must NOT be flattened to type: count. - null_block = export_measure("COUNT(NULL)") - assert "type: number" in null_block and "COUNT(NULL)" in null_block + # Zero-column aggregates that are NOT plain row counts have no round-trippable form -> skipped. + for expr in ("COUNT(NULL)", "COUNT(DISTINCT 1)", "SUM(1)", "MAX('x')"): + assert export_measure(expr) is None, f"{expr} should be skipped, not exported" def test_lookml_export_spaced_count_star_maps_to_native_count(): From abcad456014875df130f32f49c18a016d150f20a Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Thu, 16 Jul 2026 19:36:25 -0700 Subject: [PATCH 04/44] Keep the ALL aggregate modifier outside folded CASE filters COUNT(ALL x) with filters folded to COUNT(CASE WHEN ... THEN ALL x END): ALL is an aggregate modifier, not a row expression, so the exported LookML SQL was malformed while the separate filters block was suppressed. Handle ALL exactly like DISTINCT -- keep the modifier outside the CASE (COUNT(ALL CASE WHEN ... THEN x END)) and bail on a multi-column argument. The existing lookahead keeps a column actually named 'all' a plain argument. --- sidemantic/adapters/lookml.py | 11 ++++--- tests/adapters/lookml/test_edge_cases.py | 39 ++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index da13fe4d..f3353de7 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -3114,16 +3114,19 @@ def _fold_filters_into_aggregate(cls, agg_sql: str, filters: list[str], model: M # outside the CASE (it's part of the aggregate, not the value being filtered). Accept # the parenthesized spelling COUNT(DISTINCT(x)) too; the lookahead requires a space or # `(` after DISTINCT so an identifier like `DISTINCTION` is not mistaken for it. - dm = re.match(r"(?is)^DISTINCT(?=[\s(])\s*(.+)$", arg) + # DISTINCT / ALL are aggregate MODIFIERS, not row expressions: they must stay OUTSIDE + # the CASE (COUNT(DISTINCT CASE ... END), not COUNT(CASE ... THEN DISTINCT x END)). + # The lookahead keeps a column actually named `all`/`distinct` (COUNT(all)) a plain arg. + dm = re.match(r"(?is)^(DISTINCT|ALL)(?=[\s(])\s*(.+)$", arg) if dm: - distinct_arg = dm.group(1).strip() + modifier, mod_arg = dm.group(1).upper(), dm.group(2).strip() # A multi-column DISTINCT (COUNT(DISTINCT a, b)) has no single CASE result, so # bail and let the caller skip rather than emit malformed `THEN a, b END`. # quote_aware: a delimited composite key COUNT(DISTINCT a || ',' || b) is ONE # column -- the comma in the string literal must not count as a separator. - if len(cls._split_top_level_commas(distinct_arg, quote_aware=True)) > 1: + if len(cls._split_top_level_commas(mod_arg, quote_aware=True)) > 1: return None - return f"{func}(DISTINCT CASE WHEN {conds} THEN {distinct_arg} END)" + return f"{func}({modifier} CASE WHEN {conds} THEN {mod_arg} END)" # A multi-argument aggregate (WEIGHTED_AVG(price, qty)) has no single CASE result, # so bail rather than emit malformed `THEN price, qty END`. if len(cls._split_top_level_commas(arg, quote_aware=True)) > 1: diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 06c651a7..54bbbdef 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4817,6 +4817,45 @@ def test_lookml_export_string_literal_paren_in_aggregate_arg_folds(): assert "DISTINCT CASE WHEN" in text +def test_lookml_export_all_modifier_stays_outside_folded_case(): + """COUNT(ALL x) must fold as COUNT(ALL CASE ... END), not COUNT(CASE ... THEN ALL x END). + + ALL is an aggregate MODIFIER like DISTINCT, not a row expression, so wrapping it inside the + CASE emits malformed SQL while the separate filters block is suppressed. A column actually + NAMED `all` must still be treated as a plain argument. + """ + import duckdb + + from sidemantic import Dimension, Model + + model = Model( + name="o", + table="t", + primary_key="id", + dimensions=[Dimension(name="status", type="categorical", sql="status")], + ) + folded = LookMLAdapter._fold_filters_into_aggregate("COUNT(ALL {model}.user_id)", ["{model}.status = 'x'"], model) + assert folded is not None + assert folded.startswith("COUNT(ALL CASE WHEN"), folded # modifier outside the CASE + assert "THEN ALL " not in folded, folded # NOT the malformed generic wrapper + + # The folded SQL must actually execute. + con = duckdb.connect() + con.execute("create table t as select * from (values (1,'x'),(2,'y'),(3,'x')) v(user_id,status)") + runnable = folded.replace("${TABLE}.", "").replace("{model}.", "") + assert con.execute(f"select {runnable} from t").fetchone() == (2,) + + # A column literally named `all` is a plain argument, not a modifier. + plain = LookMLAdapter._fold_filters_into_aggregate("COUNT({model}.all)", ["{model}.status = 'x'"], model) + assert plain is not None and "COUNT(ALL CASE" not in plain, plain + + # Multi-column ALL has no single CASE result -> bail so the caller skips it. + assert ( + LookMLAdapter._fold_filters_into_aggregate("COUNT(ALL {model}.a, {model}.b)", ["{model}.status = 'x'"], model) + is None + ) + + def test_lookml_export_parenthesized_distinct_filter_folds(): """COUNT(DISTINCT(x)) (parenthesized, no space) must fold its filter, not emit malformed SQL.""" import tempfile From 78b2effea5a835617502445d3825b31b7631f3a5 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Thu, 16 Jul 2026 19:49:22 -0700 Subject: [PATCH 05/44] Re-check folded SQL for zero columns and reject aggregate-local ORDER BY Two filtered-complete-aggregate export bugs: - The zero-column guard ran only when the metric had NO filters, but a filter need not reference a column: COUNT(*) with '1 = 1' (or a pure template predicate) folds to COUNT(CASE WHEN (1 = 1) THEN 1 END), which still references none and re-imports as a metric over an empty model CTE. Re-run the zero-column check on the FOLDED SQL; a filter that does reference a column still exports. - An aggregate-local ORDER BY (SUM(amount ORDER BY created_at)) belongs to the aggregate call, not the argument, so wrapping the whole arg emitted SUM(CASE WHEN ... THEN amount ORDER BY created_at END). Detect a top-level ORDER BY (paren-depth 0, outside string literals, word-boundary anchored) and bail so the caller skips instead of exporting malformed SQL. --- sidemantic/adapters/lookml.py | 49 ++++++++++++++++ tests/adapters/lookml/test_edge_cases.py | 72 ++++++++++++++++++++++++ 2 files changed, 121 insertions(+) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index f3353de7..882fb5cf 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -469,6 +469,37 @@ def _sql_has_list_aggregate(sql: str) -> bool: return False return any(True for _ in tree.find_all(exp.List)) or any(True for _ in tree.find_all(exp.ArrayAgg)) + @staticmethod + def _has_top_level_order_by(s: str) -> bool: + """True if ``s`` has an ``ORDER BY`` at paren depth 0, outside string literals. + + Used to detect an aggregate-local ORDER BY (``SUM(amount ORDER BY created_at)``), which + belongs to the aggregate CALL rather than the argument expression -- so a filter must not + be folded into a CASE around it. + """ + depth, quote, i = 0, None, 0 + while i < len(s): + ch = s[i] + if quote is not None: + if ch == quote: + quote = None + i += 1 + continue + if ch in "'\"": + quote = ch + elif ch in "[(": + depth += 1 + elif ch in ")]": + depth = max(0, depth - 1) + elif ( + depth == 0 + and (i == 0 or not (s[i - 1].isalnum() or s[i - 1] == "_")) + and re.match(r"(?i)order\s+by\b", s[i:]) + ): + return True + i += 1 + return False + @staticmethod def _has_subquery(sql: str) -> bool: """True if ``sql`` contains a SELECT outside any quoted token. @@ -3106,6 +3137,12 @@ def _fold_filters_into_aggregate(cls, agg_sql: str, filters: list[str], model: M if _has_agg(arg): return None + # An aggregate-local ORDER BY (SUM(amount ORDER BY created_at), ARRAY_AGG(x ORDER BY y)) + # belongs to the aggregate CALL, not to the argument expression. Wrapping the whole arg + # in a CASE would emit `SUM(CASE WHEN ... THEN amount ORDER BY created_at END)`, which is + # malformed. Bail so the caller skips rather than export invalid SQL. + if cls._has_top_level_order_by(arg): + return None conds = cls._fold_filter_conds(filters, model) # COUNT(*) -> COUNT(CASE WHEN ... THEN 1 END): "* " can't live inside CASE. if arg == "*": @@ -3448,6 +3485,18 @@ def _export_view(self, model: Model, graph: SemanticGraph) -> dict: metric.name, ) continue + # Re-run the zero-column check on the FOLDED SQL: a filter does + # not necessarily add a column (COUNT(*) with `1 = 1`, or a pure + # template predicate), so the result can still reference none and + # would re-import as a metric over an empty model CTE. + if not self._aggregate_references_column(folded): + logger.warning( + "Metric %r folds to a zero-column aggregate SQL (%r) with no LookML " + "equivalent that round-trips; skipping on export.", + metric.name, + folded, + ) + continue measure_def["sql"] = folded filters_folded = True else: diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 54bbbdef..d67ae553 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4817,6 +4817,78 @@ def test_lookml_export_string_literal_paren_in_aggregate_arg_folds(): assert "DISTINCT CASE WHEN" in text +def test_lookml_export_folded_zero_column_aggregate_skipped(): + """A filter that adds NO column must not sneak a zero-column aggregate past the guard. + + The zero-column check runs before folding, but a filter needn't reference a column: COUNT(*) + with `1 = 1` folds to COUNT(CASE WHEN (1 = 1) THEN 1 END), which STILL references none and + re-imports as a metric over an empty model CTE. Re-check the folded SQL. A filter that does + reference a column must still export. + """ + import re + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + def export_measure(expr, filters): + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="status"), + ], + metrics=[Metric(name="c", agg=None, sql=expr, sql_is_complete=True, filters=filters)], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + m = re.search(r"measure: c \{.*?\n \}", open(out).read(), re.S) + return m.group(0) if m else None + + # Folds to COUNT(CASE WHEN (1 = 1) THEN 1 END) -> still zero-column -> skipped. + assert export_measure("COUNT(*)", ["1 = 1"]) is None + # A filter that DOES reference a column still exports (the folded CASE reads it). + block = export_measure("COUNT(*)", ["{model}.status = 'x'"]) + assert block and "COUNT(CASE WHEN" in block and "THEN 1 END" in block + + +def test_lookml_export_aggregate_order_by_not_folded_into_case(): + """An aggregate-local ORDER BY belongs to the aggregate call, not the CASE result. + + SUM(amount ORDER BY created_at) must NOT fold to + SUM(CASE WHEN ... THEN amount ORDER BY created_at END) (malformed); bail so the caller skips. + An ORDER BY inside a string literal or nested parens is not a top-level one. + """ + from sidemantic import Dimension, Model + + model = Model( + name="o", + table="t", + primary_key="id", + dimensions=[Dimension(name="status", type="categorical", sql="status")], + ) + filters = ["{model}.status = 'x'"] + assert ( + LookMLAdapter._fold_filters_into_aggregate("SUM({model}.amount ORDER BY {model}.created_at)", filters, model) + is None + ) + # A plain aggregate still folds normally. + folded = LookMLAdapter._fold_filters_into_aggregate("SUM({model}.amount)", filters, model) + assert folded and folded.startswith("SUM(CASE WHEN") + + # The detector itself: only a TOP-LEVEL, unquoted ORDER BY counts. + assert LookMLAdapter._has_top_level_order_by("{model}.a ORDER BY {model}.b") + assert LookMLAdapter._has_top_level_order_by("{model}.a order by {model}.b") # case-insensitive + assert not LookMLAdapter._has_top_level_order_by("'a order by b'") # string literal + assert not LookMLAdapter._has_top_level_order_by("F({model}.a, ' order by ')") # literal in a call + assert not LookMLAdapter._has_top_level_order_by("{model}.reorder_by_date") # word boundary + + def test_lookml_export_all_modifier_stays_outside_folded_case(): """COUNT(ALL x) must fold as COUNT(ALL CASE ... END), not COUNT(CASE ... THEN ALL x END). From e93b6916dc29d505d261c3f350e24fd640be2189 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Thu, 16 Jul 2026 19:58:10 -0700 Subject: [PATCH 06/44] Do not rewrite table qualifiers matching a dimension in folded filters The bare-dimension alternative's negative lookbehind guards the field AFTER a dot (status in customers.status), but nothing guarded the QUALIFIER before one. On a model with a 'customers' dimension, a filter like customers.status = 'vip' rewrote the qualifier and emitted (${TABLE}.customer_id).status = 'vip' while the separate filters block was suppressed. Add a negative lookahead for a following dot: a bare name followed by '.' is a table qualifier, not a column. The same name used as a real column (no dot) is still rewritten, as are {model}. and own-model qualified refs. --- sidemantic/adapters/lookml.py | 9 ++++--- tests/adapters/lookml/test_edge_cases.py | 30 ++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 882fb5cf..446c60c2 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -3005,12 +3005,15 @@ def _qualify(val: str) -> str: # negative lookbehind for `.`/word-char: the bare one so it does NOT match the field # of a foreign qualifier (`status` inside `customers.status`), and the model-name one # so it does NOT match a schema-qualified ref (`orders.status` inside - # `schema.orders.status`). The bare alt also has a negative lookahead for `(` so it - # does NOT match a function name (e.g. `date(...)`). + # `schema.orders.status`). The bare alt also has negative lookaheads: for `(` so it does + # NOT match a function name (e.g. `date(...)`), and for `.` so it does NOT match a table + # QUALIFIER (`customers` inside `customers.status` on a model that also has a `customers` + # dimension) -- the lookbehind only guards the field AFTER a dot, not the name before it, + # which would otherwise emit `(${TABLE}.customer_id).status`. names_alt = "|".join(re.escape(n) for n in sorted(dim_names, key=len, reverse=True)) pattern = rf"(?:\{{model\}}|(? str: diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index d67ae553..c23c2938 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4650,6 +4650,36 @@ def test_lookml_export_folded_filter_does_not_rewrite_cast_type(): assert "AS (${TABLE}" not in text # the cast type is NOT rewritten to a column +def test_lookml_export_folded_filter_does_not_rewrite_table_qualifier(): + """A foreign table QUALIFIER that matches a dimension name must not be rewritten. + + On an `orders` model that also has a `customers` dimension, the filter + `customers.status = 'vip'` qualifies another table. The lookbehind only guards the field + AFTER a dot, so `customers` (before the dot) still matched and produced + `(${TABLE}.customer_id).status = 'vip'`. A bare name followed by a dot is a qualifier, not a + column; a same-named dimension WITHOUT a dot is still a column. + """ + from sidemantic import Dimension, Model + + model = Model( + name="orders", + table="raw_orders", + primary_key="id", + dimensions=[ + Dimension(name="customers", type="numeric", sql="customer_id"), + Dimension(name="status", type="categorical", sql="order_status"), + ], + ) + conds = LookMLAdapter._fold_filter_conds + # Foreign qualifier left intact (neither the qualifier nor its field rewritten). + assert conds(["customers.status = 'vip'"], model) == "(customers.status = 'vip')" + # The same name used as a real column (no dot) IS still rewritten. + assert conds(["customers = 5"], model) == "((${TABLE}.customer_id) = 5)" + # Own-model and {model} qualifiers still resolve. + assert conds(["orders.status = 'done'"], model) == "((${TABLE}.order_status) = 'done')" + assert conds(["{model}.status = 'done'"], model) == "((${TABLE}.order_status) = 'done')" + + def test_lookml_export_folded_filter_does_not_rewrite_date_part_keyword(): """A folded filter's date-part / interval-unit keyword equal to a dimension must not be rewritten. From 0739c1e49d0d54b8f14aa00b4a47908051fe945b Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Thu, 16 Jul 2026 20:14:03 -0700 Subject: [PATCH 07/44] Protect unquoted date-part arguments of date functions in folded filters The date-part guard only covered EXTRACT/INTERVAL positions, but dialects such as BigQuery pass the part as an unquoted argument: DATE_TRUNC(created_at, month) or DATE_DIFF(a, b, day). On a model with a 'month' dimension that exported DATE_TRUNC(created_at, (${TABLE}.order_month)) while suppressing the filters block. Generalize instead of adding another position pattern: protect a bare token that is a known date-part KEYWORD appearing inside a call to a known date/time function (resolved by scanning back for the enclosing call). Gating on the keyword set keeps a real column argument of the SAME call resolvable (created_at above), and a keyword-named column outside a date function is still rewritten. --- sidemantic/adapters/lookml.py | 56 +++++++++++++++++++++++- tests/adapters/lookml/test_edge_cases.py | 33 ++++++++++++++ 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 446c60c2..33d78018 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -469,6 +469,50 @@ def _sql_has_list_aggregate(sql: str) -> bool: return False return any(True for _ in tree.find_all(exp.List)) or any(True for _ in tree.find_all(exp.ArrayAgg)) + # SQL date-part keywords, and the date/time functions that take one as an UNQUOTED argument. + # A bare token matching BOTH (a keyword inside such a call) is a date part, not a column, so + # folded filters must not rewrite it to a dimension's SQL -- e.g. BigQuery's + # DATE_TRUNC(created_at, month) on a model that also has a `month` dimension. Gating on the + # keyword set keeps a real column argument of the same call (created_at) resolvable. + _DATE_PART_KEYWORDS = frozenset( + { + "year", "quarter", "month", "week", "day", "hour", "minute", "second", + "millisecond", "microsecond", "nanosecond", "epoch", "date", "time", + "dayofweek", "dayofyear", "dow", "doy", "isoweek", "isoyear", "isodow", + "weekday", "yearofweek", "century", "decade", "millennium", + } + ) # fmt: skip + _DATE_PART_FUNCS = frozenset( + { + "extract", "date_part", "datepart", "date_trunc", "datetrunc", "timestamp_trunc", + "datetime_trunc", "time_trunc", "date_diff", "datediff", "timestamp_diff", + "datetime_diff", "time_diff", "date_add", "dateadd", "date_sub", "datesub", + "timestamp_add", "timestamp_sub", "datetime_add", "datetime_sub", "last_day", + "timestamp_bucket", "time_bucket", + } + ) # fmt: skip + + @staticmethod + def _enclosing_function(pre: str) -> str | None: + """Name of the function whose open paren directly encloses the end of ``pre``. + + ``pre`` is the text BEFORE a token; scanning backwards for the first unclosed ``(`` and + reading the identifier in front of it identifies the call the token sits in (used to tell + a date-part argument from a column). Returns None when the token is not inside a call. + """ + depth, i = 0, len(pre) - 1 + while i >= 0: + ch = pre[i] + if ch == ")": + depth += 1 + elif ch == "(": + if depth == 0: + m = re.search(r"(\w+)\s*$", pre[:i]) + return m.group(1) if m else None + depth -= 1 + i -= 1 + return None + @staticmethod def _has_top_level_order_by(s: str) -> bool: """True if ``s`` has an ``ORDER BY`` at paren depth 0, outside string literals. @@ -2976,8 +3020,8 @@ def export(self, graph: SemanticGraph, output_path: str | Path) -> None: lookml_str = lkml.dump(data) f.write(lookml_str) - @staticmethod - def _fold_filter_conds(filters: list[str], model: Model) -> str: + @classmethod + def _fold_filter_conds(cls, filters: list[str], model: Model) -> str: """Resolve ``metric.filters`` into an AND-joined, ``${TABLE}``-qualified SQL predicate for folding into an exported aggregate measure. @@ -3049,6 +3093,14 @@ def _one(m): re.search(r"(?i)\bextract\s*\(\s*$", pre) or re.match(r"(?is)\s+from\b", suf) or re.search(r"(?i)\binterval\s+(?:[+-]?\d+(?:\.\d+)?|'(?:[^']|'')*')\s*$", pre) + # A date-part KEYWORD passed unquoted to a date/time function, e.g. + # BigQuery's DATE_TRUNC(created_at, month) or DATE_DIFF(a, b, day). + # Gate on the keyword set so a real column argument of the SAME call + # (created_at above) is still resolved. + or ( + bare.lower() in cls._DATE_PART_KEYWORDS + and (cls._enclosing_function(pre) or "").lower() in cls._DATE_PART_FUNCS + ) ): return m.group(0) name = m.group(1) or bare diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index c23c2938..c05696c7 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4650,6 +4650,39 @@ def test_lookml_export_folded_filter_does_not_rewrite_cast_type(): assert "AS (${TABLE}" not in text # the cast type is NOT rewritten to a column +def test_lookml_export_folded_filter_does_not_rewrite_unquoted_date_part_argument(): + """An UNQUOTED date part passed to a date/time function must not be rewritten as a column. + + BigQuery-style DATE_TRUNC(created_at, month) / DATE_DIFF(a, b, day) pass the part unquoted, so + on a model with `month`/`day` dimensions it became DATE_TRUNC(..., (${TABLE}.order_month)). + Protection is gated on the date-part KEYWORD set, so a real column argument of the SAME call + (created_at) still resolves, and a keyword-named column outside a date function still resolves. + """ + from sidemantic import Dimension, Model + + model = Model( + name="orders", + table="raw_orders", + primary_key="id", + dimensions=[ + Dimension(name="month", type="time", granularity="month", sql="order_month"), + Dimension(name="day", type="time", granularity="day", sql="order_day"), + Dimension(name="created_at", type="time", granularity="day", sql="created_at"), + ], + ) + conds = LookMLAdapter._fold_filter_conds + # The part is protected; the column argument of the same call IS resolved. + assert conds(["DATE_TRUNC(created_at, month) = DATE '2024-01-01'"], model) == ( + "(DATE_TRUNC((${TABLE}.created_at), month) = DATE '2024-01-01')" + ) + assert conds(["DATE_DIFF(created_at, created_at, day) > 1"], model) == ( + "(DATE_DIFF((${TABLE}.created_at), (${TABLE}.created_at), day) > 1)" + ) + # A keyword-named column used for real is still rewritten (not over-protected). + assert conds(["month = '2024-01'"], model) == "((${TABLE}.order_month) = '2024-01')" + assert conds(["UPPER(day) = 'X'"], model) == "(UPPER((${TABLE}.order_day)) = 'X')" + + def test_lookml_export_folded_filter_does_not_rewrite_table_qualifier(): """A foreign table QUALIFIER that matches a dimension name must not be rewritten. From c898722a523b298287c45a5a05352e71eb403aa5 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Thu, 16 Jul 2026 21:02:22 -0700 Subject: [PATCH 08/44] Preserve a filtered base measure's filter in post-SQL measures A percent_of_total / percent_of_previous over a base measure that carries its own LookML filters expanded via the bare ({model}.) template, which has no filter -- so e.g. a percent over a count_distinct filtered to status: completed was computed across every row instead of the filtered population. The first pass already builds the FILTERED aggregate for such a base in measure_full_sql_lookup; expand through it when the base is known to carry filters. Unfiltered bases keep the existing template form, so the change is limited to the measures that were losing a filter. --- sidemantic/adapters/lookml.py | 37 +++++++++++++++++++++++- tests/adapters/lookml/test_edge_cases.py | 32 ++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 33d78018..706978a4 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -1297,6 +1297,11 @@ def _parse_view(self, view_def: dict) -> Model | None: # used to expand a measure ref inside an aggregate-safe mixed number measure into # its base aggregate over the REAL column (not a phantom {model}.). measure_full_sql_lookup: dict[str, str] = {} + # Base measures carrying their OWN LookML `filters`. A post-SQL measure (percent_of_total + # / percent_of_previous) that references one must expand it through the FILTERED aggregate + # in measure_full_sql_lookup, not the bare `({model}.)` template -- which + # would drop the filter and compute over every row. + filtered_base_measures: set[str] = set() def _folded_measure_filter(m_def): # AND-joined predicate for a base measure's OWN filters, with each filter field @@ -1341,6 +1346,8 @@ def _folded_measure_filter(m_def): # mixed-expr expansion of a filtered measure (e.g. completed_total with # filters: [status: "completed"]) keeps the filter, not SUM(amount). joined = _folded_measure_filter(m) + if joined: + filtered_base_measures.add(m_name) if m_type == "count" and col.strip() == "*": # `type: count sql: * ;;` is the row-count form; `*` can't live inside # a CASE, so route it through the no-sql count path instead of emitting @@ -1358,6 +1365,7 @@ def _folded_measure_filter(m_def): # expansion instead of counting all rows. joined = _folded_measure_filter(m) if joined: + filtered_base_measures.add(m_name) measure_full_sql_lookup[m_name] = f"COUNT(CASE WHEN {joined} THEN 1 END)" else: measure_full_sql_lookup[m_name] = "COUNT(*)" @@ -1477,6 +1485,7 @@ def _sub(mm): measure_full_sql_lookup, view_name=name.lstrip("+"), filter_sensitive_measures=filter_sensitive_measures, + filtered_base_measures=filtered_base_measures, ) if measure and self._leaks_cross_view_ref(measure.sql): # Like a cross-view dimension, a measure whose SQL references another view @@ -2086,6 +2095,7 @@ def _parse_measure( measure_full_sql_lookup: dict[str, str] | None = None, view_name: str | None = None, filter_sensitive_measures: set[str] | None = None, + filtered_base_measures: set[str] | None = None, ) -> Metric | None: """Parse LookML measure. @@ -2110,6 +2120,7 @@ def _parse_measure( measure_agg_lookup = measure_agg_lookup or {} measure_full_sql_lookup = measure_full_sql_lookup or {} filter_sensitive_measures = filter_sensitive_measures or set() + filtered_base_measures = filtered_base_measures or set() # Check if type is explicitly set has_explicit_type = "type" in measure_def @@ -2214,6 +2225,8 @@ def _parse_measure( dimension_sql_lookup, measure_names or set(), measure_agg_lookup or {}, + measure_full_sql_lookup, + filtered_base_measures, ) # Map LookML measure types to sidemantic aggregation types @@ -2679,6 +2692,8 @@ def _resolve_measure_reference_sql( dimension_sql_lookup: dict[str, str], measure_names: set[str] | None = None, measure_agg_lookup: dict[str, str] | None = None, + measure_full_sql_lookup: dict[str, str] | None = None, + filtered_base_measures: set[str] | None = None, ) -> str: """Resolve ${ref} in a measure-referencing SQL (e.g. running_total sql). @@ -2694,6 +2709,8 @@ def _resolve_measure_reference_sql( """ measure_names = measure_names or set() measure_agg_lookup = measure_agg_lookup or {} + measure_full_sql_lookup = measure_full_sql_lookup or {} + filtered_base_measures = filtered_base_measures or set() sql = sql.replace("${TABLE}", "{model}") def _resolve(match: re.Match) -> str: @@ -2713,6 +2730,13 @@ def _resolve(match: re.Match) -> str: if ref_name in dimension_sql_lookup: return f"({dimension_sql_lookup[ref_name]})" if ref_name in measure_names: + # A base measure with its OWN LookML `filters` must expand to the FILTERED + # aggregate built in the first pass (e.g. APPROX_COUNT_DISTINCT(CASE WHEN + # status='completed' THEN user_id END)). The `({model}.)` template + # below carries no filter, so a percent_of_total over a filtered base would be + # computed across every row instead of the filtered population. + if ref_name in filtered_base_measures and ref_name in measure_full_sql_lookup: + return f"({measure_full_sql_lookup[ref_name]})" agg_template = measure_agg_lookup.get(ref_name) if agg_template: return agg_template.format(f"{{model}}.{ref_name}") @@ -2729,6 +2753,8 @@ def _parse_post_sql_measure( dimension_sql_lookup: dict[str, str], measure_names: set[str] | None = None, measure_agg_lookup: dict[str, str] | None = None, + measure_full_sql_lookup: dict[str, str] | None = None, + filtered_base_measures: set[str] | None = None, ) -> Metric | None: """Parse a post-SQL / table-calculation measure. @@ -2760,6 +2786,8 @@ def _parse_post_sql_measure( return None measure_names = measure_names or set() measure_agg_lookup = measure_agg_lookup or {} + measure_full_sql_lookup = measure_full_sql_lookup or {} + filtered_base_measures = filtered_base_measures or set() if measure_type == "running_total": # A running_total maps to a cumulative metric whose `sql` is the base @@ -2780,7 +2808,14 @@ def _parse_post_sql_measure( # percent_of_total / percent_of_previous build window aggregates inline, # so qualify base measure refs with {model} (for the generator's _raw # column rewrite) and wrap them in the base measure's aggregate function. - base = self._resolve_measure_reference_sql(sql, dimension_sql_lookup, measure_names, measure_agg_lookup).strip() + base = self._resolve_measure_reference_sql( + sql, + dimension_sql_lookup, + measure_names, + measure_agg_lookup, + measure_full_sql_lookup, + filtered_base_measures, + ).strip() common = { "description": measure_def.get("description"), diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index c05696c7..32035ca6 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4154,6 +4154,38 @@ def test_lookml_approximate_distinct_preserved_in_post_sql_measure(): assert "COUNT(DISTINCT" not in pct.sql +def test_lookml_post_sql_measure_preserves_filtered_base_measure_filter(): + """A post-SQL measure over a FILTERED base must keep the base's filter. + + A percent_of_total over `uu` (count_distinct, approximate, filtered to completed) expanded via + the bare `({model}.uu)` template, which carries no filter -- so the percent was computed + over every row instead of the filtered population. A filtered base must expand through the + FILTERED aggregate built in the first pass. An UNFILTERED base keeps the template form. + """ + graph = _parse_lkml( + """ +view: orders { + sql_table_name: orders ;; + dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } + dimension: user_id { type: number sql: ${TABLE}.user_id ;; } + dimension: status { type: string sql: ${TABLE}.status ;; } + measure: uu { type: count_distinct approximate: yes sql: ${user_id} ;; filters: [status: "completed"] } + measure: pct { type: percent_of_total sql: ${uu} ;; } + measure: total { type: sum sql: ${TABLE}.amount ;; } + measure: pct_unfiltered { type: percent_of_total sql: ${total} ;; } +} +""" + ) + model = graph.get_model("orders") + pct = model.get_metric("pct") + # The base's filter is carried into the expansion, over the REAL column, still approximate. + assert "completed" in pct.sql, pct.sql + assert "APPROX_COUNT_DISTINCT" in pct.sql and "COUNT(DISTINCT" not in pct.sql, pct.sql + assert "user_id" in pct.sql, pct.sql + # An UNFILTERED base is unchanged (still the aggregate template over the measure ref). + assert "{model}.total" in model.get_metric("pct_unfiltered").sql + + def test_lookml_export_running_total_roundtrips(): """An imported running_total (cumulative + table_calculation meta) round-trips, not dropped.""" import tempfile From 29d6b6b65b0c7cd3bbf5c2f4a3e2b23cb2036841 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Thu, 16 Jul 2026 21:34:02 -0700 Subject: [PATCH 09/44] Make the folded-filter date-part guard position-aware Protecting any date-part KEYWORD inside a date/time function over-protected real columns: DATE_TRUNC(date, month) on a model with both a 'date' and a 'month' dimension left the 'date' COLUMN unresolved, so the folded measure read a nonexistent/renamed column while the filters block stayed suppressed. Protect only the date-part ARGUMENT SLOT, which differs by dialect: BigQuery puts the part LAST (DATE_TRUNC(value, part), DATE_DIFF(a, b, part)); SQL Server FIRST (DATEDIFF(part, a, b), DATEADD(part, n, x)). EXTRACT(part FROM x) and INTERVAL n part keep their own position checks, and quoted parts are already protected as string literals. --- sidemantic/adapters/lookml.py | 93 ++++++++++++++++-------- tests/adapters/lookml/test_edge_cases.py | 35 +++++++++ 2 files changed, 98 insertions(+), 30 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 706978a4..00c13a00 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -469,11 +469,7 @@ def _sql_has_list_aggregate(sql: str) -> bool: return False return any(True for _ in tree.find_all(exp.List)) or any(True for _ in tree.find_all(exp.ArrayAgg)) - # SQL date-part keywords, and the date/time functions that take one as an UNQUOTED argument. - # A bare token matching BOTH (a keyword inside such a call) is a date part, not a column, so - # folded filters must not rewrite it to a dimension's SQL -- e.g. BigQuery's - # DATE_TRUNC(created_at, month) on a model that also has a `month` dimension. Gating on the - # keyword set keeps a real column argument of the same call (created_at) resolvable. + # SQL date-part keywords that a date/time function can take UNQUOTED. _DATE_PART_KEYWORDS = frozenset( { "year", "quarter", "month", "week", "day", "hour", "minute", "second", @@ -482,25 +478,30 @@ def _sql_has_list_aggregate(sql: str) -> bool: "weekday", "yearofweek", "century", "decade", "millennium", } ) # fmt: skip - _DATE_PART_FUNCS = frozenset( - { - "extract", "date_part", "datepart", "date_trunc", "datetrunc", "timestamp_trunc", - "datetime_trunc", "time_trunc", "date_diff", "datediff", "timestamp_diff", - "datetime_diff", "time_diff", "date_add", "dateadd", "date_sub", "datesub", - "timestamp_add", "timestamp_sub", "datetime_add", "datetime_sub", "last_day", - "timestamp_bucket", "time_bucket", - } - ) # fmt: skip + # Which ARGUMENT of a date/time call is the date part. Position matters: the same keyword can + # be a real column in another slot -- DATE_TRUNC(date, month) on a model with BOTH a `date` + # and a `month` dimension means column `date` truncated to part `month`. Keying only on "is a + # keyword inside a date function" would wrongly protect the `date` COLUMN too. + # -1 => the LAST argument (BigQuery DATE_TRUNC(value, part), DATE_DIFF(a, b, part)) + # 0 => the FIRST argument (SQL Server DATEADD(part, n, x) / DATEDIFF(part, a, b)) + # EXTRACT(part FROM x) and INTERVAL n part are handled by their own position checks; quoted + # forms (DATE_TRUNC('day', x)) are already protected as string literals. + _DATE_PART_ARG_POS = { + "date_trunc": -1, "datetrunc": -1, "timestamp_trunc": -1, "datetime_trunc": -1, + "time_trunc": -1, "timestamp_bucket": -1, "time_bucket": -1, + "date_diff": -1, "timestamp_diff": -1, "datetime_diff": -1, "time_diff": -1, + "datediff": 0, "dateadd": 0, "datepart": 0, "date_part": 0, + } # fmt: skip @staticmethod - def _enclosing_function(pre: str) -> str | None: - """Name of the function whose open paren directly encloses the end of ``pre``. + def _enclosing_call(pre: str) -> tuple[str | None, int, int]: + """Describe the call a token sits in, given the text ``pre`` before it. - ``pre`` is the text BEFORE a token; scanning backwards for the first unclosed ``(`` and - reading the identifier in front of it identifies the call the token sits in (used to tell - a date-part argument from a column). Returns None when the token is not inside a call. + Returns ``(function_name, arg_index, open_paren_pos)`` by scanning backwards for the first + unclosed ``(`` and counting the top-level commas after it. ``(None, 0, -1)`` when the token + is not inside a call. """ - depth, i = 0, len(pre) - 1 + depth, commas, i = 0, 0, len(pre) - 1 while i >= 0: ch = pre[i] if ch == ")": @@ -508,10 +509,44 @@ def _enclosing_function(pre: str) -> str | None: elif ch == "(": if depth == 0: m = re.search(r"(\w+)\s*$", pre[:i]) - return m.group(1) if m else None + return (m.group(1) if m else None), commas, i depth -= 1 + elif ch == "," and depth == 0: + commas += 1 i -= 1 - return None + return None, 0, -1 + + @classmethod + def _is_date_part_argument(cls, pre: str, suf: str, token: str) -> bool: + """True if ``token`` occupies the date-PART argument slot of a date/time call. + + Position-aware on purpose: DATE_TRUNC(date, month) on a model with both a `date` and a + `month` dimension means column `date` truncated to part `month` -- only the LAST argument + is the part, so the `date` column must still resolve. + """ + if token.lower() not in cls._DATE_PART_KEYWORDS: + return False + func, arg_index, open_pos = cls._enclosing_call(pre) + if not func: + return False + want = cls._DATE_PART_ARG_POS.get(func.lower()) + if want is None: + return False + if want >= 0: + return arg_index == want + # want == -1: the part is the LAST argument -- true when no top-level comma follows the + # token before the call's closing paren. + depth = 0 + for ch in suf: + if ch in "([": + depth += 1 + elif ch in ")]": + if depth == 0: + return True # reached this call's close with no further top-level comma + depth -= 1 + elif ch == "," and depth == 0: + return False + return False @staticmethod def _has_top_level_order_by(s: str) -> bool: @@ -3128,14 +3163,12 @@ def _one(m): re.search(r"(?i)\bextract\s*\(\s*$", pre) or re.match(r"(?is)\s+from\b", suf) or re.search(r"(?i)\binterval\s+(?:[+-]?\d+(?:\.\d+)?|'(?:[^']|'')*')\s*$", pre) - # A date-part KEYWORD passed unquoted to a date/time function, e.g. - # BigQuery's DATE_TRUNC(created_at, month) or DATE_DIFF(a, b, day). - # Gate on the keyword set so a real column argument of the SAME call - # (created_at above) is still resolved. - or ( - bare.lower() in cls._DATE_PART_KEYWORDS - and (cls._enclosing_function(pre) or "").lower() in cls._DATE_PART_FUNCS - ) + # A date-part KEYWORD in the date-part ARGUMENT SLOT of a date/time + # call, e.g. BigQuery's DATE_TRUNC(created_at, month) or + # DATE_DIFF(a, b, day). Position-aware so a real column in another + # slot of the SAME call still resolves -- DATE_TRUNC(date, month) is + # column `date` truncated to part `month`. + or cls._is_date_part_argument(pre, suf, bare) ): return m.group(0) name = m.group(1) or bare diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 32035ca6..8c4884f7 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4715,6 +4715,41 @@ def test_lookml_export_folded_filter_does_not_rewrite_unquoted_date_part_argumen assert conds(["UPPER(day) = 'X'"], model) == "(UPPER((${TABLE}.order_day)) = 'X')" +def test_lookml_export_folded_filter_date_part_guard_is_position_aware(): + """Only the date-part ARGUMENT SLOT is protected -- a keyword-named column elsewhere resolves. + + `DATE_TRUNC(date, month)` on a model with BOTH a `date` and a `month` dimension means column + `date` truncated to part `month`: protecting every keyword inside a date function would leave + the `date` COLUMN unresolved. Covers both argument conventions -- BigQuery puts the part LAST + (DATE_TRUNC(value, part), DATE_DIFF(a, b, part)), SQL Server FIRST (DATEDIFF(part, a, b)). + """ + from sidemantic import Dimension, Model + + model = Model( + name="orders", + table="raw_orders", + primary_key="id", + dimensions=[ + Dimension(name="date", type="time", granularity="day", sql="order_date"), + Dimension(name="month", type="time", granularity="month", sql="order_month"), + Dimension(name="day", type="time", granularity="day", sql="order_day"), + Dimension(name="created_at", type="time", granularity="day", sql="created_at"), + ], + ) + conds = LookMLAdapter._fold_filter_conds + # BigQuery: part is the LAST arg -- the `date` COLUMN in slot 0 must still resolve. + assert conds(["DATE_TRUNC(date, month) = DATE '2024-01-01'"], model) == ( + "(DATE_TRUNC((${TABLE}.order_date), month) = DATE '2024-01-01')" + ) + assert conds(["DATE_DIFF(created_at, date, day) > 1"], model) == ( + "(DATE_DIFF((${TABLE}.created_at), (${TABLE}.order_date), day) > 1)" + ) + # SQL Server: part is the FIRST arg; the trailing columns still resolve. + assert conds(["DATEDIFF(day, created_at, date) > 1"], model) == ( + "(DATEDIFF(day, (${TABLE}.created_at), (${TABLE}.order_date)) > 1)" + ) + + def test_lookml_export_folded_filter_does_not_rewrite_table_qualifier(): """A foreign table QUALIFIER that matches a dimension name must not be rewritten. From d995735543ba4f891cd8de963496bda2fede749e Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Thu, 16 Jul 2026 22:03:03 -0700 Subject: [PATCH 10/44] Fix date-part argument positions for SQL Server DATETRUNC and drop non-part functions Two errors in the date-part argument-position table: - DATETRUNC was mapped as part-LAST, conflating it with BigQuery's DATE_TRUNC. The two spellings differ: underscored DATE_TRUNC is BigQuery's (value, part), while the unspaced DATETRUNC is SQL Server's (part, value). SQL Server's DATETRUNC(month, created_at) therefore rewrote the 'month' PART as a column. (Postgres/DuckDB date_trunc('part', value) quotes the part -- already protected.) - time_bucket / timestamp_bucket take an INTERVAL, not a bare date part. Listing them protected whatever sat in that slot, which would silently leave a real keyword-named COLUMN unresolved. Drop them: only functions that actually take a bare date-part keyword belong in the table. --- sidemantic/adapters/lookml.py | 16 ++++++++++------ tests/adapters/lookml/test_edge_cases.py | 10 ++++++++++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 00c13a00..16bde50d 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -483,14 +483,18 @@ def _sql_has_list_aggregate(sql: str) -> bool: # and a `month` dimension means column `date` truncated to part `month`. Keying only on "is a # keyword inside a date function" would wrongly protect the `date` COLUMN too. # -1 => the LAST argument (BigQuery DATE_TRUNC(value, part), DATE_DIFF(a, b, part)) - # 0 => the FIRST argument (SQL Server DATEADD(part, n, x) / DATEDIFF(part, a, b)) - # EXTRACT(part FROM x) and INTERVAL n part are handled by their own position checks; quoted - # forms (DATE_TRUNC('day', x)) are already protected as string literals. + # 0 => the FIRST argument (SQL Server DATETRUNC(part, x) / DATEADD(part, n, x) / + # DATEDIFF(part, a, b); also DuckDB's datetrunc(part, x) spelling) + # NOTE the two TRUNC spellings differ: underscored DATE_TRUNC is BigQuery's (value, part) -- + # Postgres/DuckDB's date_trunc('part', value) quotes the part, so it is already protected as a + # string literal -- while the unspaced DATETRUNC is SQL Server's (part, value). + # Functions with NO bare date-part argument (time_bucket takes an INTERVAL, not a keyword) are + # deliberately absent: listing one would protect whatever sits in that slot, including a real + # keyword-named column. EXTRACT(part FROM x) and INTERVAL n part have their own position checks. _DATE_PART_ARG_POS = { - "date_trunc": -1, "datetrunc": -1, "timestamp_trunc": -1, "datetime_trunc": -1, - "time_trunc": -1, "timestamp_bucket": -1, "time_bucket": -1, + "date_trunc": -1, "timestamp_trunc": -1, "datetime_trunc": -1, "time_trunc": -1, "date_diff": -1, "timestamp_diff": -1, "datetime_diff": -1, "time_diff": -1, - "datediff": 0, "dateadd": 0, "datepart": 0, "date_part": 0, + "datetrunc": 0, "datediff": 0, "dateadd": 0, "datepart": 0, "date_part": 0, } # fmt: skip @staticmethod diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 8c4884f7..7ba4866e 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4748,6 +4748,16 @@ def test_lookml_export_folded_filter_date_part_guard_is_position_aware(): assert conds(["DATEDIFF(day, created_at, date) > 1"], model) == ( "(DATEDIFF(day, (${TABLE}.created_at), (${TABLE}.order_date)) > 1)" ) + # The two TRUNC spellings differ: underscored DATE_TRUNC is BigQuery's (value, part), while + # the unspaced DATETRUNC is SQL Server's (part, value). + assert conds(["DATETRUNC(month, created_at) = DATE '2024-01-01'"], model) == ( + "(DATETRUNC(month, (${TABLE}.created_at)) = DATE '2024-01-01')" + ) + # A function with NO bare date-part argument must not protect its slots: time_bucket takes an + # INTERVAL, so a keyword-named COLUMN in its last slot is still resolved. + assert conds(["time_bucket(INTERVAL '5 minutes', date) = 1"], model) == ( + "(time_bucket(INTERVAL '5 minutes', (${TABLE}.order_date)) = 1)" + ) def test_lookml_export_folded_filter_does_not_rewrite_table_qualifier(): From 6d423f4dcdb675dce1c0f1c2947242ad97684598 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Thu, 16 Jul 2026 22:24:09 -0700 Subject: [PATCH 11/44] Disambiguate DATE_TRUNC part by content and skip template-only folded filters - DATE_TRUNC has NO fixed date-part position: BigQuery is DATE_TRUNC(value, part) while Snowflake is DATE_TRUNC(part, expr) -- the same name with opposite orders, and the adapter has no dialect context. Mapping it as part-last rewrote Snowflake's leading part as a column. Decide by CONTENT instead: whichever argument is a date-part keyword is the part. When BOTH are keywords (DATE_TRUNC(date, month)) it is genuinely ambiguous, so the LAST is taken as the part (BigQuery's order) and the other still resolves as a column -- keeping the previously-fixed case working. - The column check reads a sqlglot parse failure as 'has columns', and a Liquid segment does not parse, so a folded filter that is ONLY a template (COUNT(CASE WHEN ({{ user_filter }}) THEN 1 END)) slipped past the zero-column guard and exported a measure with no real column. Neutralise templates to NULL before parsing; a template combined with a real column still exports, and genuinely unparseable SQL keeps the safe default. --- sidemantic/adapters/lookml.py | 47 ++++++++++++++++++++- tests/adapters/lookml/test_edge_cases.py | 53 ++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 16bde50d..1ffb353b 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -492,10 +492,14 @@ def _sql_has_list_aggregate(sql: str) -> bool: # deliberately absent: listing one would protect whatever sits in that slot, including a real # keyword-named column. EXTRACT(part FROM x) and INTERVAL n part have their own position checks. _DATE_PART_ARG_POS = { - "date_trunc": -1, "timestamp_trunc": -1, "datetime_trunc": -1, "time_trunc": -1, + "timestamp_trunc": -1, "datetime_trunc": -1, "time_trunc": -1, "date_diff": -1, "timestamp_diff": -1, "datetime_diff": -1, "time_diff": -1, "datetrunc": 0, "datediff": 0, "dateadd": 0, "datepart": 0, "date_part": 0, } # fmt: skip + # DATE_TRUNC has NO fixed part position: BigQuery is DATE_TRUNC(value, part) while Snowflake is + # DATE_TRUNC(part, expr), and the adapter has no dialect context. Disambiguate by CONTENT -- + # whichever argument is a date-part keyword is the part (see _is_date_part_argument). + _DATE_PART_AMBIGUOUS_TRUNC = frozenset({"date_trunc"}) @staticmethod def _enclosing_call(pre: str) -> tuple[str | None, int, int]: @@ -520,6 +524,28 @@ def _enclosing_call(pre: str) -> tuple[str | None, int, int]: i -= 1 return None, 0, -1 + @classmethod + def _enclosing_call_arg_texts(cls, pre: str, suf: str, token: str) -> list[str]: + """Argument texts of the call a token sits in, reconstructed from ``pre``/``token``/``suf``. + + Used to disambiguate a call whose date-part slot is not fixed (see + ``_DATE_PART_AMBIGUOUS_TRUNC``). Returns [] when the token is not inside a call. + """ + _, _, open_pos = cls._enclosing_call(pre) + if open_pos < 0: + return [] + depth, end = 0, len(suf) + for i, ch in enumerate(suf): + if ch in "([": + depth += 1 + elif ch in ")]": + if depth == 0: + end = i + break + depth -= 1 + call_args = pre[open_pos + 1 :] + token + suf[:end] + return [a.strip() for a in cls._split_top_level_commas(call_args, quote_aware=True)] + @classmethod def _is_date_part_argument(cls, pre: str, suf: str, token: str) -> bool: """True if ``token`` occupies the date-PART argument slot of a date/time call. @@ -533,6 +559,17 @@ def _is_date_part_argument(cls, pre: str, suf: str, token: str) -> bool: func, arg_index, open_pos = cls._enclosing_call(pre) if not func: return False + if func.lower() in cls._DATE_PART_AMBIGUOUS_TRUNC: + # DATE_TRUNC(a, b) is BigQuery (value, part) OR Snowflake (part, expr) -- same name, + # opposite orders. Decide by CONTENT: the argument that IS a date-part keyword is the + # part. When BOTH are keywords (DATE_TRUNC(date, month)) it is genuinely ambiguous, so + # take the LAST as the part (BigQuery's order) and let the other resolve as a column. + args = cls._enclosing_call_arg_texts(pre, suf, token) + if len(args) != 2: + return False + if arg_index == 1: + return True + return args[1].strip().lower() not in cls._DATE_PART_KEYWORDS want = cls._DATE_PART_ARG_POS.get(func.lower()) if want is None: return False @@ -3212,12 +3249,18 @@ def _aggregate_references_column(sql: str) -> bool: set is empty, so compiling it builds an empty model CTE (``SELECT ... FROM`` with no select list). Callers use this to skip such measures. On a parse failure assume it DOES reference a column, so a genuine (unparseable) column expression is not wrongly dropped. + + Liquid/Jinja segments are neutralised to NULL first: a template is not a column, and left + in place it makes sqlglot fail, which the fallback above would read as "has columns" -- so + a folded filter that is ONLY a template (``COUNT(CASE WHEN ({{ user_filter }}) THEN 1 END)``) + would slip past the zero-column guard and export a measure with no real column. """ import sqlglot from sqlglot import expressions as exp + neutralised = re.sub(r"\{\{.*?\}\}|\{%.*?%\}", "NULL", sql or "") try: - tree = sqlglot.parse_one(sql.replace("{model}", "__m__").replace("${TABLE}", "__m__")) + tree = sqlglot.parse_one(neutralised.replace("{model}", "__m__").replace("${TABLE}", "__m__")) except Exception: return True return any(True for _ in tree.find_all(exp.Column)) diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 7ba4866e..70a00f66 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4758,6 +4758,59 @@ def test_lookml_export_folded_filter_date_part_guard_is_position_aware(): assert conds(["time_bucket(INTERVAL '5 minutes', date) = 1"], model) == ( "(time_bucket(INTERVAL '5 minutes', (${TABLE}.order_date)) = 1)" ) + # DATE_TRUNC has NO fixed part position -- BigQuery is (value, part), Snowflake is (part, expr) + # -- so it is disambiguated by CONTENT: whichever argument is a date-part keyword is the part. + assert conds(["DATE_TRUNC(month, created_at) = DATE '2024-01-01'"], model) == ( + "(DATE_TRUNC(month, (${TABLE}.created_at)) = DATE '2024-01-01')" + ) # Snowflake order + # ...and when BOTH arguments are keywords it is genuinely ambiguous, so the LAST is the part + # (BigQuery's order) and the other still resolves as a column. + assert conds(["DATE_TRUNC(date, month) = DATE '2024-01-01'"], model) == ( + "(DATE_TRUNC((${TABLE}.order_date), month) = DATE '2024-01-01')" + ) + + +def test_lookml_export_template_only_folded_filter_skipped(): + """A folded filter that is ONLY a Liquid template leaves no real column -> skip the measure. + + sqlglot cannot parse a Liquid segment, and the column check treats a parse failure as + "has columns", so COUNT(*) with filters: ["{{ user_filter }}"] folded to + COUNT(CASE WHEN ({{ user_filter }}) THEN 1 END) slipped past the zero-column guard and + exported a type: number with no real column. Templates are neutralised before parsing; a + template COMBINED with a real column still exports. + """ + import re + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + def export_measure(filters): + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="status"), + ], + metrics=[Metric(name="c", agg=None, sql="COUNT(*)", sql_is_complete=True, filters=filters)], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + m = re.search(r"measure: c \{.*?\n \}", open(out).read(), re.S) + return m.group(0) if m else None + + assert export_measure(["{{ user_filter }}"]) is None # template-only -> no real column + assert export_measure(["{model}.status = 'x'"]) is not None # real column filter still exports + assert export_measure(["{model}.status = 'x' AND {{ f }}"]) is not None # template + column + + # The helper itself: a template is not a column, but a genuine column alongside one is. + assert not LookMLAdapter._aggregate_references_column("COUNT(CASE WHEN ({{ user_filter }}) THEN 1 END)") + assert LookMLAdapter._aggregate_references_column("COUNT(CASE WHEN (status = 'x' AND {{ f }}) THEN 1 END)") def test_lookml_export_folded_filter_does_not_rewrite_table_qualifier(): From ce800b32731c075930628eda70adf610bb9bc525 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Thu, 16 Jul 2026 22:33:52 -0700 Subject: [PATCH 12/44] Treat COUNT(ALL ) as a native row count An explicit ALL modifier is the default and does not change the count, but the native-count guard only accepted a constant immediately after COUNT(, so COUNT(ALL 1) / COUNT(ALL TRUE) fell through to the zero-column check and were dropped instead of exporting as type: count. Accept an optional ALL. Also strip the modifier before the column check: sqlglot cannot parse an aggregate's ALL, so EVERY COUNT(ALL ...) was hitting the has-columns fallback rather than a real check -- which let COUNT(ALL NULL) export a broken zero-column type: number. It now skips like COUNT(NULL), while COUNT(ALL ) still exports. A column literally named 'all' remains a plain argument. --- sidemantic/adapters/lookml.py | 13 +++++++++++-- tests/adapters/lookml/test_edge_cases.py | 9 +++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 1ffb353b..652fa080 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -3259,6 +3259,11 @@ def _aggregate_references_column(sql: str) -> bool: from sqlglot import expressions as exp neutralised = re.sub(r"\{\{.*?\}\}|\{%.*?%\}", "NULL", sql or "") + # sqlglot cannot parse an aggregate's ALL modifier (COUNT(ALL x)), which would send every + # such expression to the has-columns fallback below instead of a real check. ALL is the + # default modifier and irrelevant to which columns are referenced, so drop it first. + # `(` must precede it, so `= ALL (SELECT ...)` is untouched. + neutralised = re.sub(r"(?i)\(\s*ALL\s+", "(", neutralised) try: tree = sqlglot.parse_one(neutralised.replace("{model}", "__m__").replace("${TABLE}", "__m__")) except Exception: @@ -3619,10 +3624,14 @@ def _export_view(self, model: Model, graph: SemanticGraph) -> dict: # A COUNT over any NON-NULL constant counts every row -- it is a native # row count, identical to type: count: `*`, an int/decimal (1, 0, 1.0, # .5), a boolean (TRUE/FALSE), or a string literal ('x'). COUNT(NULL) is - # deliberately excluded -- it is always 0, not a row count. + # deliberately excluded -- it is always 0, not a row count. An explicit + # ALL modifier (COUNT(ALL 1)) is the default and does not change the count, + # so accept it too rather than dropping the metric at the zero-column check + # below. The trailing \s+ keeps a column literally named `all` (COUNT(all)) + # a plain argument. _count_const = r"\*|[+-]?(?:\d+\.?\d*|\.\d+)|true|false|'(?:[^']|'')*'" if not metric.filters and re.fullmatch( - rf"(?i)count\s*\(\s*(?:{_count_const})\s*\)", col_sql.strip() + rf"(?i)count\s*\(\s*(?:all\s+)?(?:{_count_const})\s*\)", col_sql.strip() ): # These reference no column; a type: number would re-import as a # derived metric over an empty CTE (SELECT FROM ...), which the diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 70a00f66..b029fb42 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -5268,6 +5268,15 @@ def export_measure(expr): for expr in ("COUNT(NULL)", "COUNT(DISTINCT 1)", "SUM(1)", "MAX('x')"): assert export_measure(expr) is None, f"{expr} should be skipped, not exported" + # An explicit ALL modifier is the default and does not change the count, so COUNT(ALL ) + # is the same native row count. (sqlglot cannot parse ALL, so the column check strips it -- + # otherwise every ALL form fell back to "has columns" and COUNT(ALL NULL) exported broken.) + for expr in ("COUNT(ALL 1)", "COUNT(ALL TRUE)", "COUNT(ALL 'x')"): + block = export_measure(expr) + assert block and "type: count" in block, f"{expr} -> {block}" + assert export_measure("COUNT(ALL NULL)") is None # still not a row count + assert "type: number" in (export_measure("COUNT(ALL {model}.id)") or "") # a real column stays + def test_lookml_export_spaced_count_star_maps_to_native_count(): """A spaced `COUNT (*)` complete aggregate must still export as native type: count.""" From 627665242b50a396078f305d6bbb28352192784c Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Thu, 16 Jul 2026 22:39:09 -0700 Subject: [PATCH 13/44] Pick the DATE_TRUNC part by coarseness when both arguments are keywords Taking the LAST argument as the part when both are date-part keywords only read BigQuery's DATE_TRUNC(value, part). Under Snowflake's DATE_TRUNC(part, expr) on a model with both a 'date' and a 'month' dimension, DATE_TRUNC(month, date) then resolved 'month' as a column and protected 'date' -- exactly backwards. Neither position is decisive when both arguments look like parts, but coarseness is: truncation goes finer -> coarser, so the COARSER keyword is the part and the other is the value column. That reads DATE_TRUNC(date, month) and DATE_TRUNC(month, date) identically -- month truncates date -- which is correct under either dialect's order, and generalizes (year beats month either way). Ties fall back to BigQuery's order; a single keyword argument still decides on its own. --- sidemantic/adapters/lookml.py | 39 ++++++++++++++++++++---- tests/adapters/lookml/test_edge_cases.py | 8 +++-- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 652fa080..36c402c0 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -500,6 +500,20 @@ def _sql_has_list_aggregate(sql: str) -> bool: # DATE_TRUNC(part, expr), and the adapter has no dialect context. Disambiguate by CONTENT -- # whichever argument is a date-part keyword is the part (see _is_date_part_argument). _DATE_PART_AMBIGUOUS_TRUNC = frozenset({"date_trunc"}) + # Coarseness rank of each date-part keyword (LOWER = coarser). Used only to break the tie when + # BOTH arguments of an ambiguous DATE_TRUNC are keywords (a model with `date` AND `month` + # dimensions): truncation always goes finer -> coarser, so the COARSER keyword is the part and + # the other is the value column. That reads DATE_TRUNC(date, month) and DATE_TRUNC(month, date) + # the same way -- month truncates date -- which is right under either dialect's order. + _DATE_PART_RANK = { + "millennium": 0, "century": 1, "decade": 2, + "year": 3, "isoyear": 3, "yearofweek": 3, + "quarter": 4, "month": 5, "week": 6, "isoweek": 6, + "day": 7, "date": 7, "dayofweek": 7, "dayofyear": 7, + "dow": 7, "doy": 7, "weekday": 7, "isodow": 7, + "hour": 8, "time": 8, "minute": 9, "second": 10, + "millisecond": 11, "microsecond": 12, "nanosecond": 13, "epoch": 14, + } # fmt: skip @staticmethod def _enclosing_call(pre: str) -> tuple[str | None, int, int]: @@ -561,15 +575,28 @@ def _is_date_part_argument(cls, pre: str, suf: str, token: str) -> bool: return False if func.lower() in cls._DATE_PART_AMBIGUOUS_TRUNC: # DATE_TRUNC(a, b) is BigQuery (value, part) OR Snowflake (part, expr) -- same name, - # opposite orders. Decide by CONTENT: the argument that IS a date-part keyword is the - # part. When BOTH are keywords (DATE_TRUNC(date, month)) it is genuinely ambiguous, so - # take the LAST as the part (BigQuery's order) and let the other resolve as a column. + # opposite orders. Decide by CONTENT, not position: the argument that IS a date-part + # keyword is the part, and the other is the value column. args = cls._enclosing_call_arg_texts(pre, suf, token) if len(args) != 2: return False - if arg_index == 1: - return True - return args[1].strip().lower() not in cls._DATE_PART_KEYWORDS + first, second = args[0].strip().lower(), args[1].strip().lower() + first_kw = first in cls._DATE_PART_KEYWORDS + second_kw = second in cls._DATE_PART_KEYWORDS + if first_kw and not second_kw: + part_index = 0 # Snowflake order + elif second_kw and not first_kw: + part_index = 1 # BigQuery order + elif first_kw and second_kw: + # A model with e.g. BOTH `date` and `month` dimensions makes both arguments look + # like parts. Truncation goes finer -> coarser, so the COARSER one is the part -- + # month truncates date under either order. Ties fall back to BigQuery's order. + first_rank = cls._DATE_PART_RANK.get(first, 99) + second_rank = cls._DATE_PART_RANK.get(second, 99) + part_index = 0 if first_rank < second_rank else 1 + else: + return False + return arg_index == part_index want = cls._DATE_PART_ARG_POS.get(func.lower()) if want is None: return False diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index b029fb42..4ee5d99d 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4763,11 +4763,15 @@ def test_lookml_export_folded_filter_date_part_guard_is_position_aware(): assert conds(["DATE_TRUNC(month, created_at) = DATE '2024-01-01'"], model) == ( "(DATE_TRUNC(month, (${TABLE}.created_at)) = DATE '2024-01-01')" ) # Snowflake order - # ...and when BOTH arguments are keywords it is genuinely ambiguous, so the LAST is the part - # (BigQuery's order) and the other still resolves as a column. + # When BOTH arguments are keywords (a model with `date` AND `month` dimensions) neither order + # is decisive, so coarseness decides: truncation goes finer -> coarser, so `month` is the part + # and `date` the column -- the SAME reading under either dialect's argument order. assert conds(["DATE_TRUNC(date, month) = DATE '2024-01-01'"], model) == ( "(DATE_TRUNC((${TABLE}.order_date), month) = DATE '2024-01-01')" ) + assert conds(["DATE_TRUNC(month, date) = DATE '2024-01-01'"], model) == ( + "(DATE_TRUNC(month, (${TABLE}.order_date)) = DATE '2024-01-01')" + ) def test_lookml_export_template_only_folded_filter_skipped(): From fd91b4794d037e7a9f32766c6e9b58afe178ee3b Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Thu, 16 Jul 2026 22:50:27 -0700 Subject: [PATCH 14/44] Recognize a QUOTED DATE_TRUNC part when picking the column argument Postgres/DuckDB write the part quoted: DATE_TRUNC('month', date). The literal splitter already protects that token from rewriting, but the keyword check matched the raw argument text, so "'month'" never matched the keyword set. The OTHER argument then looked like the only keyword, and a real column named 'date' was treated as the part and left unresolved -- while the filters block stayed suppressed. Strip surrounding quotes before matching, so a quoted part is recognized and the column argument resolves. Covers single and double quotes, and leaves the unquoted BigQuery/Snowflake orders and the both-keyword coarseness tiebreak intact. --- sidemantic/adapters/lookml.py | 7 ++++++- tests/adapters/lookml/test_edge_cases.py | 9 +++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 36c402c0..d2fe09ae 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -580,7 +580,12 @@ def _is_date_part_argument(cls, pre: str, suf: str, token: str) -> bool: args = cls._enclosing_call_arg_texts(pre, suf, token) if len(args) != 2: return False - first, second = args[0].strip().lower(), args[1].strip().lower() + # Strip quotes before matching: Postgres/DuckDB write the part QUOTED + # (DATE_TRUNC('month', date)). The quoted token is already protected from rewriting by + # the literal splitter, but it must still be RECOGNISED as the part here -- otherwise + # the other argument looks like the only keyword and a real column named `date` is + # left unresolved. + first, second = (a.strip().strip("'\"").lower() for a in (args[0], args[1])) first_kw = first in cls._DATE_PART_KEYWORDS second_kw = second in cls._DATE_PART_KEYWORDS if first_kw and not second_kw: diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 4ee5d99d..594fa3ed 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4772,6 +4772,15 @@ def test_lookml_export_folded_filter_date_part_guard_is_position_aware(): assert conds(["DATE_TRUNC(month, date) = DATE '2024-01-01'"], model) == ( "(DATE_TRUNC(month, (${TABLE}.order_date)) = DATE '2024-01-01')" ) + # Postgres/DuckDB quote the part. The quoted token is already protected from rewriting, but it + # must still be RECOGNISED as the part -- otherwise the `date` COLUMN looks like the only + # keyword and is left unresolved. + assert conds(["DATE_TRUNC('month', date) = DATE '2024-01-01'"], model) == ( + "(DATE_TRUNC('month', (${TABLE}.order_date)) = DATE '2024-01-01')" + ) + assert conds(["DATE_TRUNC(\"month\", date) = DATE '2024-01-01'"], model) == ( + "(DATE_TRUNC(\"month\", (${TABLE}.order_date)) = DATE '2024-01-01')" + ) def test_lookml_export_template_only_folded_filter_skipped(): From 1454303b58715805a28527e31b729435ce4a2edb Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Thu, 16 Jul 2026 23:05:10 -0700 Subject: [PATCH 15/44] Guard the stddev/variance export path against zero-column aggregates The stddev/variance branch also emits a type: number measure, but unlike the agg-less path it never checked whether the SQL references a column. A constant expression -- Metric(agg=stddev, sql=1) -> STDDEV(1) -- therefore exported a measure whose re-import builds a complete-SQL metric with no source columns, so compiling it hits the empty model CTE this guard exists to avoid. Apply the same check here: skip an unfiltered aggregate that references no column, and re-check after folding since a filter need not add one either (a constant predicate or a pure template). Column-based aggregates, filtered or not, and natively-mapped aggregates are unaffected. --- sidemantic/adapters/lookml.py | 27 +++++++++++++- tests/adapters/lookml/test_edge_cases.py | 45 ++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index d2fe09ae..538af78e 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -3622,8 +3622,22 @@ def _export_view(self, model: Model, graph: SemanticGraph) -> dict: if col_sql: measure_def["sql"] = col_sql elif metric.agg in sql_agg_funcs and col_sql: - measure_def["type"] = "number" agg_sql = f"{sql_agg_funcs[metric.agg]}({col_sql})" + # This path also emits a type: number, so it needs the same zero-column + # guard as the agg-less branch above: STDDEV(1) over a constant references + # no column, and re-importing builds a metric over an empty model CTE + # (SELECT ... FROM with no select list). Checked BEFORE folding; a filter + # that adds a column is re-checked after it (below). + if not self._aggregate_references_column(agg_sql) and not metric.filters: + logger.warning( + "Metric %r (agg=%r) has a zero-column aggregate SQL (%r) with no " + "LookML equivalent that round-trips; skipping on export.", + metric.name, + metric.agg, + agg_sql, + ) + continue + measure_def["type"] = "number" if metric.filters: # type: number re-imports as a derived metric whose generator # does not apply LookML `filters`, so fold them into the aggregate @@ -3640,6 +3654,17 @@ def _export_view(self, model: Model, graph: SemanticGraph) -> dict: metric.agg, ) continue + # A filter does not necessarily add a column (a constant predicate or a + # pure template), so the folded SQL can still reference none. + if not self._aggregate_references_column(folded): + logger.warning( + "Metric %r (agg=%r) folds to a zero-column aggregate SQL (%r) " + "with no LookML equivalent that round-trips; skipping on export.", + metric.name, + metric.agg, + folded, + ) + continue measure_def["sql"] = folded filters_folded = True else: diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 594fa3ed..92283e0e 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -5023,6 +5023,51 @@ def test_lookml_export_string_literal_paren_in_aggregate_arg_folds(): assert "DISTINCT CASE WHEN" in text +def test_lookml_export_zero_column_stddev_skipped(): + """A stddev/variance over a CONSTANT also emits type: number -> needs the zero-column guard. + + STDDEV(1) references no column, so re-importing builds a metric over an empty model CTE -- + the same failure the agg-less path already guards. A filter needn't add a column either, so + the folded SQL is re-checked. Column-based and native aggregates are unaffected. + """ + import re + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + def export_measure(**metric_kwargs): + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="status"), + Dimension(name="amount", type="numeric", sql="amount"), + ], + metrics=[Metric(name="c", **metric_kwargs)], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + m = re.search(r"measure: c \{.*?\n \}", open(out).read(), re.S) + return m.group(0) if m else None + + assert export_measure(agg="stddev", sql="1") is None # STDDEV(1) -> no column + assert export_measure(agg="variance", sql="1") is None + assert export_measure(agg="stddev", sql="1", filters=["1 = 1"]) is None # filter adds no column + # A real column still exports, filtered or not. + assert "type: number" in (export_measure(agg="stddev", sql="{model}.amount") or "") + assert "type: number" in ( + export_measure(agg="stddev", sql="{model}.amount", filters=["{model}.status = 'x'"]) or "" + ) + # A natively-mapped aggregate is untouched. + assert "type: sum" in (export_measure(agg="sum", sql="{model}.amount") or "") + + def test_lookml_export_folded_zero_column_aggregate_skipped(): """A filter that adds NO column must not sneak a zero-column aggregate past the guard. From 7bcab4dedbe3dc2694623a060d10767c701e78ca Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Thu, 16 Jul 2026 23:18:35 -0700 Subject: [PATCH 16/44] Normalize the ALL modifier out of exported complete aggregates An opaque complete aggregate written with the explicit default modifier -- COUNT(ALL id), SUM(ALL amount) -- was exported verbatim as type: number. sqlglot cannot parse an aggregate's ALL, so the type: number import safety check dropped the measure and the round-trip silently lost it. ALL is the default and changes nothing, so strip it on export, leaving equivalent SQL that re-imports. DISTINCT is NOT a no-op and is preserved; a column literally named `all` is untouched (a `(` must precede the modifier, which also leaves `= ALL (SELECT ...)` alone). --- sidemantic/adapters/lookml.py | 8 +++++ tests/adapters/lookml/test_edge_cases.py | 46 ++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 538af78e..8f3312b8 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -3610,6 +3610,14 @@ def _export_view(self, model: Model, graph: SemanticGraph) -> dict: "variance_pop": "VAR_POP", } col_sql = metric.sql.replace("{model}", "${TABLE}") if metric.sql else None + # Drop an explicit ALL aggregate modifier: it is the DEFAULT and changes + # nothing, but emitting it produces LookML that will not round-trip -- sqlglot + # cannot parse `COUNT(ALL x)`, so the type: number import safety check drops + # the measure. Normalizing here keeps the exported SQL equivalent AND readable + # back. (`(` must precede it, so `= ALL (SELECT ...)` and a column named `all` + # are untouched.) + if col_sql: + col_sql = re.sub(r"(?i)\(\s*ALL\s+", "(", col_sql) if metric.agg == "approx_count_distinct": # Looker represents this as count_distinct with approximate: yes. diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 92283e0e..1fd1087b 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -5023,6 +5023,52 @@ def test_lookml_export_string_literal_paren_in_aggregate_arg_folds(): assert "DISTINCT CASE WHEN" in text +def test_lookml_export_normalizes_all_modifier_for_roundtrip(): + """An explicit ALL modifier must be normalized away so the exported measure round-trips. + + ALL is the DEFAULT aggregate modifier and changes nothing, but sqlglot cannot parse + COUNT(ALL x), so a type: number measure exported with it is DROPPED on re-import. Strip it on + export; DISTINCT (which is NOT a no-op) and a column named `all` must survive untouched. + """ + import re + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + def roundtrip(expr): + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="amount", type="numeric", sql="amount"), + ], + metrics=[Metric(name="c", agg=None, sql=expr, sql_is_complete=True)], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + block = re.search(r"measure: c \{.*?\n \}", text, re.S) + reimported = "c" in {m.name for m in LookMLAdapter().parse(Path(out)).get_model("o").metrics} + return (block.group(0) if block else None), reimported + + for expr in ("COUNT(ALL {model}.id)", "SUM(ALL {model}.amount)"): + block, kept = roundtrip(expr) + assert block and "ALL" not in block, block # normalized away + assert kept, f"{expr} must survive the round-trip" + + # DISTINCT is not a no-op and must be preserved; a plain aggregate is unchanged. + block, kept = roundtrip("COUNT(DISTINCT {model}.id)") + assert block and "DISTINCT" in block and kept + block, kept = roundtrip("SUM({model}.amount)") + assert block and kept + + def test_lookml_export_zero_column_stddev_skipped(): """A stddev/variance over a CONSTANT also emits type: number -> needs the zero-column guard. From 17445f4adf0c514d2e442117ddc67cfd069eb91f Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Thu, 16 Jul 2026 23:38:57 -0700 Subject: [PATCH 17/44] Protect date parts in TIMESTAMPADD/TIMESTAMPDIFF The date-part argument map omitted the unspaced TIMESTAMPADD / TIMESTAMPDIFF aliases, which take the part as their FIRST argument. On a model with a `day` dimension a folded filter like TIMESTAMPADD(day, 1, created_at) rewrote the part as a column -- TIMESTAMPADD((${TABLE}.order_day), 1, ...) -- while the folded export suppressed the separate filters block, so the emitted LookML SQL was invalid. Add them to the first-argument guard. The column arguments of the same call still resolve, and a keyword-named column outside a date function is unaffected. --- sidemantic/adapters/lookml.py | 1 + tests/adapters/lookml/test_edge_cases.py | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 8f3312b8..2bdb3b91 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -495,6 +495,7 @@ def _sql_has_list_aggregate(sql: str) -> bool: "timestamp_trunc": -1, "datetime_trunc": -1, "time_trunc": -1, "date_diff": -1, "timestamp_diff": -1, "datetime_diff": -1, "time_diff": -1, "datetrunc": 0, "datediff": 0, "dateadd": 0, "datepart": 0, "date_part": 0, + "timestampadd": 0, "timestampdiff": 0, "timestampdiff_big": 0, "datetimediff": 0, } # fmt: skip # DATE_TRUNC has NO fixed part position: BigQuery is DATE_TRUNC(value, part) while Snowflake is # DATE_TRUNC(part, expr), and the adapter has no dialect context. Disambiguate by CONTENT -- diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 1fd1087b..7ed2e21c 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4748,6 +4748,13 @@ def test_lookml_export_folded_filter_date_part_guard_is_position_aware(): assert conds(["DATEDIFF(day, created_at, date) > 1"], model) == ( "(DATEDIFF(day, (${TABLE}.created_at), (${TABLE}.order_date)) > 1)" ) + # The unspaced TIMESTAMPADD/TIMESTAMPDIFF aliases also take the part first. + assert conds(["TIMESTAMPADD(day, 1, created_at) > 1"], model) == ( + "(TIMESTAMPADD(day, 1, (${TABLE}.created_at)) > 1)" + ) + assert conds(["TIMESTAMPDIFF(day, created_at, date) > 1"], model) == ( + "(TIMESTAMPDIFF(day, (${TABLE}.created_at), (${TABLE}.order_date)) > 1)" + ) # The two TRUNC spellings differ: underscored DATE_TRUNC is BigQuery's (value, part), while # the unspaced DATETRUNC is SQL Server's (part, value). assert conds(["DATETRUNC(month, created_at) = DATE '2024-01-01'"], model) == ( From 7b5f02fe5e4099ecc2e5d60d222899e40908f3e9 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 09:51:24 -0700 Subject: [PATCH 18/44] Protect SQL Server datepart-first functions in folded filters DATENAME, DATEDIFF_BIG, and DATE_BUCKET take the datepart as their first argument but were absent from the datepart-position map, so a folded filter's DATENAME(day, created_at) on a model with a `day` dimension rewrote the part token to (${TABLE}.order_day). The stddev/complete-aggregate export paths suppress the filters block, so the exported LookML SQL was then invalid. datediff_big is the sibling of the already-listed timestampdiff_big; add it alongside datename and date_bucket to complete the datepart-first family. --- sidemantic/adapters/lookml.py | 5 ++++ tests/adapters/lookml/test_edge_cases.py | 33 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 2bdb3b91..a270fa7a 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -496,6 +496,11 @@ def _sql_has_list_aggregate(sql: str) -> bool: "date_diff": -1, "timestamp_diff": -1, "datetime_diff": -1, "time_diff": -1, "datetrunc": 0, "datediff": 0, "dateadd": 0, "datepart": 0, "date_part": 0, "timestampadd": 0, "timestampdiff": 0, "timestampdiff_big": 0, "datetimediff": 0, + # SQL Server functions taking the datepart as the FIRST argument. datediff_big is the + # sibling of the already-listed timestampdiff_big; datename and date_bucket round out the + # datepart-first family. Without them a folded filter's DATENAME(day, col) leaves `day` + # unprotected, so a model with a `day` dimension rewrites it to (${TABLE}.order_day). + "datename": 0, "datediff_big": 0, "date_bucket": 0, } # fmt: skip # DATE_TRUNC has NO fixed part position: BigQuery is DATE_TRUNC(value, part) while Snowflake is # DATE_TRUNC(part, expr), and the adapter has no dialect context. Disambiguate by CONTENT -- diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 7ed2e21c..863dedf5 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4790,6 +4790,39 @@ def test_lookml_export_folded_filter_date_part_guard_is_position_aware(): ) +def test_lookml_export_folded_filter_protects_sql_server_datepart_first_functions(): + """SQL Server functions taking the datepart FIRST must protect that part token. + + DATENAME / DATEDIFF_BIG / DATE_BUCKET were absent from the datepart-position map, so on a model + with a `day` dimension a folded filter's DATENAME(day, created_at) rewrote `day` to + (${TABLE}.order_day) -- and the stddev/complete-aggregate export paths suppress the filters + block, so the exported LookML SQL was invalid. The trailing column arguments still resolve. + """ + from sidemantic import Dimension, Model + + model = Model( + name="orders", + table="raw_orders", + primary_key="id", + dimensions=[ + Dimension(name="day", type="time", granularity="day", sql="order_day"), + Dimension(name="created_at", type="time", granularity="day", sql="created_at"), + Dimension(name="start_at", type="time", granularity="day", sql="started_at"), + ], + ) + conds = LookMLAdapter._fold_filter_conds + # The datepart token is protected; the column argument(s) of the SAME call still resolve. + assert conds(["DATENAME(day, created_at) = 'Monday'"], model) == ( + "(DATENAME(day, (${TABLE}.created_at)) = 'Monday')" + ) + assert conds(["DATEDIFF_BIG(day, start_at, created_at) > 1"], model) == ( + "(DATEDIFF_BIG(day, (${TABLE}.started_at), (${TABLE}.created_at)) > 1)" + ) + assert conds(["DATE_BUCKET(day, 1, created_at) > 1"], model) == ("(DATE_BUCKET(day, 1, (${TABLE}.created_at)) > 1)") + # A keyword-named column used for real (outside a date function) is still rewritten. + assert conds(["UPPER(day) = 'X'"], model) == "(UPPER((${TABLE}.order_day)) = 'X')" + + def test_lookml_export_template_only_folded_filter_skipped(): """A folded filter that is ONLY a Liquid template leaves no real column -> skip the measure. From 8ca16cc4114cc74abd80d13668ce8f50db5ffde5 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 10:20:50 -0700 Subject: [PATCH 19/44] Protect multiline Liquid blocks when folding filters The folded-filter template guard split and neutralised Liquid/Jinja segments with `.*?`, which does not span newlines, so a `{% ... %}` or `{{ ... }}` tag broken across lines was not protected. A bare dimension name on an inner line of such a tag was then rewritten to its column, corrupting the template while the normal filters block is suppressed. Use `[\s\S]*?` for both template patterns so a tag spanning newlines stays one protected segment. Only those alternatives use `.`; the quoted-string patterns match character classes, so nothing else changes. --- sidemantic/adapters/lookml.py | 8 +++++-- tests/adapters/lookml/test_edge_cases.py | 29 ++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index a270fa7a..fa87535e 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -3264,9 +3264,12 @@ def _one(m): # Rewrite refs only in the remaining (even-index) segments so string VALUES, quoted # identifiers, typed literals, and template variables are untouched. The template # patterns require DOUBLE braces / brace-percent, so the single-brace {model} is safe. + # They use [\s\S]*? (not .*?) so a Liquid/Jinja tag that SPANS NEWLINES is still one + # protected segment -- otherwise a bare dimension name on an inner line of a multiline + # {% ... %} / {{ ... }} would be rewritten, corrupting the template. parts = re.split( r"""((?i:\b(?:date|time|timestamp|timestamptz|datetime|interval)\s+'(?:[^']|'')*')""" - r"""|'(?:[^']|'')*'|"(?:[^"]|"")*"|`[^`]*`|\[[^\]]*\]|\{\{.*?\}\}|\{%.*?%\})""", + r"""|'(?:[^']|'')*'|"(?:[^"]|"")*"|`[^`]*`|\[[^\]]*\]|\{\{[\s\S]*?\}\}|\{%[\s\S]*?%\})""", fstr, ) offset = 0 @@ -3296,7 +3299,8 @@ def _aggregate_references_column(sql: str) -> bool: import sqlglot from sqlglot import expressions as exp - neutralised = re.sub(r"\{\{.*?\}\}|\{%.*?%\}", "NULL", sql or "") + # [\s\S]*? (not .*?) so a Liquid/Jinja tag spanning newlines is neutralised as one unit. + neutralised = re.sub(r"\{\{[\s\S]*?\}\}|\{%[\s\S]*?%\}", "NULL", sql or "") # sqlglot cannot parse an aggregate's ALL modifier (COUNT(ALL x)), which would send every # such expression to the has-columns fallback below instead of a real check. ALL is the # default modifier and irrelevant to which columns are referenced, so drop it first. diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 863dedf5..fc2cdabc 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4590,6 +4590,35 @@ def test_lookml_export_folded_filter_does_not_rewrite_template_variable(): assert "order_status" in text # the real column operand IS resolved +def test_lookml_export_folded_filter_protects_multiline_liquid_block(): + """A Liquid/Jinja tag that SPANS NEWLINES must be protected whole, not corrupted mid-tag. + + The template patterns used `.*?` without DOTALL, so a `{% ... %}` / `{{ ... }}` spanning a + newline was not split out as one protected segment, and a bare dimension name on an inner line + was rewritten to its column -- corrupting the template. Using `[\\s\\S]*?` spans newlines. + """ + from sidemantic import Dimension, Model + + model = Model( + name="orders", + table="raw", + primary_key="id", + dimensions=[Dimension(name="status", type="categorical", sql="order_status")], + ) + conds = LookMLAdapter._fold_filter_conds + + # Each tag itself spans a newline: the inner `status` must NOT become `order_status`. + for predicate in ( + "{% condition\n status %}\n1\n{% endcondition %}", + "{{\n status\n}} = 1", + "{% condition\nstatus\n%} 1 {% endcondition %}", + ): + assert "order_status" not in conds([predicate], model), predicate + + # A real bare-dimension filter (no template) still resolves to its column. + assert conds(["status = 'x'"], model) == "((${TABLE}.order_status) = 'x')" + + def test_lookml_export_folded_filter_no_dimensions_does_not_crash(): """Folding a qualified filter on a model with NO dimensions must not IndexError. From b7ca0bad918e43b53eba8e53eb571b15daf7b14c Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 10:51:15 -0700 Subject: [PATCH 20/44] Resolve equal-rank DATE_TRUNC arguments by truncation unit When both DATE_TRUNC arguments were date-part keywords of the SAME coarseness rank -- DATE_TRUNC(day, date), both rank 7 -- the tie-break had no finer/coarser signal and fell back to a fixed position, picking `date` as the part. On a model where `date` is a renamed dimension that left the real column unresolved (and rewrote `day` if it was a dimension), producing invalid folded-export SQL. A DATE_TRUNC part must be a truncation UNIT. Among equal-rank keywords, prefer the one that is a real unit (day) as the part and resolve the other (date, an extraction-only keyword) as a column. Falls back to position only when both or neither is a unit. --- sidemantic/adapters/lookml.py | 26 ++++++++++++++++++-- tests/adapters/lookml/test_edge_cases.py | 30 ++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index fa87535e..c2d22b29 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -520,6 +520,14 @@ def _sql_has_list_aggregate(sql: str) -> bool: "hour": 8, "time": 8, "minute": 9, "second": 10, "millisecond": 11, "microsecond": 12, "nanosecond": 13, "epoch": 14, } # fmt: skip + # Keywords that are valid DATE_TRUNC truncation UNITS. The rest of _DATE_PART_KEYWORDS are + # extraction parts only (date, dow, doy, weekday, epoch, ...) -- you EXTRACT them but do not + # DATE_TRUNC to them. Used to break an equal-coarseness tie: DATE_TRUNC(day, date) shares rank 7 + # for both args, but only `day` is a trunc unit, so it is the part and `date` is the column. + _DATE_TRUNC_UNITS = frozenset( + {"millennium", "century", "decade", "year", "quarter", "month", "week", "day", + "hour", "minute", "second", "millisecond", "microsecond", "nanosecond"} + ) # fmt: skip @staticmethod def _enclosing_call(pre: str) -> tuple[str | None, int, int]: @@ -601,10 +609,24 @@ def _is_date_part_argument(cls, pre: str, suf: str, token: str) -> bool: elif first_kw and second_kw: # A model with e.g. BOTH `date` and `month` dimensions makes both arguments look # like parts. Truncation goes finer -> coarser, so the COARSER one is the part -- - # month truncates date under either order. Ties fall back to BigQuery's order. + # month truncates date under either order. first_rank = cls._DATE_PART_RANK.get(first, 99) second_rank = cls._DATE_PART_RANK.get(second, 99) - part_index = 0 if first_rank < second_rank else 1 + if first_rank != second_rank: + part_index = 0 if first_rank < second_rank else 1 + else: + # Equal coarseness (DATE_TRUNC(day, date): both rank 7) gives no finer/coarser + # signal. A DATE_TRUNC part must be a truncation UNIT, so if exactly one + # argument is a real unit (day) and the other an extraction-only keyword that is + # also a column (date), the unit is the part and the other resolves as a column. + first_unit = first in cls._DATE_TRUNC_UNITS + second_unit = second in cls._DATE_TRUNC_UNITS + if first_unit and not second_unit: + part_index = 0 + elif second_unit and not first_unit: + part_index = 1 + else: + part_index = 1 # both or neither a unit: fall back to BigQuery's order else: return False return arg_index == part_index diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index fc2cdabc..f2b71b68 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4819,6 +4819,36 @@ def test_lookml_export_folded_filter_date_part_guard_is_position_aware(): ) +def test_lookml_export_folded_filter_equal_rank_date_trunc_uses_trunc_unit_as_part(): + """When both DATE_TRUNC args share coarseness, the truncation UNIT is the part. + + DATE_TRUNC(day, date) has both args at rank 7, so the coarseness tie-break gave no signal and + fell back to a fixed position -- picking `date` as the part and leaving the real `date` column + unresolved (or rewriting `day`). A DATE_TRUNC part must be a truncation unit, and only `day` + is one (`date`/`dow`/`doy` are extraction-only), so `day` is the part and `date` resolves. + """ + from sidemantic import Dimension, Model + + model = Model( + name="orders", + table="raw", + primary_key="id", + dimensions=[ + Dimension(name="date", type="time", granularity="day", sql="order_date"), + Dimension(name="day", type="time", granularity="day", sql="order_day"), + ], + ) + conds = LookMLAdapter._fold_filter_conds + # Snowflake order (part first): day is the part, date resolves to its column. + assert conds(["DATE_TRUNC(day, date) = DATE '2024-01-01'"], model) == ( + "(DATE_TRUNC(day, (${TABLE}.order_date)) = DATE '2024-01-01')" + ) + # BigQuery order (part last): still day is the part, date resolves. + assert conds(["DATE_TRUNC(date, day) = DATE '2024-01-01'"], model) == ( + "(DATE_TRUNC((${TABLE}.order_date), day) = DATE '2024-01-01')" + ) + + def test_lookml_export_folded_filter_protects_sql_server_datepart_first_functions(): """SQL Server functions taking the datepart FIRST must protect that part token. From f9b3141ac7b33c3ccb0b2930407534ab64aea2dc Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 11:13:04 -0700 Subject: [PATCH 21/44] Keep ALL normalization out of string literals The ALL-modifier normalization (COUNT(ALL x) -> COUNT(x), so the exported measure round-trips) ran a global regex, so "(ALL " inside a string literal was also rewritten -- CASE WHEN label = '(ALL users)' THEN amount END exported as '(users)', changing which rows the metric includes. Add a quote-aware _strip_all_modifier that substitutes only outside string literals and quoted identifiers, and use it for both the export path and the has-columns check. --- sidemantic/adapters/lookml.py | 27 ++++++++++++++++++------ tests/adapters/lookml/test_edge_cases.py | 5 +++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index c2d22b29..06c9393f 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -701,6 +701,22 @@ def _has_subquery(sql: str) -> bool: ) return bool(re.search(r"(?is)\bselect\b", stripped)) + @staticmethod + def _strip_all_modifier(sql: str) -> str: + """Drop an explicit ``ALL`` aggregate modifier (``COUNT(ALL x)`` -> ``COUNT(x)``) OUTSIDE + string literals and quoted identifiers. + + ``ALL`` is the default modifier and changes nothing, but sqlglot cannot parse it, so + normalizing it away keeps exported SQL round-trippable and the column check accurate. A + string literal like ``'(ALL users)'`` is DATA, not a modifier, so the substitution runs only + on the segments outside any quoted token (``(`` must precede ``ALL`` regardless, so + ``= ALL (SELECT ...)`` and a column named ``all`` are already untouched). + """ + parts = re.split(r"""('(?:[^']|'')*'|"(?:[^"]|"")*"|`[^`]*`|\[[^\]]*\])""", sql or "") + for i in range(0, len(parts), 2): # even indices are outside any quoted token + parts[i] = re.sub(r"(?i)\(\s*ALL\s+", "(", parts[i]) + return "".join(parts) + @classmethod def _mixed_is_aggregate_safe(cls, sql: str, is_dim_ref, dim_sql_lookup: dict[str, str] | None = None) -> bool: """For a ``type: number`` measure that mixes measure refs with dimension refs, @@ -3325,9 +3341,9 @@ def _aggregate_references_column(sql: str) -> bool: neutralised = re.sub(r"\{\{[\s\S]*?\}\}|\{%[\s\S]*?%\}", "NULL", sql or "") # sqlglot cannot parse an aggregate's ALL modifier (COUNT(ALL x)), which would send every # such expression to the has-columns fallback below instead of a real check. ALL is the - # default modifier and irrelevant to which columns are referenced, so drop it first. - # `(` must precede it, so `= ALL (SELECT ...)` is untouched. - neutralised = re.sub(r"(?i)\(\s*ALL\s+", "(", neutralised) + # default modifier and irrelevant to which columns are referenced, so drop it first -- + # quote-aware, so a string literal containing "(ALL " is not mangled. + neutralised = LookMLAdapter._strip_all_modifier(neutralised) try: tree = sqlglot.parse_one(neutralised.replace("{model}", "__m__").replace("${TABLE}", "__m__")) except Exception: @@ -3646,10 +3662,9 @@ def _export_view(self, model: Model, graph: SemanticGraph) -> dict: # nothing, but emitting it produces LookML that will not round-trip -- sqlglot # cannot parse `COUNT(ALL x)`, so the type: number import safety check drops # the measure. Normalizing here keeps the exported SQL equivalent AND readable - # back. (`(` must precede it, so `= ALL (SELECT ...)` and a column named `all` - # are untouched.) + # back. Quote-aware, so a string literal like '(ALL users)' is left intact. if col_sql: - col_sql = re.sub(r"(?i)\(\s*ALL\s+", "(", col_sql) + col_sql = self._strip_all_modifier(col_sql) if metric.agg == "approx_count_distinct": # Looker represents this as count_distinct with approximate: yes. diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index f2b71b68..367f8628 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -5167,6 +5167,11 @@ def roundtrip(expr): block, kept = roundtrip("SUM({model}.amount)") assert block and kept + # `(ALL ` inside a STRING LITERAL is data, not a modifier -- it must be left intact so the + # metric still includes the same rows, while a real ALL modifier in the same SQL is stripped. + block, kept = roundtrip("SUM(ALL CASE WHEN {model}.amount > 0 THEN 1 ELSE 0 END) || '(ALL x)'") + assert block and "(ALL x)" in block and "SUM(ALL" not in block, block + def test_lookml_export_zero_column_stddev_skipped(): """A stddev/variance over a CONSTANT also emits type: number -> needs the zero-column guard. From 4256ef4c76217130ee797c8b163919d71e2cec92 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 11:34:57 -0700 Subject: [PATCH 22/44] Add SQL Server datepart abbreviations to the date-part keyword set DATEADD/DATEDIFF/DATENAME accept abbreviated dateparts (dd, mm, d, ...), but they were absent from the date-part keyword set, so a folded filter on a model with a dd/mm dimension rewrote DATEADD(dd, 1, created_at) into DATEADD((${TABLE}.order_dd), ...) and produced invalid SQL. Add Microsoft's accepted abbreviations via a canonical-name map, so they are recognised as keywords and their coarseness resolves for the ambiguous DATE_TRUNC tie-break. They are only protected in the part slot, so a same-named column elsewhere still resolves. --- sidemantic/adapters/lookml.py | 25 ++++++++++++++--- tests/adapters/lookml/test_edge_cases.py | 34 ++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 06c9393f..3d36dbc4 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -470,6 +470,18 @@ def _sql_has_list_aggregate(sql: str) -> bool: return any(True for _ in tree.find_all(exp.List)) or any(True for _ in tree.find_all(exp.ArrayAgg)) # SQL date-part keywords that a date/time function can take UNQUOTED. + # SQL Server DATEADD/DATEDIFF/DATENAME datepart ABBREVIATIONS -> canonical name (Microsoft's + # accepted forms). Folded filters see these in a date function's part slot, so they must be + # recognised as date-part keywords and resolved to the right coarseness. They are only ever + # protected in the part SLOT (position-guarded), so a single-letter column like `m`/`d` used + # anywhere else still resolves as a column. + _DATE_PART_ABBR = { + "yy": "year", "yyyy": "year", "qq": "quarter", "q": "quarter", + "mm": "month", "m": "month", "dy": "dayofyear", "y": "dayofyear", + "dd": "day", "d": "day", "wk": "week", "ww": "week", + "dw": "weekday", "w": "weekday", "hh": "hour", "mi": "minute", "n": "minute", + "ss": "second", "s": "second", "ms": "millisecond", "mcs": "microsecond", "ns": "nanosecond", + } # fmt: skip _DATE_PART_KEYWORDS = frozenset( { "year", "quarter", "month", "week", "day", "hour", "minute", "second", @@ -477,6 +489,7 @@ def _sql_has_list_aggregate(sql: str) -> bool: "dayofweek", "dayofyear", "dow", "doy", "isoweek", "isoyear", "isodow", "weekday", "yearofweek", "century", "decade", "millennium", } + | set(_DATE_PART_ABBR) ) # fmt: skip # Which ARGUMENT of a date/time call is the date part. Position matters: the same keyword can # be a real column in another slot -- DATE_TRUNC(date, month) on a model with BOTH a `date` @@ -602,6 +615,10 @@ def _is_date_part_argument(cls, pre: str, suf: str, token: str) -> bool: first, second = (a.strip().strip("'\"").lower() for a in (args[0], args[1])) first_kw = first in cls._DATE_PART_KEYWORDS second_kw = second in cls._DATE_PART_KEYWORDS + # Normalize an abbreviation (mm, dd, ...) to its canonical name so rank/unit lookups + # below see the real coarseness rather than defaulting to 99 / not-a-unit. + first_n = cls._DATE_PART_ABBR.get(first, first) + second_n = cls._DATE_PART_ABBR.get(second, second) if first_kw and not second_kw: part_index = 0 # Snowflake order elif second_kw and not first_kw: @@ -610,8 +627,8 @@ def _is_date_part_argument(cls, pre: str, suf: str, token: str) -> bool: # A model with e.g. BOTH `date` and `month` dimensions makes both arguments look # like parts. Truncation goes finer -> coarser, so the COARSER one is the part -- # month truncates date under either order. - first_rank = cls._DATE_PART_RANK.get(first, 99) - second_rank = cls._DATE_PART_RANK.get(second, 99) + first_rank = cls._DATE_PART_RANK.get(first_n, 99) + second_rank = cls._DATE_PART_RANK.get(second_n, 99) if first_rank != second_rank: part_index = 0 if first_rank < second_rank else 1 else: @@ -619,8 +636,8 @@ def _is_date_part_argument(cls, pre: str, suf: str, token: str) -> bool: # signal. A DATE_TRUNC part must be a truncation UNIT, so if exactly one # argument is a real unit (day) and the other an extraction-only keyword that is # also a column (date), the unit is the part and the other resolves as a column. - first_unit = first in cls._DATE_TRUNC_UNITS - second_unit = second in cls._DATE_TRUNC_UNITS + first_unit = first_n in cls._DATE_TRUNC_UNITS + second_unit = second_n in cls._DATE_TRUNC_UNITS if first_unit and not second_unit: part_index = 0 elif second_unit and not first_unit: diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 367f8628..7f581e65 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4882,6 +4882,40 @@ def test_lookml_export_folded_filter_protects_sql_server_datepart_first_function assert conds(["UPPER(day) = 'X'"], model) == "(UPPER((${TABLE}.order_day)) = 'X')" +def test_lookml_export_folded_filter_protects_sql_server_datepart_abbreviations(): + """SQL Server datepart ABBREVIATIONS (dd, mm, d, ...) in a part slot must be protected. + + DATEADD/DATEDIFF accept abbreviated dateparts, but they were absent from the keyword set, so on + a model with a `dd`/`mm` dimension a folded filter's DATEADD(dd, 1, created_at) rewrote `dd` to + (${TABLE}.order_dd), producing invalid SQL. They are protected only in the part slot, so a + same-named column used elsewhere still resolves. + """ + from sidemantic import Dimension, Model + + model = Model( + name="orders", + table="raw_orders", + primary_key="id", + dimensions=[ + Dimension(name="dd", type="time", granularity="day", sql="order_dd"), + Dimension(name="mm", type="time", granularity="month", sql="order_mm"), + Dimension(name="d", type="time", granularity="day", sql="order_d"), + Dimension(name="created_at", type="time", granularity="day", sql="created_at"), + ], + ) + conds = LookMLAdapter._fold_filter_conds + # Abbreviated part is protected; the trailing column arguments resolve. Covers two-letter and + # single-letter forms and the datepart-first DATENAME. + assert conds(["DATEADD(dd, 1, created_at) > 1"], model) == "(DATEADD(dd, 1, (${TABLE}.created_at)) > 1)" + assert conds(["DATEDIFF(mm, created_at, created_at) > 1"], model) == ( + "(DATEDIFF(mm, (${TABLE}.created_at), (${TABLE}.created_at)) > 1)" + ) + assert conds(["DATEADD(d, 1, created_at) > 1"], model) == "(DATEADD(d, 1, (${TABLE}.created_at)) > 1)" + assert conds(["DATENAME(dd, created_at) = 'x'"], model) == "(DATENAME(dd, (${TABLE}.created_at)) = 'x')" + # A same-named column used outside a date function is still rewritten. + assert conds(["mm = '2024-01'"], model) == "((${TABLE}.order_mm) = '2024-01')" + + def test_lookml_export_template_only_folded_filter_skipped(): """A folded filter that is ONLY a Liquid template leaves no real column -> skip the measure. From ced8db5fe5f3c54c124295474884685747cd6f73 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 12:07:27 -0700 Subject: [PATCH 23/44] Expand untemplated complete type:number bases in post-SQL measures A percent_of_total / percent_of_previous over an unfiltered complete type:number base (e.g. a re-imported STDDEV/VAR_SAMP) resolved the base to {model}., because it has no native aggregate template and is not filtered. Complete measures are projected via dedicated raw aliases, not as ., so the query referenced a missing column and failed. Use measure_full_sql_lookup for any base with no plain aggregate template, not just filtered ones. A native aggregate base still uses its template. --- sidemantic/adapters/lookml.py | 9 +++++- tests/adapters/lookml/test_edge_cases.py | 38 ++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 3d36dbc4..7cbcd25a 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -2904,7 +2904,14 @@ def _resolve(match: re.Match) -> str: # status='completed' THEN user_id END)). The `({model}.)` template # below carries no filter, so a percent_of_total over a filtered base would be # computed across every row instead of the filtered population. - if ref_name in filtered_base_measures and ref_name in measure_full_sql_lookup: + # Expand through the full SQL when the base has no plain ({model}.) + # form: a FILTERED base (carries a CASE filter), OR an untemplated COMPLETE + # type:number base (e.g. a re-imported STDDEV/VAR_SAMP). Both are projected via + # dedicated raw aliases, not as `.`, so `{model}.` would + # reference a missing column. + if ref_name in measure_full_sql_lookup and ( + ref_name in filtered_base_measures or ref_name not in measure_agg_lookup + ): return f"({measure_full_sql_lookup[ref_name]})" agg_template = measure_agg_lookup.get(ref_name) if agg_template: diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 7f581e65..df24b2ab 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4186,6 +4186,44 @@ def test_lookml_post_sql_measure_preserves_filtered_base_measure_filter(): assert "{model}.total" in model.get_metric("pct_unfiltered").sql +def test_lookml_post_sql_measure_expands_untemplated_complete_base(): + """A post-SQL measure over an UNTEMPLATED complete type:number base must expand its full SQL. + + An unfiltered complete base like STDDEV has no native ({model}.) template, so the + percent_of_total resolved it to `{model}.sd` -- but complete measures are projected via + dedicated raw aliases, so the query referenced a missing `orders_cte.sd` and failed. Expand + such a base through its full SQL; a native aggregate base keeps the template form. + """ + import duckdb + + from sidemantic import SemanticLayer + + graph = _parse_lkml( + """ +view: orders { + sql_table_name: orders ;; + dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } + dimension: amount { type: number sql: ${TABLE}.amount ;; } + measure: sd { type: number sql: STDDEV(${amount}) ;; } + measure: total { type: sum sql: ${amount} ;; } + measure: pct { type: percent_of_total sql: ${sd} ;; } + measure: pct_native { type: percent_of_total sql: ${total} ;; } +} +""" + ) + model = graph.get_model("orders") + # The complete base's full SQL is inlined; the native base keeps its measure-ref template. + assert "STDDEV" in model.get_metric("pct").sql and "{model}.sd" not in model.get_metric("pct").sql + assert "{model}.total" in model.get_metric("pct_native").sql + + layer = SemanticLayer(auto_register=False) + layer.add_model(model) + sql = layer.compile(metrics=["orders.pct"]) + con = duckdb.connect() + con.execute("create table orders as select 1 id, 10.0 amount union all select 2, 30 union all select 3, 60") + assert con.execute(sql).fetchall() == [(1.0,)] # single group -> its share is 1.0 + + def test_lookml_export_running_total_roundtrips(): """An imported running_total (cumulative + table_calculation meta) round-trips, not dropped.""" import tempfile From 47cb76648d29dfd02629bfe20f961285b5d08d2f Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 12:26:15 -0700 Subject: [PATCH 24/44] Neutralize {model} before the complete-SQL export agg check The agg-less complete-SQL export gate ran sql_has_aggregate on the raw {model} SQL, unlike its sibling call sites which replace {model} first. sqlglot happens to parse {model}.col as a struct today so VAR_SAMP/STDDEV_SAMP are still detected and exported, but that is incidental: if the parse ever failed the regex fallback omits var_samp/stddev_samp and would drop a valid sample- aggregate measure. Replace {model} with a plain identifier so detection stays on sqlglot's accurate path, consistent with the other call sites. --- sidemantic/adapters/lookml.py | 12 ++++++- tests/adapters/lookml/test_edge_cases.py | 43 ++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 7cbcd25a..4b54e9ef 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -3748,7 +3748,17 @@ def _export_view(self, model: Model, graph: SemanticGraph) -> dict: filters_folded = True else: measure_def["sql"] = agg_sql - elif metric.agg is None and col_sql and _sql_has_aggregate(metric.sql or ""): + elif ( + metric.agg is None + and col_sql + # Neutralize {model} before detection (as the other call sites do). sqlglot + # happens to parse {model}.col as a struct so VAR_SAMP({model}.amount) is + # detected today, but that is incidental -- if it ever failed to parse, the + # regex fallback (which omits var_samp/stddev_samp) would drop a valid + # sample-aggregate measure on export. A clean column keeps sqlglot on the + # accurate path. + and _sql_has_aggregate((metric.sql or "").replace("{model}", "x")) + ): # An agg-less measure whose SQL is itself an aggregate (a complete # SUM({model}.amount) imported from Cube, or an inline aggregate # expression). Faithfully maps to a LookML type: number with the diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index df24b2ab..f50b9188 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -5245,6 +5245,49 @@ def roundtrip(expr): assert block and "(ALL x)" in block and "SUM(ALL" not in block, block +def test_lookml_export_sample_aggregate_complete_measures_survive(): + """Agg-less complete measures using sample-aggregate aliases must export, not be dropped. + + The agg-detection at the export gate ran on the raw {model} SQL. sqlglot happens to parse + {model}.col as a struct so VAR_SAMP/STDDEV_SAMP are detected today, but if that ever failed the + regex fallback (which omits var_samp/stddev_samp) would skip the measure. Neutralizing {model} + first (as the sibling call sites do) keeps detection on sqlglot's accurate path. + """ + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + def exports(sql): + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="amount", type="numeric", sql="amount"), + ], + metrics=[Metric(name="m", agg=None, sql=sql, sql_is_complete=True)], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + reimported = "m" in {x.name for x in LookMLAdapter().parse(Path(out)).get_model("orders").metrics} + return "measure: m" in text and reimported + + for sql in ( + "VAR_SAMP({model}.amount)", + "STDDEV_SAMP({model}.amount)", + "VARIANCE({model}.amount)", + "STDDEV_POP({model}.amount)", + "VAR_POP({model}.amount)", + ): + assert exports(sql), sql + + def test_lookml_export_zero_column_stddev_skipped(): """A stddev/variance over a CONSTANT also emits type: number -> needs the zero-column guard. From eeabea0973958b39628df9168182ca64fa734159 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 12:58:39 -0700 Subject: [PATCH 25/44] Detect aggregates on the ALL-stripped SQL for complete-measure export The agg-less complete-measure export gate ran aggregate detection on the raw metric.sql, which still carries an explicit ALL modifier. sqlglot fails on VAR_SAMP(ALL x), and the regex fallback omits var_samp/stddev_samp, so such a measure was skipped even though col_sql above already normalized the ALL away. Check the normalized col_sql (ALL stripped, {model} -> ${TABLE}) with ${TABLE} neutralized, so sqlglot stays on its accurate parse path. --- sidemantic/adapters/lookml.py | 13 ++++++------- tests/adapters/lookml/test_edge_cases.py | 4 ++++ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 4b54e9ef..6d01a0b7 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -3751,13 +3751,12 @@ def _export_view(self, model: Model, graph: SemanticGraph) -> dict: elif ( metric.agg is None and col_sql - # Neutralize {model} before detection (as the other call sites do). sqlglot - # happens to parse {model}.col as a struct so VAR_SAMP({model}.amount) is - # detected today, but that is incidental -- if it ever failed to parse, the - # regex fallback (which omits var_samp/stddev_samp) would drop a valid - # sample-aggregate measure on export. A clean column keeps sqlglot on the - # accurate path. - and _sql_has_aggregate((metric.sql or "").replace("{model}", "x")) + # Detect on the NORMALIZED col_sql (ALL modifier already stripped, {model} + # -> ${TABLE}), not the raw metric.sql: an explicit ALL -- VAR_SAMP(ALL x) -- + # makes sqlglot fail, and the regex fallback omits var_samp/stddev_samp, so + # checking the raw form drops a valid sample-aggregate measure. Neutralize + # ${TABLE} to a clean column so sqlglot stays on the accurate parse path. + and _sql_has_aggregate(col_sql.replace("${TABLE}", "x")) ): # An agg-less measure whose SQL is itself an aggregate (a complete # SUM({model}.amount) imported from Cube, or an inline aggregate diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index f50b9188..998143f7 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -5284,6 +5284,10 @@ def exports(sql): "VARIANCE({model}.amount)", "STDDEV_POP({model}.amount)", "VAR_POP({model}.amount)", + # An explicit ALL modifier makes sqlglot fail; detection must run on the ALL-stripped + # col_sql, not the raw metric.sql, so these are not dropped. + "VAR_SAMP(ALL {model}.amount)", + "STDDEV_SAMP(ALL {model}.amount)", ): assert exports(sql), sql From a4ffbcef24c2d4423eea9ebd47d65f7ee21c39da Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 13:21:30 -0700 Subject: [PATCH 26/44] Fold filters into multi-aggregate complete measures on export A filtered complete measure with more than one aggregate -- SUM(a) / COUNT(*) -- or a scalar-wrapped aggregate -- ABS(SUM(x)) -- is not a single outer FUNC(arg), so _fold_filters_into_aggregate bailed and the measure was skipped on export. Fall back to the complete-SQL folder, which wraps every aggregate's argument in the filter, gated to single-argument aggregates: a multi-arg aggregate (WEIGHTED_AVG(a, b)) or multi-column COUNT(DISTINCT a, b) would fold to a malformed FUNC(CASE..., CASE...) the engine rejects, so those still skip. A DISTINCT over a single tuple stays foldable. --- sidemantic/adapters/lookml.py | 41 +++++++++++++++ tests/adapters/lookml/test_edge_cases.py | 67 ++++++++++++++++++++++-- 2 files changed, 104 insertions(+), 4 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 6d01a0b7..a238cea3 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -3374,6 +3374,30 @@ def _aggregate_references_column(sql: str) -> bool: return True return any(True for _ in tree.find_all(exp.Column)) + @staticmethod + def _complete_sql_fold_is_safe(sql: str) -> bool: + """True if folding a filter into every aggregate of ``sql`` yields VALID SQL. + + The complete-SQL folder wraps each aggregate argument in ``CASE WHEN THEN arg``. + That is safe for a single-argument aggregate (``SUM(x)``, ``COUNT(*)``, ``SUM(x)/COUNT(*)``, + ``ABS(SUM(x))``), but a MULTI-argument aggregate -- ``WEIGHTED_AVG(a, b)`` or a multi-column + ``COUNT(DISTINCT a, b)`` -- would fold to a malformed ``FUNC(CASE..., CASE...)`` the engine + rejects. A DISTINCT wrapping a single tuple ``(a, b)`` is one argument and stays safe. + """ + import sqlglot + from sqlglot import expressions as exp + + from sidemantic.sql.aggregation_detection import _ANONYMOUS_AGGREGATE_FUNCTIONS + + try: + tree = sqlglot.parse_one(sql.replace("{model}", "__m__")) + except Exception: + return False + for n in tree.find_all(exp.Anonymous): + if (n.name or "").lower() in _ANONYMOUS_AGGREGATE_FUNCTIONS and len(n.expressions) > 1: + return False + return all(len(d.expressions) <= 1 for d in tree.find_all(exp.Distinct)) + @classmethod def _fold_filters_into_aggregate(cls, agg_sql: str, filters: list[str], model: Model) -> str | None: """Fold ``filters`` into a single-outer-aggregate SQL expression. @@ -3802,6 +3826,23 @@ def _export_view(self, model: Model, graph: SemanticGraph) -> dict: measure_def["type"] = "number" if metric.filters: folded = self._fold_filters_into_aggregate(col_sql, metric.filters, model) + if folded is None and self._complete_sql_fold_is_safe( + col_sql.replace("${TABLE}", "{model}") + ): + # A MULTI-aggregate complete expr (SUM(a) / COUNT(*)) or a scalar- + # wrapped one (ABS(SUM(x))) is not a single outer FUNC(arg), so + # _fold_filters_into_aggregate bails. Fall back to the complete-SQL + # folder, which wraps EVERY aggregate's argument in the filter (as + # the import path does) -- gated above to single-argument aggregates + # so a multi-arg one is not folded into malformed SQL. It works in + # {model} form, so resolve the filters and convert around the call. + resolved_pred = self._fold_filter_conds(metric.filters, model).replace( + "${TABLE}", "{model}" + ) + folded_model = self._fold_complete_sql_filters( + col_sql.replace("${TABLE}", "{model}"), [resolved_pred], force=True + ) + folded = folded_model.replace("{model}", "${TABLE}") if folded_model else None if folded is None: logger.warning( "Metric %r has filters over a complex aggregate SQL expression that " diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 998143f7..0fc8fc14 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -5062,8 +5062,13 @@ def test_lookml_export_folded_filter_does_not_rewrite_date_part_keyword(): assert conds(["LOWER(day) = 'x'"], model) == "(LOWER((${TABLE}.order_day)) = 'x')" -def test_lookml_export_scalar_wrapped_aggregate_filter_skipped(): - """A scalar-wrapped aggregate with filters (ABS(SUM(x))) can't fold -> skipped, not mangled.""" +def test_lookml_export_scalar_wrapped_aggregate_filter_folds_into_inner(): + """A scalar-wrapped aggregate with filters (ABS(SUM(x))) folds into the INNER aggregate. + + The complete-SQL folder wraps the filter around SUM's argument -- ABS(SUM(CASE WHEN ... THEN + amount END)) -- which is faithful, so the measure exports. It must NEVER push the CASE around + the nested aggregate itself (ABS(CASE WHEN ... THEN SUM(amount) END)), which would be wrong. + """ import tempfile from sidemantic import Dimension, Metric, Model @@ -5093,8 +5098,9 @@ def test_lookml_export_scalar_wrapped_aggregate_filter_skipped(): out = tempfile.mktemp(suffix=".lkml") LookMLAdapter().export(graph, out) text = open(out).read() - assert "measure: aw" not in text # can't fold CASE around the inner aggregate -> skipped - assert "ABS(CASE WHEN" not in text # never push CASE around a nested aggregate + assert "measure: aw" in text # exports: the filter folds into the inner SUM + assert "ABS(CASE WHEN" not in text # never push CASE around the nested aggregate + assert "SUM(CASE WHEN" in text # the CASE wraps SUM's argument, correctly def test_lookml_export_multi_column_distinct_filter_skipped(): @@ -5523,6 +5529,59 @@ def test_lookml_export_delimited_distinct_filter_folds_not_skipped(): assert "DISTINCT CASE WHEN" in txt # filter folded inside the single-column DISTINCT +def test_lookml_export_filtered_multi_aggregate_measure_folds_and_roundtrips(): + """A filtered complete measure with MULTIPLE aggregates must export, folding each aggregate. + + SUM(a) / NULLIF(COUNT(*), 0) is not a single outer FUNC(arg), so _fold_filters_into_aggregate + bailed and the measure was skipped. The complete-SQL folder wraps EVERY aggregate's argument in + the filter, so it exports and round-trips to the correct filtered value; a renamed filter + dimension resolves to its column. + """ + import tempfile + + import duckdb + + from sidemantic import Dimension, Metric, Model, SemanticLayer + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="orders", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="amount", type="numeric", sql="amount"), + Dimension(name="status", type="categorical", sql="order_status"), # renamed column + ], + metrics=[ + Metric( + name="avg_completed", + agg=None, + sql="SUM({model}.amount) / NULLIF(COUNT(*), 0)", + sql_is_complete=True, + filters=["{model}.status = 'completed'"], + ) + ], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + reimported = LookMLAdapter().parse(Path(out)).get_model("orders") + assert reimported.get_metric("avg_completed") is not None # not skipped + + layer = SemanticLayer(auto_register=False) + layer.add_model(reimported) + con = duckdb.connect() + con.execute( + "create table orders as select 1 id, 10 amount, 'completed' order_status " + "union all select 2, 30, 'pending' union all select 3, 20, 'completed'" + ) + # completed rows: amount 10 + 20 = 30 over 2 rows -> 15.0. + assert con.execute(layer.compile(metrics=["orders.avg_completed"])).fetchall() == [(15.0,)] + + def test_lookml_export_multi_arg_aggregate_filter_skipped(): """A multi-argument aggregate WEIGHTED_AVG(price, qty) with filters skips, not malformed CASE.""" import tempfile From 4a604e2d208c7446723e93ac16873b89e23ae1bb Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 13:33:15 -0700 Subject: [PATCH 27/44] Skip subquery aggregates on LookML export instead of losing them An agg-less complete-SQL measure whose expression carries a scalar subquery -- SUM({model}.amount) / (SELECT COUNT(*) FROM other) -- was exported as a `type: number` measure, but the import side's _parse_measure rejects subquery SQL, so the measure silently vanished on re-import. The export/import round-trip therefore lost the metric. Guard the agg-less-complete export branch with _has_subquery and skip with a warning, matching the import-side subquery rejection so the two sides stay consistent. --- sidemantic/adapters/lookml.py | 14 +++++++++++ tests/adapters/lookml/test_edge_cases.py | 32 ++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index a238cea3..ae26c8db 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -3798,6 +3798,20 @@ def _export_view(self, model: Model, graph: SemanticGraph) -> dict: # so accept it too rather than dropping the metric at the zero-column check # below. The trailing \s+ keeps a column literally named `all` (COUNT(all)) # a plain argument. + if self._has_subquery(col_sql): + # A type: number carrying a scalar subquery (SUM({model}.amount) / + # (SELECT COUNT(*) FROM other)) exports fine, but the import side's + # _parse_measure rejects subquery SQL, so the measure silently vanishes + # on re-import. Skip it here so export/import round-trips consistently + # rather than emitting a measure the adapter cannot read back. + logger.warning( + "Metric %r has a scalar subquery in its aggregate SQL (%r); the LookML " + "adapter cannot re-import a type: number measure containing a subquery, " + "so skipping it on export to keep round-trips consistent.", + metric.name, + col_sql, + ) + continue _count_const = r"\*|[+-]?(?:\d+\.?\d*|\.\d+)|true|false|'(?:[^']|'')*'" if not metric.filters and re.fullmatch( rf"(?i)count\s*\(\s*(?:all\s+)?(?:{_count_const})\s*\)", col_sql.strip() diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 0fc8fc14..c4e279f4 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -5136,6 +5136,38 @@ def test_lookml_export_multi_column_distinct_filter_skipped(): assert "measure: du" not in open(out).read() # skipped, no malformed `THEN a, b END` +def test_lookml_export_aggregate_with_scalar_subquery_skipped(): + """An agg-less complete measure carrying a scalar subquery is skipped, not silently lost on re-import. + + SUM({model}.amount) / (SELECT COUNT(*) FROM other) would export as type: number, but the import + side's _parse_measure rejects subquery SQL, so the measure vanishes on re-import. Skip it on + export so the round-trip stays consistent rather than dropping a measure the adapter emitted. + """ + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="t", + primary_key="id", + dimensions=[Dimension(name="id", type="numeric", sql="id")], + metrics=[ + Metric(name="sq", agg=None, sql="SUM({model}.amount) / (SELECT COUNT(*) FROM other)"), + Metric(name="total", agg="sum", sql="{model}.amount"), + ], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "measure: sq" not in text # subquery measure skipped rather than emitted-then-dropped + assert "measure: total" in text # ordinary aggregates still export + + def test_lookml_export_folded_filter_leaves_backtick_identifier_untouched(): """A folded filter's backtick/bracket-quoted identifier must not be rewritten inside the quotes.""" import tempfile From f2c8602f6ea0523fa5970a4eda8cb2fd902e01e5 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 13:48:07 -0700 Subject: [PATCH 28/44] Reject ordered-set aggregates and keep literals in folded export filters Two fixes to the complete-SQL filter folding used on LookML export: - An ordered-set aggregate (SUM(x ORDER BY y)) was treated as foldable, so the folder wrapped only the argument -- SUM(CASE WHEN ... THEN x ORDER BY y END) -- burying the ORDER BY inside the CASE, which is malformed. _complete_sql_fold_is_safe now rejects any aggregate-local ORDER BY so the measure is skipped instead of exported broken. - A folded filter's bare true/false/null token was rewritten to a same-named dimension's SQL when such a dimension existed, silently changing the predicate (status = true -> status = (${TABLE}.is_active)). SQL requires a real column named `true` to be quoted, so a bare token is always the literal; leave it intact. --- sidemantic/adapters/lookml.py | 16 +++++ tests/adapters/lookml/test_edge_cases.py | 74 ++++++++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index ae26c8db..8b903950 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -3283,6 +3283,13 @@ def _one(m): # read group 2 defensively (m.group(2) would raise IndexError otherwise). bare = m.group(2) if m.re.groups >= 2 else None if bare is not None: + # A bare token equal to a SQL boolean/NULL literal (true/false/null) is a + # VALUE, not a column -- SQL requires an actual column named `true` to be + # quoted. When a dimension happens to share the name, rewriting the literal + # to that dimension's SQL silently changes the predicate (`status = true` + # -> `status = (${TABLE}.is_active)`), so leave the literal intact. + if bare.lower() in ("true", "false", "null"): + return m.group(0) # Bare dimension-name alternative: skip when it sits in a SQL TYPE context # (a cast target), not a column operand -- e.g. CAST(x AS date) or x::date # with a `date` dimension. Rewriting the type token to a column would emit @@ -3383,6 +3390,11 @@ def _complete_sql_fold_is_safe(sql: str) -> bool: ``ABS(SUM(x))``), but a MULTI-argument aggregate -- ``WEIGHTED_AVG(a, b)`` or a multi-column ``COUNT(DISTINCT a, b)`` -- would fold to a malformed ``FUNC(CASE..., CASE...)`` the engine rejects. A DISTINCT wrapping a single tuple ``(a, b)`` is one argument and stays safe. + + An ORDERED-set aggregate (``SUM(x ORDER BY y)``, ``ARRAY_AGG(x ORDER BY y)``) is also + unsafe: the folder wraps only the argument, so the ORDER BY lands INSIDE the CASE + (``SUM(CASE WHEN ... THEN x ORDER BY y END)``) rather than on the aggregate call. Reject + it so the measure is skipped instead of exporting malformed LookML SQL. """ import sqlglot from sqlglot import expressions as exp @@ -3393,6 +3405,10 @@ def _complete_sql_fold_is_safe(sql: str) -> bool: tree = sqlglot.parse_one(sql.replace("{model}", "__m__")) except Exception: return False + # An ORDER BY here can only be an aggregate-local ordered-set clause (the input is an + # aggregate expression, never a full SELECT); folding would bury it inside the CASE. + if any(True for _ in tree.find_all(exp.Order)): + return False for n in tree.find_all(exp.Anonymous): if (n.name or "").lower() in _ANONYMOUS_AGGREGATE_FUNCTIONS and len(n.expressions) > 1: return False diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index c4e279f4..f36d7a9a 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4954,6 +4954,80 @@ def test_lookml_export_folded_filter_protects_sql_server_datepart_abbreviations( assert conds(["mm = '2024-01'"], model) == "((${TABLE}.order_mm) = '2024-01')" +def test_lookml_export_folded_filter_keeps_boolean_null_literals(): + """A bare true/false/null in a folded filter stays a literal even when a dimension shares the name. + + SQL requires a real column named `true` to be quoted, so a bare token is always the literal. + Rewriting it to a same-named dimension's SQL silently changed the predicate + (`status = true` -> `status = (${TABLE}.is_active)`). The literal must survive; a genuine + same-named column used as an operand (LHS) still resolves. + """ + from sidemantic import Dimension, Model + + model = Model( + name="orders", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="status", type="categorical", sql="status_col"), + Dimension(name="true", type="boolean", sql="is_active"), + Dimension(name="false", type="boolean", sql="is_closed"), + Dimension(name="null", type="categorical", sql="missing_col"), + ], + ) + conds = LookMLAdapter._fold_filter_conds + # RHS literals are preserved, not rewritten to the same-named dimension SQL. + assert conds(["{model}.status = true"], model) == "((${TABLE}.status_col) = true)" + assert conds(["{model}.status = false"], model) == "((${TABLE}.status_col) = false)" + assert conds(["{model}.status IS NOT null"], model) == "((${TABLE}.status_col) IS NOT null)" + # Case-insensitive: TRUE / NULL keywords are literals too. + assert conds(["{model}.status = TRUE"], model) == "((${TABLE}.status_col) = TRUE)" + + +def test_lookml_export_ordered_set_aggregate_with_filter_skipped(): + """A filtered ordered-set aggregate can't fold (ORDER BY would land inside the CASE) -> skipped. + + Folding wraps only the aggregate ARGUMENT in CASE, so SUM(x ORDER BY y) would become + SUM(CASE WHEN ... THEN x ORDER BY y END) -- ORDER BY buried inside the CASE, malformed LookML + SQL. _complete_sql_fold_is_safe must reject it so the measure is skipped, not emitted broken. + """ + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + assert LookMLAdapter._complete_sql_fold_is_safe("SUM({model}.amount ORDER BY {model}.created_at)") is False + # A plain aggregate (no ORDER BY) is still foldable. + assert LookMLAdapter._complete_sql_fold_is_safe("SUM({model}.amount) / COUNT({model}.id)") is True + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="status"), + ], + metrics=[ + Metric( + name="oa", + agg=None, + sql="SUM({model}.amount ORDER BY {model}.created_at)", + sql_is_complete=True, + filters=["{model}.status = 'x'"], + ) + ], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "measure: oa" not in text # skipped, not emitted with ORDER BY inside the CASE + assert "ORDER BY ${TABLE}.created_at END" not in text # never the malformed inside-CASE form + + def test_lookml_export_template_only_folded_filter_skipped(): """A folded filter that is ONLY a Liquid template leaves no real column -> skip the measure. From 483dded7069d6386f221f0ba48642295197312bc Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 14:26:54 -0700 Subject: [PATCH 29/44] Harden folded-filter export: quote-aware scan, multi-word casts, renamed aggregates Three export-fold correctness fixes: - The backward scan that finds a folded filter's enclosing date/time call counted a `)` inside a string literal (label = ')') as syntax and lost the call, so a DATE_TRUNC part token was rewritten to a same-named dimension. The scan now blanks string-literal contents first. - The cast-type guard only protected the token immediately after AS/::, so the second word of a multi-word type (double precision) was rewritten to a same-named `precision` dimension. Protect the whole word run. - Folding re-serializes through sqlglot, which canonicalizes dialect aggregate spellings (APPROX_COUNT_DISTINCT -> APPROX_DISTINCT, which DuckDB rejects). Bail the fold when an aggregate would render under a name absent from the source, so the caller skips the measure rather than emit an invalid function. Safe structural rewrites (IF -> CASE, ::t -> CAST) are unaffected. Also rebuild the cross-view running_total export test via the Python API, since such a measure is now dropped at import by the reference resolver. --- sidemantic/adapters/lookml.py | 47 +++++++- tests/adapters/lookml/test_edge_cases.py | 130 +++++++++++++++++++++-- 2 files changed, 166 insertions(+), 11 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 8b903950..9549bc70 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -292,6 +292,22 @@ def _case(cond, then): if not aggs or not parsed_conds: return None + # sqlglot canonicalizes some dialect-specific AGGREGATE spellings when it re-serializes the + # tree below (APPROX_COUNT_DISTINCT -> APPROX_DISTINCT, VAR_POP -> VARIANCE_POP, ...). The + # rewritten spelling can be INVALID on the warehouse the original targeted (DuckDB rejects + # APPROX_DISTINCT), and this fold is the only place the SQL is round-tripped through the + # serializer. If any aggregate would render under a name that does NOT appear as a call in + # the source, we cannot reproduce it faithfully -> bail so the caller skips the measure + # rather than emit a renamed aggregate. Only aggregate function names are checked, so a + # SAFE structural rewrite the target still accepts (IF -> CASE, `::t` -> CAST) is allowed. + for a in aggs: + try: + rendered_name = re.match(r"\s*(\w+)\s*\(", a.sql()) + except Exception: + return None + if rendered_name and not re.search(rf"\b{re.escape(rendered_name.group(1))}\s*\(", sql, re.I): + return None + # An ordered-set aggregate (PERCENTILE_CONT(0.5) WITHIN GROUP (ORDER BY x)) keeps its # value column in the enclosing WithinGroup's ORDER BY, NOT inside the AggFunc (whose # only arg is the percentile constant). Its "aggregate scope" for the column check and @@ -542,14 +558,37 @@ def _sql_has_list_aggregate(sql: str) -> bool: "hour", "minute", "second", "millisecond", "microsecond", "nanosecond"} ) # fmt: skip + @staticmethod + def _blank_string_literals(s: str) -> str: + """Replace the CONTENTS of single/double-quoted spans with spaces, preserving length. + + A paren-depth scan can then treat a ``)`` inside a string literal (``label = ')'``) as + non-syntax without shifting any character position the caller relies on. + """ + out = list(s) + quote = None + for i, ch in enumerate(s): + if quote is not None: + if ch == quote: + quote = None + else: + out[i] = " " + elif ch in "'\"": + quote = ch + return "".join(out) + @staticmethod def _enclosing_call(pre: str) -> tuple[str | None, int, int]: """Describe the call a token sits in, given the text ``pre`` before it. Returns ``(function_name, arg_index, open_paren_pos)`` by scanning backwards for the first unclosed ``(`` and counting the top-level commas after it. ``(None, 0, -1)`` when the token - is not inside a call. + is not inside a call. The scan is quote-aware: a paren or comma inside a string literal + (``DATE_TRUNC(CASE WHEN x = ')' THEN a END, month)``) is not counted as syntax. """ + # Blank string-literal contents first (length-preserving) so a `)`/`,` inside a quoted + # value does not skew the paren depth or comma count; positions stay valid for the caller. + pre = LookMLAdapter._blank_string_literals(pre) depth, commas, i = 0, 0, len(pre) - 1 while i >= 0: ch = pre[i] @@ -3295,8 +3334,12 @@ def _one(m): # with a `date` dimension. Rewriting the type token to a column would emit # invalid SQL like CAST(x AS (${TABLE}.order_date)). (Typed literals like # `date '2024-01-01'` are protected earlier, in the split below.) + # The `(?:\w+\s+)*` tail also covers a MULTI-WORD type: in + # `cast(x AS double precision)` the second type word `precision` is still in + # the type slot (only words + spaces separate it from AS/::), so protect it + # too rather than rewrite it to a same-named `precision` dimension. pre = fstr[: base + m.start()] - if re.search(r"(?is)\bAS\s+$", pre) or pre.rstrip().endswith("::"): + if re.search(r"(?is)\bAS\s+(?:\w+\s+)*$", pre) or re.search(r"::\s*(?:\w+\s+)*$", pre): return m.group(0) # Skip a bare token in a DATE-PART / INTERVAL-UNIT keyword position, not a # column operand -- e.g. EXTRACT(day FROM ...) or `INTERVAL 7 day` on a diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index f36d7a9a..d9ceadba 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4954,6 +4954,102 @@ def test_lookml_export_folded_filter_protects_sql_server_datepart_abbreviations( assert conds(["mm = '2024-01'"], model) == "((${TABLE}.order_mm) = '2024-01')" +def test_lookml_export_folded_filter_date_part_scan_is_quote_aware(): + """A `)` inside a string literal must not break the date-part call scan. + + The backward scan that finds the enclosing DATE_TRUNC counted the `)` inside `= ')'` as syntax + and lost the call, so the `month` date-part token was rewritten to the `month` dimension SQL. + The scan is now quote-aware, so the part token stays intact and the real column still resolves. + """ + from sidemantic import Dimension, Model + + model = Model( + name="orders", + table="raw_orders", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="month", type="time", granularity="month", sql="order_month"), + Dimension(name="created_at", type="time", granularity="day", sql="created_at"), + ], + ) + conds = LookMLAdapter._fold_filter_conds + out = conds(["DATE_TRUNC(CASE WHEN label = ')' THEN created_at END, month) = DATE '2024-01-01'"], model) + assert ", month)" in out # date-part token untouched despite the ')' inside the string + assert "order_month" not in out # not rewritten to the same-named dimension + assert "(${TABLE}.created_at)" in out # the real column argument still resolves + # A same-named `month` column used as a real operand still resolves. + assert conds(["month = '2024-01'"], model) == "((${TABLE}.order_month) = '2024-01')" + + +def test_lookml_export_folded_filter_protects_multi_word_cast_type(): + """Every token of a multi-word cast type (double precision) is protected, not just the first.""" + from sidemantic import Dimension, Model + + model = Model( + name="orders", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="precision", type="numeric", sql="precision_col"), + Dimension(name="amount", type="numeric", sql="amt"), + ], + ) + conds = LookMLAdapter._fold_filter_conds + assert conds(["cast(amount as double precision) > 0"], model) == "(cast((${TABLE}.amt) as double precision) > 0)" + # The `::` form is protected across the whole multi-word type too. + assert conds(["amount::double precision > 0"], model) == "((${TABLE}.amt)::double precision > 0)" + # A same-named `precision` column used as a real operand still resolves. + assert conds(["precision > 5"], model) == "((${TABLE}.precision_col) > 5)" + + +def test_lookml_export_dialect_renamed_aggregate_skipped(): + """A filtered complete metric whose aggregate sqlglot would RENAME on export is skipped, not mangled. + + Serializing the fold rewrites APPROX_COUNT_DISTINCT to APPROX_DISTINCT, which DuckDB rejects, so + the fold bails and the measure is skipped rather than exported with an invalid function name. A + plain SUM/COUNT (no renamed aggregate) still folds and exports. + """ + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + # The fold itself bails (returns None) on a renamed aggregate, but folds a stable one. + assert ( + LookMLAdapter._fold_complete_sql_filters( + "APPROX_COUNT_DISTINCT({model}.user_id) / COUNT(*)", ["{model}.status = 'x'"], force=True + ) + is None + ) + assert ( + LookMLAdapter._fold_complete_sql_filters("SUM({model}.amount) / COUNT(*)", ["{model}.status = 'x'"], force=True) + is not None + ) + + def _export(sql, name): + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="status"), + ], + metrics=[Metric(name=name, agg=None, sql=sql, sql_is_complete=True, filters=["{model}.status = 'x'"])], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + return open(out).read() + + assert "measure: au" not in _export("APPROX_COUNT_DISTINCT({model}.user_id) / COUNT(*)", "au") # skipped + assert "measure: ratio" in _export("SUM({model}.amount) / COUNT(*)", "ratio") # ordinary fold exports + + def test_lookml_export_folded_filter_keeps_boolean_null_literals(): """A bare true/false/null in a folded filter stays a literal even when a dimension shares the name. @@ -5879,17 +5975,33 @@ def test_lookml_export_bare_count_star_uses_native_count_type(): def test_lookml_export_running_total_cross_view_ref_not_double_wrapped(): - """A running_total over an unsupported (already-braced) cross-view ref must not emit ${${...}}.""" + """A cumulative metric whose sql is a single already-braced cross-view ref exports as-is (no ${${}}). + + Such a metric is no longer importable from LookML (a cross-view ref is dropped at parse time, + see test_lookml_cross_view_reference), but it can still arrive via the Python API, so the export + path must pass the single ${...} ref straight through rather than re-wrap it into ${${...}}. + """ import tempfile - graph = _parse_lkml( - """ -view: orders { - sql_table_name: t ;; - dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } - measure: rt { type: running_total sql: ${other_view.total} ;; } -} -""" + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="t", + primary_key="id", + dimensions=[Dimension(name="id", type="numeric", sql="id")], + metrics=[ + Metric( + name="rt", + type="cumulative", + sql="${other_view.total}", + meta={"table_calculation": "running_total"}, + ) + ], + ) ) out = tempfile.mktemp(suffix=".lkml") LookMLAdapter().export(graph, out) From aa6f73b423bbc40b7905bd46fa98eaaa47f1a177 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 15:01:40 -0700 Subject: [PATCH 30/44] Fold WITHIN GROUP aggregates and protect SQL operators in folded filters Two follow-ups to the folded-filter export hardening: - The ORDER BY safety check rejected ALL ordered-set aggregates, but a WITHIN GROUP (ORDER BY x) form is folded correctly by _fold_complete_sql_filters (the predicate goes into the ORDER BY value), so filtered percentile/median complete metrics were dropped from export for no reason. Reject only an ORDER BY that is NOT inside a WITHIN GROUP. - The bare-dimension rewrite treated a dimension named after a SQL operator/keyword (or/and/in/between/case ...) as a column, so a folded filter `status = 'done' or status = 'paid'` became invalid SQL. Extend the literal guard to the SQL keyword/operator set alongside true/false/null. --- sidemantic/adapters/lookml.py | 42 ++++++++++++++++-------- tests/adapters/lookml/test_edge_cases.py | 33 +++++++++++++++++++ 2 files changed, 62 insertions(+), 13 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 9549bc70..2d97a2d3 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -498,6 +498,19 @@ def _sql_has_list_aggregate(sql: str) -> bool: "dw": "weekday", "w": "weekday", "hh": "hour", "mi": "minute", "n": "minute", "ss": "second", "s": "second", "ms": "millisecond", "mcs": "microsecond", "ns": "nanosecond", } # fmt: skip + # Bare tokens that are SQL SYNTAX (operators, logical/comparison keywords, literals), never a + # column reference -- a real column with such a name must be quoted. A folded filter must leave + # these intact rather than rewrite one that happens to share a dimension name (e.g. a dimension + # named `or` in `status = 'done' or status = 'paid'`) into that dimension's SQL. + _SQL_KEYWORD_TOKENS = frozenset( + { + "true", "false", "null", "unknown", + "and", "or", "not", "is", "in", "like", "ilike", "rlike", "similar", + "between", "exists", "escape", "all", "any", "some", + "case", "when", "then", "else", "end", + } + ) # fmt: skip + _DATE_PART_KEYWORDS = frozenset( { "year", "quarter", "month", "week", "day", "hour", "minute", "second", @@ -3322,12 +3335,13 @@ def _one(m): # read group 2 defensively (m.group(2) would raise IndexError otherwise). bare = m.group(2) if m.re.groups >= 2 else None if bare is not None: - # A bare token equal to a SQL boolean/NULL literal (true/false/null) is a - # VALUE, not a column -- SQL requires an actual column named `true` to be - # quoted. When a dimension happens to share the name, rewriting the literal - # to that dimension's SQL silently changes the predicate (`status = true` - # -> `status = (${TABLE}.is_active)`), so leave the literal intact. - if bare.lower() in ("true", "false", "null"): + # A bare token that is SQL SYNTAX -- a boolean/NULL literal (true/false/null) + # or a logical/comparison operator/keyword (and/or/not/is/in/like/between/ + # case...) -- is never a column reference; a real column with such a name + # must be quoted. When a dimension happens to share the name, rewriting it to + # that dimension's SQL corrupts the predicate (`status = true` -> `status = + # (${TABLE}.is_active)`; `a or b` -> `a (${TABLE}.or_col) b`), so leave it. + if bare.lower() in cls._SQL_KEYWORD_TOKENS: return m.group(0) # Bare dimension-name alternative: skip when it sits in a SQL TYPE context # (a cast target), not a column operand -- e.g. CAST(x AS date) or x::date @@ -3434,10 +3448,12 @@ def _complete_sql_fold_is_safe(sql: str) -> bool: ``COUNT(DISTINCT a, b)`` -- would fold to a malformed ``FUNC(CASE..., CASE...)`` the engine rejects. A DISTINCT wrapping a single tuple ``(a, b)`` is one argument and stays safe. - An ORDERED-set aggregate (``SUM(x ORDER BY y)``, ``ARRAY_AGG(x ORDER BY y)``) is also - unsafe: the folder wraps only the argument, so the ORDER BY lands INSIDE the CASE - (``SUM(CASE WHEN ... THEN x ORDER BY y END)``) rather than on the aggregate call. Reject - it so the measure is skipped instead of exporting malformed LookML SQL. + An ORDER BY inside an aggregate's ARGUMENT list (``SUM(x ORDER BY y)``, + ``ARRAY_AGG(x ORDER BY y)``) is unsafe: the folder wraps only the argument, so the ORDER BY + lands INSIDE the CASE (``SUM(CASE WHEN ... THEN x ORDER BY y END)``). But a ``WITHIN GROUP + (ORDER BY x)`` ordered-set aggregate (``PERCENTILE_CONT(0.5) WITHIN GROUP (ORDER BY x)``) IS + safe -- ``_fold_complete_sql_filters`` folds the predicate into the ORDER BY value there -- + so reject only an ORDER BY that is NOT inside a WITHIN GROUP. """ import sqlglot from sqlglot import expressions as exp @@ -3448,9 +3464,9 @@ def _complete_sql_fold_is_safe(sql: str) -> bool: tree = sqlglot.parse_one(sql.replace("{model}", "__m__")) except Exception: return False - # An ORDER BY here can only be an aggregate-local ordered-set clause (the input is an - # aggregate expression, never a full SELECT); folding would bury it inside the CASE. - if any(True for _ in tree.find_all(exp.Order)): + # An ORDER BY in an aggregate's argument list would be buried in the CASE by folding; a + # WITHIN GROUP ORDER BY is folded correctly, so reject only the former. + if any(o.find_ancestor(exp.WithinGroup) is None for o in tree.find_all(exp.Order)): return False for n in tree.find_all(exp.Anonymous): if (n.name or "").lower() in _ANONYMOUS_AGGREGATE_FUNCTIONS and len(n.expressions) > 1: diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index d9ceadba..52614f42 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -5080,6 +5080,34 @@ def test_lookml_export_folded_filter_keeps_boolean_null_literals(): assert conds(["{model}.status = TRUE"], model) == "((${TABLE}.status_col) = TRUE)" +def test_lookml_export_folded_filter_keeps_sql_operator_keywords(): + """Bare SQL operators/keywords (or/and/in/between) stay operators even when a dimension shares the name. + + An unquoted `or` is always the operator, never a column, so a folded filter must not rewrite it + to a same-named `or` dimension's SQL (which produced invalid `... 'done' (${TABLE}.or_col) ...`). + """ + from sidemantic import Dimension, Model + + model = Model( + name="orders", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="status", type="categorical", sql="status_col"), + Dimension(name="or", type="categorical", sql="or_col"), + Dimension(name="and", type="categorical", sql="and_col"), + Dimension(name="in", type="categorical", sql="in_col"), + ], + ) + conds = LookMLAdapter._fold_filter_conds + assert conds(["status = 'done' or status = 'paid'"], model) == ( + "((${TABLE}.status_col) = 'done' or (${TABLE}.status_col) = 'paid')" + ) + assert conds(["status in ('a', 'b') and status = 'c'"], model) == ( + "((${TABLE}.status_col) in ('a', 'b') and (${TABLE}.status_col) = 'c')" + ) + + def test_lookml_export_ordered_set_aggregate_with_filter_skipped(): """A filtered ordered-set aggregate can't fold (ORDER BY would land inside the CASE) -> skipped. @@ -5122,6 +5150,11 @@ def test_lookml_export_ordered_set_aggregate_with_filter_skipped(): text = open(out).read() assert "measure: oa" not in text # skipped, not emitted with ORDER BY inside the CASE assert "ORDER BY ${TABLE}.created_at END" not in text # never the malformed inside-CASE form + # A WITHIN GROUP ordered-set aggregate IS foldable (the folder targets the ORDER BY value), so + # it must NOT be rejected -- filtered percentile/median metrics still export. + assert ( + LookMLAdapter._complete_sql_fold_is_safe("PERCENTILE_CONT(0.5) WITHIN GROUP (ORDER BY {model}.amount)") is True + ) def test_lookml_export_template_only_folded_filter_skipped(): From 75c7c72a52da81fbb8dda8ca2668936ed54f5d37 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 15:17:36 -0700 Subject: [PATCH 31/44] Guard constant SQL and raw-column aggregates in complete-SQL export Two more folded-filter/complete-SQL export fixes: - A folded filter over a dimension whose resolved SQL is an identifier-shaped literal (sql="1", sql="TRUE") was table-qualified to ${TABLE}.1, invalid SQL. Parenthesize a numeric/boolean/null literal without a table qualifier. - The agg-less complete export branch accepted any expression containing an aggregate, including one with a raw column OUTSIDE the aggregate (SUM({model}.amount) + {model}.tax_rate). The import side rejects it via _mixed_is_aggregate_safe, so the round-trip lost the metric. Apply the same aggregate-safety check before emitting type: number. --- sidemantic/adapters/lookml.py | 23 ++++++++-- tests/adapters/lookml/test_edge_cases.py | 54 ++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 3 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 2d97a2d3..e8a39eda 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -3299,9 +3299,12 @@ def _fold_filter_conds(cls, filters: list[str], model: Model) -> str: dim_names = {d.name for d in model.dimensions} def _qualify(val: str) -> str: - # Bare column -> qualify with {model}. so it stays unambiguous in joins; - # any resolved expression is parenthesized to preserve precedence. - if re.fullmatch(r"\w+", val): + # Bare column -> qualify with {model}. so it stays unambiguous in joins; any resolved + # expression is parenthesized to preserve precedence. But an identifier-shaped SQL + # LITERAL -- a numeric constant (`1`) or true/false/null -- is a VALUE, not a column, so + # parenthesize it WITHOUT a table qualifier (${TABLE}.1 is invalid SQL). A column name + # cannot start with a digit unquoted, so a leading digit marks a numeric literal. + if re.fullmatch(r"\w+", val) and not val[0].isdigit() and val.lower() not in ("true", "false", "null"): return f"({{model}}.{val})" return f"({val})" @@ -3887,6 +3890,20 @@ def _export_view(self, model: Model, graph: SemanticGraph) -> dict: col_sql, ) continue + # An aggregate expression that ALSO carries a raw column OUTSIDE any + # aggregate (SUM({model}.amount) + {model}.tax_rate) is not a valid grouped + # measure: the import side's _mixed_is_aggregate_safe rejects it, so a + # type: number here would be dropped on re-import (and Looker would GROUP BY + # error on the ungrouped column). Skip to keep the round-trip consistent. + if not self._mixed_is_aggregate_safe(col_sql.replace("${TABLE}", "{model}"), lambda rn: False): + logger.warning( + "Metric %r has a raw column outside an aggregate in its complete SQL " + "(%r); a type: number measure would be dropped on re-import, so " + "skipping on export.", + metric.name, + col_sql, + ) + continue _count_const = r"\*|[+-]?(?:\d+\.?\d*|\.\d+)|true|false|'(?:[^']|'')*'" if not metric.filters and re.fullmatch( rf"(?i)count\s*\(\s*(?:all\s+)?(?:{_count_const})\s*\)", col_sql.strip() diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 52614f42..b28b4b26 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -5371,6 +5371,60 @@ def test_lookml_export_aggregate_with_scalar_subquery_skipped(): assert "measure: total" in text # ordinary aggregates still export +def test_lookml_export_complete_sql_with_raw_column_outside_aggregate_skipped(): + """An aggregate expression with a raw column OUTSIDE any aggregate is skipped, not lost on re-import. + + SUM({model}.amount) + {model}.tax_rate is not a valid grouped measure (tax_rate is ungrouped); + the import side's _mixed_is_aggregate_safe rejects it, so a type: number export would be dropped + on re-import. Skip on export to keep the round-trip consistent; a pure aggregate still exports. + """ + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="t", + primary_key="id", + dimensions=[Dimension(name="id", type="numeric", sql="id")], + metrics=[ + Metric(name="mixed", agg=None, sql="SUM({model}.amount) + {model}.tax_rate", sql_is_complete=True), + Metric(name="ratio", agg=None, sql="SUM({model}.amount) / COUNT(*)", sql_is_complete=True), + ], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + assert "measure: mixed" not in text # raw column outside aggregate -> skipped + assert "measure: ratio" in text # fully-aggregated expression still exports + + +def test_lookml_export_folded_filter_keeps_constant_dimension_sql_unqualified(): + """A folded filter over a constant-SQL dimension keeps the literal unqualified, not ${TABLE}.1.""" + from sidemantic import Dimension, Model + + model = Model( + name="orders", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="status", type="categorical", sql="status_col"), + Dimension(name="flag", type="numeric", sql="1"), # constant-valued dimension + Dimension(name="active", type="boolean", sql="TRUE"), + ], + ) + conds = LookMLAdapter._fold_filter_conds + # A constant/literal resolved SQL is parenthesized WITHOUT a table qualifier. + assert conds(["{model}.flag = 1"], model) == "((1) = 1)" + assert conds(["{model}.active = true"], model) == "((TRUE) = true)" + # A genuine column still gets qualified. + assert conds(["{model}.status = 'x'"], model) == "((${TABLE}.status_col) = 'x')" + + def test_lookml_export_folded_filter_leaves_backtick_identifier_untouched(): """A folded filter's backtick/bracket-quoted identifier must not be rewritten inside the quotes.""" import tempfile From f6d7fedd89d32c81d166583dc00c716ac20b9815 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 15:28:08 -0700 Subject: [PATCH 32/44] Protect reserved SQL grammar keywords in folded filters The folded-filter keyword guard covered operators and logical keywords but omitted reserved grammar keywords like FROM and AS. On a model with a dimension named `from`, a folded filter's EXTRACT(day FROM created_at) rewrote the FROM keyword to that dimension's SQL, corrupting the SQL (and the filters block is suppressed after folding). Add the reserved grammar/clause keywords (from/as/select/where/group/order/join/...); a real column with such a name must be quoted, so a bare token is always the keyword. --- sidemantic/adapters/lookml.py | 8 ++++++++ tests/adapters/lookml/test_edge_cases.py | 9 +++++++++ 2 files changed, 17 insertions(+) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index e8a39eda..83dd0507 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -508,6 +508,14 @@ def _sql_has_list_aggregate(sql: str) -> bool: "and", "or", "not", "is", "in", "like", "ilike", "rlike", "similar", "between", "exists", "escape", "all", "any", "some", "case", "when", "then", "else", "end", + # Reserved grammar keywords that appear INSIDE an expression/subquery of a predicate -- + # EXTRACT(day FROM x), CAST(x AS t), a scalar subquery's SELECT/FROM/WHERE/GROUP/ORDER/ + # JOIN clauses, DISTINCT, set operators. An unquoted column with any of these names is + # invalid SQL (they must be quoted), so a bare token is always the keyword. + "as", "from", "where", "select", "distinct", "group", "order", "by", "having", + "join", "inner", "outer", "left", "right", "full", "cross", "natural", "on", "using", + "union", "intersect", "except", "with", "asc", "desc", "nulls", "over", "partition", + "within", } ) # fmt: skip diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index b28b4b26..a759437a 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -5094,9 +5094,12 @@ def test_lookml_export_folded_filter_keeps_sql_operator_keywords(): primary_key="id", dimensions=[ Dimension(name="status", type="categorical", sql="status_col"), + Dimension(name="created_at", type="time", sql="created_col"), Dimension(name="or", type="categorical", sql="or_col"), Dimension(name="and", type="categorical", sql="and_col"), Dimension(name="in", type="categorical", sql="in_col"), + Dimension(name="from", type="categorical", sql="from_col"), + Dimension(name="as", type="categorical", sql="as_col"), ], ) conds = LookMLAdapter._fold_filter_conds @@ -5106,6 +5109,12 @@ def test_lookml_export_folded_filter_keeps_sql_operator_keywords(): assert conds(["status in ('a', 'b') and status = 'c'"], model) == ( "((${TABLE}.status_col) in ('a', 'b') and (${TABLE}.status_col) = 'c')" ) + # Grammar keywords inside an expression (EXTRACT ... FROM, CAST ... AS) stay keywords; the real + # column argument still resolves. + assert conds(["extract(day from created_at) = 5"], model) == ("(extract(day from (${TABLE}.created_col)) = 5)") + assert conds(["cast(created_at as date) = '2024-01-01'"], model) == ( + "(cast((${TABLE}.created_col) as date) = '2024-01-01')" + ) def test_lookml_export_ordered_set_aggregate_with_filter_skipped(): From 8e1b0efc510d15a5f04726ee93b1b274563774e3 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 15:44:26 -0700 Subject: [PATCH 33/44] Only bail complete-SQL folding on renamed aggregates for force callers The dialect-rename bail returned None for every caller, but None means different things: a FORCE caller (export/inline) skips the measure, while the IMPORT caller falls back to generator column-nulling, which silently drops the filter on a zero-column term. So for a filtered mix like APPROX_COUNT_DISTINCT(x) / NULLIF(COUNT(*), 0), the import path stopped folding the COUNT(*) denominator and computed it over all rows (completed 1/2 became 1/3). Gate the rename bail on `force` so import still folds (accepting the rename) while export still skips. --- sidemantic/adapters/lookml.py | 26 +++++++++++++----------- tests/adapters/lookml/test_edge_cases.py | 11 +++++++++- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 83dd0507..65afe7df 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -295,18 +295,20 @@ def _case(cond, then): # sqlglot canonicalizes some dialect-specific AGGREGATE spellings when it re-serializes the # tree below (APPROX_COUNT_DISTINCT -> APPROX_DISTINCT, VAR_POP -> VARIANCE_POP, ...). The # rewritten spelling can be INVALID on the warehouse the original targeted (DuckDB rejects - # APPROX_DISTINCT), and this fold is the only place the SQL is round-tripped through the - # serializer. If any aggregate would render under a name that does NOT appear as a call in - # the source, we cannot reproduce it faithfully -> bail so the caller skips the measure - # rather than emit a renamed aggregate. Only aggregate function names are checked, so a - # SAFE structural rewrite the target still accepts (IF -> CASE, `::t` -> CAST) is allowed. - for a in aggs: - try: - rendered_name = re.match(r"\s*(\w+)\s*\(", a.sql()) - except Exception: - return None - if rendered_name and not re.search(rf"\b{re.escape(rendered_name.group(1))}\s*\(", sql, re.I): - return None + # APPROX_DISTINCT). Bail (None) so a FORCE caller -- the export/inline path, which SKIPS the + # measure on None -- does not emit a renamed aggregate. The import caller (force=False) + # instead falls back to generator column-nulling on None, which silently drops the filter on + # a zero-column term (a COUNT(*) denominator), so it must NOT bail here: fold and accept the + # rename. Only aggregate names are checked, so a SAFE structural rewrite the target still + # accepts (IF -> CASE, `::t` -> CAST) is allowed. + if force: + for a in aggs: + try: + rendered_name = re.match(r"\s*(\w+)\s*\(", a.sql()) + except Exception: + return None + if rendered_name and not re.search(rf"\b{re.escape(rendered_name.group(1))}\s*\(", sql, re.I): + return None # An ordered-set aggregate (PERCENTILE_CONT(0.5) WITHIN GROUP (ORDER BY x)) keeps its # value column in the enclosing WithinGroup's ORDER BY, NOT inside the AggFunc (whose diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index a759437a..9506be19 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -5016,7 +5016,8 @@ def test_lookml_export_dialect_renamed_aggregate_skipped(): from sidemantic import Dimension, Metric, Model from sidemantic.core.semantic_graph import SemanticGraph - # The fold itself bails (returns None) on a renamed aggregate, but folds a stable one. + # The fold bails (returns None) on a renamed aggregate ONLY for a FORCE caller (export/inline, + # which skips the measure on None), but folds a stable one. assert ( LookMLAdapter._fold_complete_sql_filters( "APPROX_COUNT_DISTINCT({model}.user_id) / COUNT(*)", ["{model}.status = 'x'"], force=True @@ -5027,6 +5028,14 @@ def test_lookml_export_dialect_renamed_aggregate_skipped(): LookMLAdapter._fold_complete_sql_filters("SUM({model}.amount) / COUNT(*)", ["{model}.status = 'x'"], force=True) is not None ) + # The IMPORT caller (force=False) falls back to column-nulling on None, which would drop the + # filter on a zero-column COUNT(*) denominator, so it must NOT bail on a rename -- it folds, + # filtering BOTH the numerator and the denominator. + _imported = LookMLAdapter._fold_complete_sql_filters( + "APPROX_COUNT_DISTINCT({model}.user_id) / NULLIF(COUNT(*), 0)", ["{model}.status = 'completed'"] + ) + assert _imported is not None + assert "COUNT(CASE" in _imported # denominator folded, not left over all rows def _export(sql, name): graph = SemanticGraph() From 670ab4428cf1988bec908e7abd577fe3cf51c4b2 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 15:58:30 -0700 Subject: [PATCH 34/44] Reject multi-input aggregates and resolve quoted DATE_TRUNC value columns Two folded-filter export fixes: - The fold-safety check only rejected multi-arg anonymous aggregates and multi-column DISTINCT, so a standard two-input aggregate (CORR(x, y), COVAR_POP, REGR_*) passed. The folder wraps only the first argument, so CORR(CASE WHEN ... THEN x END, y) filtered x but left y over all rows -- a wrong statistic. Reject any AggFunc with a column in an argument other than `this` (a WITHIN GROUP aggregate keeps its column in the enclosing clause, so it stays safe). - The DATE_TRUNC part/value disambiguation stripped quotes before matching, so DATE_TRUNC('week', week) saw two identical unit tokens and the tie-break picked the value column as the part, leaving it unresolved. A quoted argument is a string literal and can only be the part, so let quoting decide before the by-keyword tie-break. --- sidemantic/adapters/lookml.py | 24 +++++++- tests/adapters/lookml/test_edge_cases.py | 73 ++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 2 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 65afe7df..64c3ddc7 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -674,14 +674,25 @@ def _is_date_part_argument(cls, pre: str, suf: str, token: str) -> bool: # the literal splitter, but it must still be RECOGNISED as the part here -- otherwise # the other argument looks like the only keyword and a real column named `date` is # left unresolved. - first, second = (a.strip().strip("'\"").lower() for a in (args[0], args[1])) + first_raw, second_raw = (a.strip() for a in (args[0], args[1])) + first_quoted = first_raw[:1] in ("'", '"') + second_quoted = second_raw[:1] in ("'", '"') + first, second = (a.strip("'\"").lower() for a in (first_raw, second_raw)) first_kw = first in cls._DATE_PART_KEYWORDS second_kw = second in cls._DATE_PART_KEYWORDS # Normalize an abbreviation (mm, dd, ...) to its canonical name so rank/unit lookups # below see the real coarseness rather than defaulting to 99 / not-a-unit. first_n = cls._DATE_PART_ABBR.get(first, first) second_n = cls._DATE_PART_ABBR.get(second, second) - if first_kw and not second_kw: + if first_quoted and not second_quoted: + # A QUOTED argument is a string literal -> can only be the date PART, never the + # value column. Decide by quoting BEFORE the by-keyword tie-break, which would + # otherwise leave a value column sharing the unit's name (DATE_TRUNC('week', week)) + # unresolved. + part_index = 0 + elif second_quoted and not first_quoted: + part_index = 1 + elif first_kw and not second_kw: part_index = 0 # Snowflake order elif second_kw and not first_kw: part_index = 1 # BigQuery order @@ -3484,6 +3495,15 @@ def _complete_sql_fold_is_safe(sql: str) -> bool: for n in tree.find_all(exp.Anonymous): if (n.name or "").lower() in _ANONYMOUS_AGGREGATE_FUNCTIONS and len(n.expressions) > 1: return False + # The folder wraps ONLY an aggregate's `this` argument, so a standard multi-INPUT aggregate + # (CORR(x, y), COVAR_POP, REGR_*) would filter only its first input and leave the second over + # all rows -- a wrong statistic. Reject any AggFunc that has a column in an argument OTHER + # than `this`. (A WITHIN GROUP aggregate keeps its column in the enclosing WithinGroup, not + # inside the AggFunc, so it has none here and stays safe.) + for a in tree.find_all(exp.AggFunc): + this_col_ids = {id(c) for c in (a.this.find_all(exp.Column) if a.this else [])} + if any(id(c) not in this_col_ids for c in a.find_all(exp.Column)): + return False return all(len(d.expressions) <= 1 for d in tree.find_all(exp.Distinct)) @classmethod diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 9506be19..c71a9da7 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4920,6 +4920,38 @@ def test_lookml_export_folded_filter_protects_sql_server_datepart_first_function assert conds(["UPPER(day) = 'X'"], model) == "(UPPER((${TABLE}.order_day)) = 'X')" +def test_lookml_export_folded_filter_quoted_date_trunc_unit_resolves_value_column(): + """A QUOTED DATE_TRUNC unit (part) is decided by its quotes, so a same-named value column resolves. + + DATE_TRUNC('week', week) on a model with a `week` dimension: the quoted 'week' is the part and + the bare `week` is the value column. Stripping quotes made both look like the unit, and the + tie-break picked the value column as the part, leaving it unresolved. Quoting decides the part. + """ + from sidemantic import Dimension, Model + + model = Model( + name="orders", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="week", type="time", granularity="week", sql="order_week"), + Dimension(name="date", type="time", granularity="day", sql="order_date"), + ], + ) + conds = LookMLAdapter._fold_filter_conds + # Postgres/DuckDB order (quoted part first): the value column resolves, the quoted part stays. + assert conds(["DATE_TRUNC('week', week) = DATE '2024-01-01'"], model) == ( + "(DATE_TRUNC('week', (${TABLE}.order_week)) = DATE '2024-01-01')" + ) + assert conds(["DATE_TRUNC('month', date) = DATE '2024-01-01'"], model) == ( + "(DATE_TRUNC('month', (${TABLE}.order_date)) = DATE '2024-01-01')" + ) + # BigQuery order (unquoted, value first) is unaffected: date resolves, month is the part. + assert conds(["DATE_TRUNC(date, month) = DATE '2024-01-01'"], model) == ( + "(DATE_TRUNC((${TABLE}.order_date), month) = DATE '2024-01-01')" + ) + + def test_lookml_export_folded_filter_protects_sql_server_datepart_abbreviations(): """SQL Server datepart ABBREVIATIONS (dd, mm, d, ...) in a part slot must be protected. @@ -5357,6 +5389,47 @@ def test_lookml_export_multi_column_distinct_filter_skipped(): assert "measure: du" not in open(out).read() # skipped, no malformed `THEN a, b END` +def test_lookml_export_multi_input_aggregate_filter_skipped(): + """A filtered two-input aggregate (CORR(x, y)) can't fold consistently -> skipped, not mis-filtered. + + The folder wraps only the aggregate's first argument, so CORR(CASE WHEN ... THEN x END, y) would + filter x but leave y over all rows -- a wrong statistic. _complete_sql_fold_is_safe rejects it. + """ + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + assert LookMLAdapter._complete_sql_fold_is_safe("CORR({model}.x, {model}.y)") is False + assert LookMLAdapter._complete_sql_fold_is_safe("COVAR_POP({model}.x, {model}.y)") is False + assert LookMLAdapter._complete_sql_fold_is_safe("SUM({model}.amount) / COUNT(*)") is True + + graph = SemanticGraph() + graph.add_model( + Model( + name="orders", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="status", type="categorical", sql="status"), + ], + metrics=[ + Metric( + name="c", + agg=None, + sql="CORR({model}.x, {model}.y)", + sql_is_complete=True, + filters=["{model}.status = 'x'"], + ) + ], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + assert "measure: c" not in open(out).read() # skipped rather than filtering only the first input + + def test_lookml_export_aggregate_with_scalar_subquery_skipped(): """An agg-less complete measure carrying a scalar subquery is skipped, not silently lost on re-import. From ceb700b9d29bb5191eac70244f8d9139de0c6b51 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 16:23:10 -0700 Subject: [PATCH 35/44] Drop post-SQL measures whose base complete SQL cannot be resolved A percent_of_total / percent_of_previous over a filtered type: number base whose complete SQL was not cached -- e.g. a base using a dialect- renamed aggregate (APPROX_COUNT_DISTINCT) whose force-fold prepass bails -- fell through to a bare {model}.. A complete number measure is projected via a raw alias, not a column, so the post-SQL measure compiled against a missing column and produced an empty CTE. Drop the post-SQL measure when a base measure ref is neither a native aggregate nor a cached complete SQL. Resolvable bases are unaffected. --- sidemantic/adapters/lookml.py | 26 ++++++++++++++++++++ tests/adapters/lookml/test_edge_cases.py | 31 ++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 64c3ddc7..4ef70a02 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -3054,6 +3054,32 @@ def _parse_post_sql_measure( format=measure_def.get("value_format"), ) + # A base ${ref} that is a measure but has NO resolvable form -- not a simple aggregate + # (measure_agg_lookup) and not a cached complete SQL (measure_full_sql_lookup, e.g. a + # filtered type: number whose force-fold prepass bailed on a dialect rename) -- would fall + # through to a bare {model}.. A complete number measure is projected via a raw + # alias, not a column, so this post-SQL measure would compile against a missing + # column. Drop it rather than emit an empty/missing-column CTE. + for _m in self._REF_RE.finditer(sql): + _rn = _m.group(2) + if ( + _m.group(1) is None + and _rn != "TABLE" + and _rn in measure_names + and _rn not in measure_agg_lookup + and _rn not in measure_full_sql_lookup + and _rn not in dimension_sql_lookup + ): + logger.warning( + "LookML %s measure %r references base measure %r whose complete SQL could not be " + "resolved (e.g. a filtered type: number with a dialect-renamed aggregate); " + "dropping the measure rather than compiling a missing-column reference.", + measure_type, + name, + _rn, + ) + return None + # percent_of_total / percent_of_previous build window aggregates inline, # so qualify base measure refs with {model} (for the generator's _raw # column rewrite) and wrap them in the base measure's aggregate function. diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index c71a9da7..68a3aa2a 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4224,6 +4224,37 @@ def test_lookml_post_sql_measure_expands_untemplated_complete_base(): assert con.execute(sql).fetchall() == [(1.0,)] # single group -> its share is 1.0 +def test_lookml_post_sql_measure_with_unresolvable_base_dropped(): + """A percent_of_total over a filtered type:number base whose complete SQL can't be cached is dropped. + + A filtered type: number using a dialect-renamed aggregate (APPROX_COUNT_DISTINCT) has its + force-fold prepass bail, so the base is not in measure_full_sql_lookup; the ref would fall + through to a bare {model}. the complete measure never projects as a column. Drop the + post-SQL measure instead of emitting a missing-column CTE. A resolvable base still works. + """ + graph = _parse_lkml( + """ +view: orders { + sql_table_name: orders ;; + dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } + dimension: status { type: string sql: ${TABLE}.status ;; } + measure: approx_ratio { + type: number + sql: APPROX_COUNT_DISTINCT(${TABLE}.user_id) / NULLIF(COUNT(*), 0) ;; + filters: [status: "completed"] + } + measure: total { type: sum sql: ${TABLE}.amount ;; } + measure: pct_bad { type: percent_of_total sql: ${approx_ratio} ;; } + measure: pct_ok { type: percent_of_total sql: ${total} ;; } +} +""" + ) + names = {m.name for m in graph.get_model("orders").metrics} + assert "pct_bad" not in names # unresolvable base -> dropped + assert "pct_ok" in names # resolvable native base -> kept + assert "approx_ratio" in names # the base measure itself still imports + + def test_lookml_export_running_total_roundtrips(): """An imported running_total (cumulative + table_calculation meta) round-trips, not dropped.""" import tempfile From 1e8a48f2edd8ee9b973b273d1fb2832090a72761 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 16:31:41 -0700 Subject: [PATCH 36/44] Only treat single-quoted DATE_TRUNC values as the date part The quoted-argument shortcut for the DATE_TRUNC part/value tie-break accepted double-quoted tokens too, but a double-quoted token is a quoted IDENTIFIER (a column), not a string literal. So DATE_TRUNC(month, "date") treated "date" as the part and rewrote the real `month` part token to a same-named dimension's SQL. Only a single-quoted string literal triggers the shortcut now. --- sidemantic/adapters/lookml.py | 7 +++++-- tests/adapters/lookml/test_edge_cases.py | 5 +++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 4ef70a02..78761293 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -675,8 +675,11 @@ def _is_date_part_argument(cls, pre: str, suf: str, token: str) -> bool: # the other argument looks like the only keyword and a real column named `date` is # left unresolved. first_raw, second_raw = (a.strip() for a in (args[0], args[1])) - first_quoted = first_raw[:1] in ("'", '"') - second_quoted = second_raw[:1] in ("'", '"') + # Only a SINGLE-quoted string literal is a date PART shortcut. A double-quoted token is + # a quoted IDENTIFIER (a column, e.g. Snowflake DATE_TRUNC(month, "date")), so it must + # NOT be treated as the part -- otherwise the real part token loses its protection. + first_quoted = first_raw[:1] == "'" + second_quoted = second_raw[:1] == "'" first, second = (a.strip("'\"").lower() for a in (first_raw, second_raw)) first_kw = first in cls._DATE_PART_KEYWORDS second_kw = second in cls._DATE_PART_KEYWORDS diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 68a3aa2a..30178f05 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4981,6 +4981,11 @@ def test_lookml_export_folded_filter_quoted_date_trunc_unit_resolves_value_colum assert conds(["DATE_TRUNC(date, month) = DATE '2024-01-01'"], model) == ( "(DATE_TRUNC((${TABLE}.order_date), month) = DATE '2024-01-01')" ) + # A DOUBLE-quoted second argument is a quoted IDENTIFIER, not a string-literal part, so it must + # NOT be treated as the part -- the real `month` part token stays protected (Snowflake style). + assert conds(["DATE_TRUNC(month, \"date\") = DATE '2024-01-01'"], model) == ( + "(DATE_TRUNC(month, \"date\") = DATE '2024-01-01')" + ) def test_lookml_export_folded_filter_protects_sql_server_datepart_abbreviations(): From fdac952e77d742a29d8c17faf1f8ea7174d10034 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 16:46:17 -0700 Subject: [PATCH 37/44] Add Snowflake TIMEADD/TIMEDIFF and TRUNC/TRUNCATE to the date-part guard Folded filters using Snowflake's TIMEADD/TIMEDIFF (datepart-first aliases of DATEADD/DATEDIFF) or the reversed-argument TRUNC/TRUNCATE alternative to DATE_TRUNC (datepart last) left the unit token unprotected, so a model with a same-named dimension rewrote it to that dimension's SQL. Add them to the date-part argument-position map. A numeric TRUNC(n, 2) is unaffected -- the trailing 2 is not a date part. --- sidemantic/adapters/lookml.py | 6 +++++ tests/adapters/lookml/test_edge_cases.py | 33 ++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 78761293..56fb1cb1 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -553,6 +553,12 @@ def _sql_has_list_aggregate(sql: str) -> bool: # datepart-first family. Without them a folded filter's DATENAME(day, col) leaves `day` # unprotected, so a model with a `day` dimension rewrites it to (${TABLE}.order_day). "datename": 0, "datediff_big": 0, "date_bucket": 0, + # Snowflake TIMEADD/TIMEDIFF are documented aliases of DATEADD/DATEDIFF (datepart first). + "timeadd": 0, "timediff": 0, + # Snowflake's reversed-argument TRUNC/TRUNCATE alternative to DATE_TRUNC: TRUNC(expr, part), + # so the date part is the LAST argument. A numeric TRUNC(n, 2) is unaffected -- the trailing + # `2` is not a date-part keyword, so it is never treated as a part. + "trunc": -1, "truncate": -1, } # fmt: skip # DATE_TRUNC has NO fixed part position: BigQuery is DATE_TRUNC(value, part) while Snowflake is # DATE_TRUNC(part, expr), and the adapter has no dialect context. Disambiguate by CONTENT -- diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 30178f05..3cf0c388 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4951,6 +4951,39 @@ def test_lookml_export_folded_filter_protects_sql_server_datepart_first_function assert conds(["UPPER(day) = 'X'"], model) == "(UPPER((${TABLE}.order_day)) = 'X')" +def test_lookml_export_folded_filter_protects_snowflake_timeadd_and_trunc_parts(): + """Snowflake TIMEADD/TIMEDIFF (datepart first) and TRUNC/TRUNCATE (datepart last) protect the part. + + Without these aliases a folded filter's TIMEADD(day, 1, x) or TRUNC(x, month) rewrote the unit + to a same-named dimension's SQL. Numeric TRUNC(n, 2) is unaffected -- the trailing 2 is not a + date part. + """ + from sidemantic import Dimension, Model + + model = Model( + name="orders", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="day", type="time", granularity="day", sql="order_day"), + Dimension(name="month", type="time", granularity="month", sql="order_month"), + Dimension(name="created_at", type="time", granularity="day", sql="created_at"), + Dimension(name="n", type="numeric", sql="num_col"), + ], + ) + conds = LookMLAdapter._fold_filter_conds + assert conds(["TIMEADD(day, 1, created_at) > 1"], model) == "(TIMEADD(day, 1, (${TABLE}.created_at)) > 1)" + assert conds(["TIMEDIFF(day, created_at, created_at) > 1"], model) == ( + "(TIMEDIFF(day, (${TABLE}.created_at), (${TABLE}.created_at)) > 1)" + ) + # TRUNC(expr, part): the LAST arg is the part -> month protected, created_at resolves. + assert conds(["TRUNC(created_at, month) = DATE '2024-01-01'"], model) == ( + "(TRUNC((${TABLE}.created_at), month) = DATE '2024-01-01')" + ) + # Numeric TRUNC(n, 2): the trailing 2 is not a date part, so the real column resolves. + assert conds(["TRUNC(n, 2) > 5"], model) == "(TRUNC((${TABLE}.num_col), 2) > 5)" + + def test_lookml_export_folded_filter_quoted_date_trunc_unit_resolves_value_column(): """A QUOTED DATE_TRUNC unit (part) is decided by its quotes, so a same-named value column resolves. From 910db9efdaeedf381dcb7a95f0e555c3fe2aab65 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 16:58:34 -0700 Subject: [PATCH 38/44] Keep nullary SQL constants unqualified in folded filters A folded filter over a dimension whose SQL is a nullary SQL constant (CURRENT_DATE, CURRENT_TIMESTAMP, ...) table-qualified it to ${TABLE}.CURRENT_DATE, a nonexistent column, because the constant is identifier-shaped. Add these constants to the set left unqualified alongside numeric / true / false / null literals. --- sidemantic/adapters/lookml.py | 25 ++++++++++++++++++++---- tests/adapters/lookml/test_edge_cases.py | 5 +++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 56fb1cb1..84cd4cbd 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -521,6 +521,17 @@ def _sql_has_list_aggregate(sql: str) -> bool: } ) # fmt: skip + # Nullary SQL constants that read like a bare identifier but are VALUES, not columns -- a folded + # filter must NOT table-qualify a resolved dimension SQL equal to one of these (${TABLE}.CURRENT_DATE + # is invalid; the constant should stay bare). + _SQL_NULLARY_CONSTANTS = frozenset( + { + "current_date", "current_time", "current_timestamp", "localtime", "localtimestamp", + "current_user", "session_user", "system_user", "current_role", "current_catalog", + "current_schema", "current_database", "sysdate", "user", + } + ) # fmt: skip + _DATE_PART_KEYWORDS = frozenset( { "year", "quarter", "month", "week", "day", "hour", "minute", "second", @@ -3357,10 +3368,16 @@ def _fold_filter_conds(cls, filters: list[str], model: Model) -> str: def _qualify(val: str) -> str: # Bare column -> qualify with {model}. so it stays unambiguous in joins; any resolved # expression is parenthesized to preserve precedence. But an identifier-shaped SQL - # LITERAL -- a numeric constant (`1`) or true/false/null -- is a VALUE, not a column, so - # parenthesize it WITHOUT a table qualifier (${TABLE}.1 is invalid SQL). A column name - # cannot start with a digit unquoted, so a leading digit marks a numeric literal. - if re.fullmatch(r"\w+", val) and not val[0].isdigit() and val.lower() not in ("true", "false", "null"): + # LITERAL -- a numeric constant (`1`), true/false/null, or a nullary SQL constant + # (CURRENT_DATE, CURRENT_TIMESTAMP, ...) -- is a VALUE, not a column, so parenthesize it + # WITHOUT a table qualifier (${TABLE}.1 / ${TABLE}.CURRENT_DATE are invalid SQL). A column + # name cannot start with a digit unquoted, so a leading digit marks a numeric literal. + if ( + re.fullmatch(r"\w+", val) + and not val[0].isdigit() + and val.lower() not in ("true", "false", "null") + and val.lower() not in cls._SQL_NULLARY_CONSTANTS + ): return f"({{model}}.{val})" return f"({val})" diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 3cf0c388..309076b2 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -5575,12 +5575,17 @@ def test_lookml_export_folded_filter_keeps_constant_dimension_sql_unqualified(): Dimension(name="status", type="categorical", sql="status_col"), Dimension(name="flag", type="numeric", sql="1"), # constant-valued dimension Dimension(name="active", type="boolean", sql="TRUE"), + Dimension(name="today", type="time", sql="CURRENT_DATE"), # nullary SQL constant + Dimension(name="ts", type="time", sql="current_timestamp"), ], ) conds = LookMLAdapter._fold_filter_conds # A constant/literal resolved SQL is parenthesized WITHOUT a table qualifier. assert conds(["{model}.flag = 1"], model) == "((1) = 1)" assert conds(["{model}.active = true"], model) == "((TRUE) = true)" + # A nullary SQL constant (CURRENT_DATE / current_timestamp) is a value, not a column. + assert conds(["{model}.today = CURRENT_DATE"], model) == "((CURRENT_DATE) = CURRENT_DATE)" + assert conds(["{model}.ts > current_timestamp"], model) == "((current_timestamp) > current_timestamp)" # A genuine column still gets qualified. assert conds(["{model}.status = 'x'"], model) == "((${TABLE}.status_col) = 'x')" From 0bf47877f592ba86348971f72829c3acf36db7ff Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 17:15:49 -0700 Subject: [PATCH 39/44] Recognize ISO DATE_TRUNC units and disambiguate numeric TRUNC Two folded-filter date-part fixes: - DATE_TRUNC(week, isoweek) left the value column `week` unresolved: the ISO truncation units isoweek/isoyear were absent from the unit set, so the tie-break treated the value column as the part. Add them. - TRUNC also has a NUMERIC overload (TRUNC(number, scale)), so marking its last argument a date part rewrote a numeric scale column named like a unit (TRUNC(amount, month)). Guard TRUNC's part slot only when the value argument is a known time dimension; a date/time value (TRUNC(created_at, month)) still protects the part. --- sidemantic/adapters/lookml.py | 28 +++++++++-- tests/adapters/lookml/test_edge_cases.py | 59 ++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 3 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 84cd4cbd..6407c71c 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -595,7 +595,10 @@ def _sql_has_list_aggregate(sql: str) -> bool: # for both args, but only `day` is a trunc unit, so it is the part and `date` is the column. _DATE_TRUNC_UNITS = frozenset( {"millennium", "century", "decade", "year", "quarter", "month", "week", "day", - "hour", "minute", "second", "millisecond", "microsecond", "nanosecond"} + "hour", "minute", "second", "millisecond", "microsecond", "nanosecond", + # ISO truncation units (BigQuery isoweek/isoyear). Without them a DATE_TRUNC(week, isoweek) + # tie-break saw only `week` as a unit and treated the value column as the part. + "isoweek", "isoyear"} ) # fmt: skip @staticmethod @@ -666,13 +669,21 @@ def _enclosing_call_arg_texts(cls, pre: str, suf: str, token: str) -> list[str]: call_args = pre[open_pos + 1 :] + token + suf[:end] return [a.strip() for a in cls._split_top_level_commas(call_args, quote_aware=True)] + # Functions with a NUMERIC overload as well as a date/time one: TRUNC(date, part) vs + # TRUNC(number, scale). Only guard their part slot with evidence the value argument is date/time. + _DATE_PART_NUMERIC_OVERLOAD = frozenset({"trunc", "truncate"}) + @classmethod - def _is_date_part_argument(cls, pre: str, suf: str, token: str) -> bool: + def _is_date_part_argument(cls, pre: str, suf: str, token: str, time_dim_names: set | None = None) -> bool: """True if ``token`` occupies the date-PART argument slot of a date/time call. Position-aware on purpose: DATE_TRUNC(date, month) on a model with both a `date` and a `month` dimension means column `date` truncated to part `month` -- only the LAST argument is the part, so the `date` column must still resolve. + + ``time_dim_names`` disambiguates a numeric-overloaded function (TRUNC): its part slot is + guarded only when the value argument is a known time dimension, so a numeric + ``TRUNC(amount, month)`` (month = a scale column) still resolves `month` to its dimension. """ if token.lower() not in cls._DATE_PART_KEYWORDS: return False @@ -743,6 +754,14 @@ def _is_date_part_argument(cls, pre: str, suf: str, token: str) -> bool: want = cls._DATE_PART_ARG_POS.get(func.lower()) if want is None: return False + if func.lower() in cls._DATE_PART_NUMERIC_OVERLOAD: + # TRUNC/TRUNCATE is also numeric (TRUNC(number, scale)). Only treat the part slot as a + # date part when the VALUE argument is a known time dimension; otherwise a scale column + # named like a unit (TRUNC(amount, month)) must still resolve to its dimension SQL. + _args = cls._enclosing_call_arg_texts(pre, suf, token) + _value = _args[0].strip().strip("`\"[]'") if _args else "" + if not (time_dim_names and _value in time_dim_names): + return False if want >= 0: return arg_index == want # want == -1: the part is the LAST argument -- true when no top-level comma follows the @@ -3364,6 +3383,9 @@ def _fold_filter_conds(cls, filters: list[str], model: Model) -> str: """ dim_sql = {d.name: d.sql for d in model.dimensions if d.sql} dim_names = {d.name for d in model.dimensions} + # Time-dimension names disambiguate a numeric-overloaded date function (TRUNC): its part + # slot is only guarded when the value argument is one of these (see _is_date_part_argument). + time_dim_names = {d.name for d in model.dimensions if getattr(d, "type", None) == "time"} def _qualify(val: str) -> str: # Bare column -> qualify with {model}. so it stays unambiguous in joins; any resolved @@ -3449,7 +3471,7 @@ def _one(m): # DATE_DIFF(a, b, day). Position-aware so a real column in another # slot of the SAME call still resolves -- DATE_TRUNC(date, month) is # column `date` truncated to part `month`. - or cls._is_date_part_argument(pre, suf, bare) + or cls._is_date_part_argument(pre, suf, bare, time_dim_names) ): return m.group(0) name = m.group(1) or bare diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 309076b2..5c645ddd 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4984,6 +4984,65 @@ def test_lookml_export_folded_filter_protects_snowflake_timeadd_and_trunc_parts( assert conds(["TRUNC(n, 2) > 5"], model) == "(TRUNC((${TABLE}.num_col), 2) > 5)" +def test_lookml_export_folded_filter_numeric_trunc_scale_column_resolves(): + """TRUNC's numeric overload must not protect a scale column named like a date part. + + TRUNC(amount, month) where amount is numeric is TRUNC(number, scale), so `month` is a scale + COLUMN and must resolve to its dimension SQL -- only a date/time value argument makes the last + arg a date part (TRUNC(created_at, month)). + """ + from sidemantic import Dimension, Model + + numeric = Model( + name="orders", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="amount", type="numeric", sql="amt"), + Dimension(name="month", type="numeric", sql="scale_col"), + ], + ) + conds = LookMLAdapter._fold_filter_conds + # Numeric value arg -> `month` is a scale column, resolved (not protected as a part). + assert conds(["TRUNC(amount, month) > 0"], numeric) == "(TRUNC((${TABLE}.amt), (${TABLE}.scale_col)) > 0)" + + dated = Model( + name="orders", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="created_at", type="time", granularity="day", sql="created_at"), + Dimension(name="month", type="time", granularity="month", sql="order_month"), + ], + ) + # Time value arg -> `month` is the date part, protected. + assert conds(["TRUNC(created_at, month) = DATE '2024-01-01'"], dated) == ( + "(TRUNC((${TABLE}.created_at), month) = DATE '2024-01-01')" + ) + + +def test_lookml_export_folded_filter_iso_date_trunc_units_resolve_value_column(): + """DATE_TRUNC ISO units (isoweek/isoyear) are recognized so the value column still resolves.""" + from sidemantic import Dimension, Model + + model = Model( + name="orders", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="week", type="time", granularity="week", sql="order_week"), + Dimension(name="year", type="time", granularity="year", sql="order_year"), + ], + ) + conds = LookMLAdapter._fold_filter_conds + assert conds(["DATE_TRUNC(week, isoweek) = DATE '2024-01-01'"], model) == ( + "(DATE_TRUNC((${TABLE}.order_week), isoweek) = DATE '2024-01-01')" + ) + assert conds(["DATE_TRUNC(year, isoyear) = DATE '2024-01-01'"], model) == ( + "(DATE_TRUNC((${TABLE}.order_year), isoyear) = DATE '2024-01-01')" + ) + + def test_lookml_export_folded_filter_quoted_date_trunc_unit_resolves_value_column(): """A QUOTED DATE_TRUNC unit (part) is decided by its quotes, so a same-named value column resolves. From 86c8e3f2b05284c2f2102137bc76d5a8f8078252 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 17:25:19 -0700 Subject: [PATCH 40/44] Make the folded-filter {model} replacement quote-aware The final {model} -> ${TABLE} replacement in a folded filter ran on the whole resolved string, so a {model} inside a string LITERAL (a filter like {model}.label = '{model}.status') had its literal value rewritten. Replace the placeholder only outside quoted literals/identifiers. --- sidemantic/adapters/lookml.py | 15 ++++++++++++++- tests/adapters/lookml/test_edge_cases.py | 19 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 6407c71c..b78761bf 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -620,6 +620,19 @@ def _blank_string_literals(s: str) -> str: quote = ch return "".join(out) + @staticmethod + def _model_to_table_outside_quotes(s: str) -> str: + """Replace the ``{model}`` placeholder with ``${TABLE}`` OUTSIDE quoted literals/identifiers. + + A blanket ``.replace`` would rewrite a ``{model}`` that is part of a string VALUE + (``label = '{model}.status'``), changing the matched literal; only real placeholders are + converted here. + """ + parts = re.split(r"""('(?:[^']|'')*'|"(?:[^"]|"")*"|`[^`]*`|\[[^\]]*\])""", s) + for i in range(0, len(parts), 2): # even indices sit OUTSIDE quoted segments + parts[i] = parts[i].replace("{model}", "${TABLE}") + return "".join(parts) + @staticmethod def _enclosing_call(pre: str) -> tuple[str | None, int, int]: """Describe the call a token sits in, given the text ``pre`` before it. @@ -3503,7 +3516,7 @@ def _one(m): offset += len(part) return "".join(parts) - return " AND ".join("(" + _resolve(f).replace("{model}", "${TABLE}") + ")" for f in filters) + return " AND ".join("(" + cls._model_to_table_outside_quotes(_resolve(f)) + ")" for f in filters) @staticmethod def _aggregate_references_column(sql: str) -> bool: diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 5c645ddd..7f1c7e90 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -5678,6 +5678,25 @@ def test_lookml_export_folded_filter_leaves_backtick_identifier_untouched(): assert "`(${TABLE}" not in text # not rewritten inside the backticks +def test_lookml_export_folded_filter_model_placeholder_in_string_literal_preserved(): + """A `{model}` inside a string LITERAL is a value, not a placeholder, so it must not become ${TABLE}.""" + from sidemantic import Dimension, Model + + model = Model( + name="orders", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="label", type="categorical", sql="label_col"), + Dimension(name="status", type="categorical", sql="status_col"), + ], + ) + conds = LookMLAdapter._fold_filter_conds + # The literal value keeps `{model}`; a real placeholder outside the quotes is converted. + assert conds(["{model}.label = '{model}.status'"], model) == "((${TABLE}.label_col) = '{model}.status')" + assert conds(["{model}.label = {model}.status"], model) == "((${TABLE}.label_col) = (${TABLE}.status_col))" + + def test_lookml_export_string_literal_paren_in_aggregate_arg_folds(): """A valid aggregate whose arg has a paren inside a STRING LITERAL must fold, not be rejected.""" import tempfile From 1e1a4fc257900a4fb697ec590f5a4fe646e30efb Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 17:41:11 -0700 Subject: [PATCH 41/44] Strip qualifiers for TRUNC value check and skip LIST/ARRAY_AGG export folds Two folded-filter export fixes: - The numeric-TRUNC overload guard compared the raw value argument ({model}.created_at) against bare time-dimension names, so a qualified date value was not recognized and TRUNC's part token was rewritten. Strip the model/table qualifier before the check. - The export fold-safety check accepted a NULL-retaining array collector (LIST / ARRAY_AGG), folding the filter into LIST(CASE ...) even though the excluded row becomes a NULL element (the import path already rejects this). Reject these so the measure is skipped on export. --- sidemantic/adapters/lookml.py | 11 ++++++++++- tests/adapters/lookml/test_edge_cases.py | 10 ++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index b78761bf..96acf233 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -772,7 +772,10 @@ def _is_date_part_argument(cls, pre: str, suf: str, token: str, time_dim_names: # date part when the VALUE argument is a known time dimension; otherwise a scale column # named like a unit (TRUNC(amount, month)) must still resolve to its dimension SQL. _args = cls._enclosing_call_arg_texts(pre, suf, token) - _value = _args[0].strip().strip("`\"[]'") if _args else "" + # The value is normally qualified in a folded filter ({model}.created_at); strip the + # model placeholder / a bare table qualifier and any quotes so it matches a time dim name. + _value = _args[0].strip() if _args else "" + _value = re.sub(r"^(?:\{model\}|\$\{TABLE\}|\w+)\.", "", _value).strip("`\"[]'") if not (time_dim_names and _value in time_dim_names): return False if want >= 0: @@ -3575,6 +3578,12 @@ def _complete_sql_fold_is_safe(sql: str) -> bool: tree = sqlglot.parse_one(sql.replace("{model}", "__m__")) except Exception: return False + # A NULL-retaining array collector (LIST / ARRAY_AGG) cannot be filtered by folding a CASE + # into its argument -- the excluded row becomes a NULL element rather than disappearing + # (ARRAY_LENGTH still counts it), exactly as the import path rejects. Skip so the measure is + # not exported with an ineffective filter. + if any(tree.find_all(exp.List)) or any(tree.find_all(exp.ArrayAgg)): + return False # An ORDER BY in an aggregate's argument list would be buried in the CASE by folding; a # WITHIN GROUP ORDER BY is folded correctly, so reject only the former. if any(o.find_ancestor(exp.WithinGroup) is None for o in tree.find_all(exp.Order)): diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index 7f1c7e90..f92fc9b4 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -5019,6 +5019,12 @@ def test_lookml_export_folded_filter_numeric_trunc_scale_column_resolves(): assert conds(["TRUNC(created_at, month) = DATE '2024-01-01'"], dated) == ( "(TRUNC((${TABLE}.created_at), month) = DATE '2024-01-01')" ) + # The QUALIFIED form ({model}.created_at) must still be recognized as a time value. + assert conds(["TRUNC({model}.created_at, month) = DATE '2024-01-01'"], dated) == ( + "(TRUNC((${TABLE}.created_at), month) = DATE '2024-01-01')" + ) + # And a qualified numeric value still resolves the scale column. + assert conds(["TRUNC({model}.amount, month) > 0"], numeric) == ("(TRUNC((${TABLE}.amt), (${TABLE}.scale_col)) > 0)") def test_lookml_export_folded_filter_iso_date_trunc_units_resolve_value_column(): @@ -5301,6 +5307,10 @@ def test_lookml_export_ordered_set_aggregate_with_filter_skipped(): assert LookMLAdapter._complete_sql_fold_is_safe("SUM({model}.amount ORDER BY {model}.created_at)") is False # A plain aggregate (no ORDER BY) is still foldable. assert LookMLAdapter._complete_sql_fold_is_safe("SUM({model}.amount) / COUNT({model}.id)") is True + # A NULL-retaining array collector (LIST/ARRAY_AGG) is NOT foldable -- a folded CASE leaves the + # excluded row as a NULL element, so the measure must be skipped on export (as import does). + assert LookMLAdapter._complete_sql_fold_is_safe("ARRAY_LENGTH(LIST({model}.amount))") is False + assert LookMLAdapter._complete_sql_fold_is_safe("ARRAY_LENGTH(ARRAY_AGG({model}.amount))") is False graph = SemanticGraph() graph.add_model( From bf24ccc5263bdefb9af4a832610d1e62d37615e4 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 17:53:10 -0700 Subject: [PATCH 42/44] Add LAST_DAY to the folded-filter date-part guard The two-argument LAST_DAY(expr, part) form left its part token unprotected, so a folded filter's LAST_DAY(created_at, month) rewrote `month` to a same-named dimension's SQL. Add last_day with a last- argument date-part position; single-argument LAST_DAY(date) is unaffected. --- sidemantic/adapters/lookml.py | 3 +++ tests/adapters/lookml/test_edge_cases.py | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 96acf233..d33740e3 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -570,6 +570,9 @@ def _sql_has_list_aggregate(sql: str) -> bool: # so the date part is the LAST argument. A numeric TRUNC(n, 2) is unaffected -- the trailing # `2` is not a date-part keyword, so it is never treated as a part. "trunc": -1, "truncate": -1, + # BigQuery/Snowflake two-argument LAST_DAY(expr, part): the part is the LAST argument. The + # single-argument LAST_DAY(date) is unaffected (no trailing date-part keyword). + "last_day": -1, } # fmt: skip # DATE_TRUNC has NO fixed part position: BigQuery is DATE_TRUNC(value, part) while Snowflake is # DATE_TRUNC(part, expr), and the adapter has no dialect context. Disambiguate by CONTENT -- diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index f92fc9b4..a4f83107 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4982,6 +4982,13 @@ def test_lookml_export_folded_filter_protects_snowflake_timeadd_and_trunc_parts( ) # Numeric TRUNC(n, 2): the trailing 2 is not a date part, so the real column resolves. assert conds(["TRUNC(n, 2) > 5"], model) == "(TRUNC((${TABLE}.num_col), 2) > 5)" + # Two-argument LAST_DAY(expr, part): the part is protected; single-argument LAST_DAY is unaffected. + assert conds(["LAST_DAY(created_at, month) = DATE '2024-01-31'"], model) == ( + "(LAST_DAY((${TABLE}.created_at), month) = DATE '2024-01-31')" + ) + assert conds(["LAST_DAY(created_at) = DATE '2024-01-31'"], model) == ( + "(LAST_DAY((${TABLE}.created_at)) = DATE '2024-01-31')" + ) def test_lookml_export_folded_filter_numeric_trunc_scale_column_resolves(): From e6be1bcb1f261d5ead7045d55a8c862347d03154 Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 18:54:30 -0700 Subject: [PATCH 43/44] Disambiguate date_diff part position by content and strip a leading ALL modifier --- sidemantic/adapters/lookml.py | 45 ++++++++++++++- tests/adapters/lookml/test_edge_cases.py | 73 ++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 1 deletion(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index d33740e3..833f1282 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -556,7 +556,7 @@ def _sql_has_list_aggregate(sql: str) -> bool: # keyword-named column. EXTRACT(part FROM x) and INTERVAL n part have their own position checks. _DATE_PART_ARG_POS = { "timestamp_trunc": -1, "datetime_trunc": -1, "time_trunc": -1, - "date_diff": -1, "timestamp_diff": -1, "datetime_diff": -1, "time_diff": -1, + "timestamp_diff": -1, "datetime_diff": -1, "time_diff": -1, "datetrunc": 0, "datediff": 0, "dateadd": 0, "datepart": 0, "date_part": 0, "timestampadd": 0, "timestampdiff": 0, "timestampdiff_big": 0, "datetimediff": 0, # SQL Server functions taking the datepart as the FIRST argument. datediff_big is the @@ -578,6 +578,13 @@ def _sql_has_list_aggregate(sql: str) -> bool: # DATE_TRUNC(part, expr), and the adapter has no dialect context. Disambiguate by CONTENT -- # whichever argument is a date-part keyword is the part (see _is_date_part_argument). _DATE_PART_AMBIGUOUS_TRUNC = frozenset({"date_trunc"}) + # date_diff has NO fixed part position either: BigQuery is DATE_DIFF(end, start, part) (part + # LAST) while DuckDB is date_diff(part, start, end) (part FIRST), same name and no dialect + # context. Disambiguate by CONTENT -- whichever END argument is a date-part keyword is the part + # (see _is_date_part_argument). The unambiguous spellings stay in _DATE_PART_ARG_POS: `datediff` + # (no underscore) is part-FIRST everywhere, and BigQuery's timestamp_diff/datetime_diff/time_diff + # are part-LAST. + _DATE_PART_AMBIGUOUS_DIFF = frozenset({"date_diff"}) # Coarseness rank of each date-part keyword (LOWER = coarser). Used only to break the tie when # BOTH arguments of an ambiguous DATE_TRUNC are keywords (a model with `date` AND `month` # dimensions): truncation always goes finer -> coarser, so the COARSER keyword is the part and @@ -767,6 +774,34 @@ def _is_date_part_argument(cls, pre: str, suf: str, token: str, time_dim_names: else: return False return arg_index == part_index + if func.lower() in cls._DATE_PART_AMBIGUOUS_DIFF: + # date_diff(a, b, c): BigQuery puts the part LAST, DuckDB puts it FIRST. The token is + # already known to be a date-part keyword, so it is the part when it sits at a candidate + # END (first or last argument) and the OTHER end is not itself a keyword. If both ends + # look like keywords (a column literally named after a unit), fall back to BigQuery's + # part-LAST order. A part in the MIDDLE argument never occurs, so reject it. + args = cls._enclosing_call_arg_texts(pre, suf, token) + if len(args) < 2: + return False + first = args[0].strip().strip("'\"").lower() + last = args[-1].strip().strip("'\"").lower() + first_kw = first in cls._DATE_PART_KEYWORDS + last_kw = last in cls._DATE_PART_KEYWORDS + if first_kw and not last_kw: + part_index = 0 + elif last_kw and not first_kw: + part_index = len(args) - 1 + else: + # Both ends look like keywords (a column literally named after a unit, e.g. a `date` + # dimension). A diff PART is a genuine unit (day, month, ...), so prefer the end that + # is a real truncation/diff unit; otherwise fall back to BigQuery's part-LAST order. + first_unit = first in cls._DATE_TRUNC_UNITS + last_unit = last in cls._DATE_TRUNC_UNITS + if first_unit and not last_unit: + part_index = 0 + else: + part_index = len(args) - 1 + return arg_index == part_index want = cls._DATE_PART_ARG_POS.get(func.lower()) if want is None: return False @@ -859,10 +894,18 @@ def _strip_all_modifier(sql: str) -> str: string literal like ``'(ALL users)'`` is DATA, not a modifier, so the substitution runs only on the segments outside any quoted token (``(`` must precede ``ALL`` regardless, so ``= ALL (SELECT ...)`` and a column named ``all`` are already untouched). + + A LEADING ``ALL`` is also dropped: a metric SQL that IS the bare aggregate argument + (``Metric(agg="stddev", sql="ALL {model}.amount")``) has no ``(`` yet, and wrapping it as + ``STDDEV(ALL amount)`` is likewise unparseable, so the exported measure would not round-trip. """ parts = re.split(r"""('(?:[^']|'')*'|"(?:[^"]|"")*"|`[^`]*`|\[[^\]]*\])""", sql or "") for i in range(0, len(parts), 2): # even indices are outside any quoted token parts[i] = re.sub(r"(?i)\(\s*ALL\s+", "(", parts[i]) + # Only the first outside-quotes segment can hold the string's start; a `\s+` after ALL keeps + # a column named `all` (or an `all(` function) untouched. + if parts: + parts[0] = re.sub(r"(?i)^\s*ALL\s+", "", parts[0]) return "".join(parts) @classmethod diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index a4f83107..ff6284dc 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -4888,6 +4888,44 @@ def test_lookml_export_folded_filter_date_part_guard_is_position_aware(): ) +def test_lookml_export_folded_filter_date_diff_part_position_is_content_based(): + """date_diff's part is at EITHER end depending on dialect, so decide by content, not position. + + BigQuery is DATE_DIFF(end, start, part) (part LAST); DuckDB is date_diff(part, start, end) (part + FIRST). A fixed position leaves the DuckDB spelling's first-argument unit unprotected, so a model + with a `day` dimension rewrites the unit to (${TABLE}.order_day). Whichever END argument is a + date-part keyword is the part; the other columns still resolve. + """ + from sidemantic import Dimension, Model + + model = Model( + name="orders", + table="raw_orders", + primary_key="id", + dimensions=[ + Dimension(name="date", type="time", granularity="day", sql="order_date"), + Dimension(name="day", type="time", granularity="day", sql="order_day"), + Dimension(name="created_at", type="time", granularity="day", sql="created_at"), + Dimension(name="start_at", type="time", granularity="day", sql="started_at"), + Dimension(name="end_at", type="time", granularity="day", sql="ended_at"), + ], + ) + conds = LookMLAdapter._fold_filter_conds + # DuckDB: part FIRST -- `day` must stay a unit, and start/end columns still resolve. + assert conds(["date_diff(day, start_at, end_at) > 1"], model) == ( + "(date_diff(day, (${TABLE}.started_at), (${TABLE}.ended_at)) > 1)" + ) + # BigQuery: part LAST -- unchanged; the `date` COLUMN in the middle still resolves. + assert conds(["DATE_DIFF(created_at, date, day) > 1"], model) == ( + "(DATE_DIFF((${TABLE}.created_at), (${TABLE}.order_date), day) > 1)" + ) + # Both ends look like keywords (a `date` dimension at the far end): the genuine UNIT `day` wins, + # so the DuckDB unit is protected and the `date` column resolves. + assert conds(["date_diff(day, created_at, date) > 1"], model) == ( + "(date_diff(day, (${TABLE}.created_at), (${TABLE}.order_date)) > 1)" + ) + + def test_lookml_export_folded_filter_equal_rank_date_trunc_uses_trunc_unit_as_part(): """When both DATE_TRUNC args share coarseness, the truncation UNIT is the part. @@ -5800,6 +5838,41 @@ def roundtrip(expr): assert block and "(ALL x)" in block and "SUM(ALL" not in block, block +def test_lookml_export_strips_leading_all_modifier_in_stddev_argument(): + """A LEADING ALL on a stddev/variance argument must be stripped, not wrapped as STDDEV(ALL x). + + The stddev/variance export wraps the metric SQL as STDDEV(). When the arg itself carries an + explicit ALL (Metric(agg='stddev', sql='ALL {model}.amount')), emitting STDDEV(ALL amount) is + unparseable, so the re-imported type: number measure is dropped. The ALL is the default modifier + and must be normalized away before wrapping.""" + import re + import tempfile + + from sidemantic import Dimension, Metric, Model + from sidemantic.core.semantic_graph import SemanticGraph + + graph = SemanticGraph() + graph.add_model( + Model( + name="o", + table="t", + primary_key="id", + dimensions=[ + Dimension(name="id", type="numeric", sql="id"), + Dimension(name="amount", type="numeric", sql="amount"), + ], + metrics=[Metric(name="sd", agg="stddev", sql="ALL {model}.amount")], + ) + ) + out = tempfile.mktemp(suffix=".lkml") + LookMLAdapter().export(graph, out) + text = open(out).read() + block = re.search(r"measure: sd \{.*?\n \}", text, re.S) + assert block and "ALL" not in block.group(0), block # STDDEV(${TABLE}.amount), not STDDEV(ALL ...) + reimported = {m.name for m in LookMLAdapter().parse(Path(out)).get_model("o").metrics} + assert "sd" in reimported # survives the round-trip + + def test_lookml_export_sample_aggregate_complete_measures_survive(): """Agg-less complete measures using sample-aggregate aliases must export, not be dropped. From 40bf616329ef95ae4b6c9cd4ac56dd88d3275ace Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Sat, 18 Jul 2026 19:10:30 -0700 Subject: [PATCH 44/44] Fix zero-column complete-SQL CTE and recognize TRUNC date expressions A bare COUNT(*) type:number measure took the complete-SQL path but projected no columns, emitting an invalid SELECT ... FROM CTE; add a constant projection when a complete measure references no columns. Also treat TRUNC/TRUNCATE as the date overload when its value is a date expression (references a time dimension or an explicit date cast), not only when it is a bare time-dimension name. --- sidemantic/adapters/lookml.py | 28 +++++++-- sidemantic/sql/generator.py | 6 ++ tests/adapters/lookml/test_edge_cases.py | 80 ++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 5 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index 833f1282..d9fc0c3a 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -696,6 +696,14 @@ def _enclosing_call_arg_texts(cls, pre: str, suf: str, token: str) -> list[str]: # TRUNC(number, scale). Only guard their part slot with evidence the value argument is date/time. _DATE_PART_NUMERIC_OVERLOAD = frozenset({"trunc", "truncate"}) + # An explicit date/time cast around a value marks the date overload of a numeric-overloaded + # function (a numeric scale value is never cast to a date type). Covers CAST(x AS DATE) and the + # `x::date` shorthand for DATE / DATETIME / TIMESTAMP(TZ) / TIME family types. + _DATE_VALUE_CAST_RE = re.compile( + r"(?i)\bCAST\s*\(.*\bAS\s+(?:DATE|DATETIME|SMALLDATETIME|TIMESTAMP\w*|TIME)\b" + r"|::\s*(?:DATE|DATETIME|SMALLDATETIME|TIMESTAMP\w*|TIME)\b" + ) + @classmethod def _is_date_part_argument(cls, pre: str, suf: str, token: str, time_dim_names: set | None = None) -> bool: """True if ``token`` occupies the date-PART argument slot of a date/time call. @@ -807,14 +815,24 @@ def _is_date_part_argument(cls, pre: str, suf: str, token: str, time_dim_names: return False if func.lower() in cls._DATE_PART_NUMERIC_OVERLOAD: # TRUNC/TRUNCATE is also numeric (TRUNC(number, scale)). Only treat the part slot as a - # date part when the VALUE argument is a known time dimension; otherwise a scale column - # named like a unit (TRUNC(amount, month)) must still resolve to its dimension SQL. + # date part when the VALUE argument is date/time-typed; otherwise a scale column named + # like a unit (TRUNC(amount, month)) must still resolve to its dimension SQL. _args = cls._enclosing_call_arg_texts(pre, suf, token) + _value_raw = _args[0].strip() if _args else "" # The value is normally qualified in a folded filter ({model}.created_at); strip the # model placeholder / a bare table qualifier and any quotes so it matches a time dim name. - _value = _args[0].strip() if _args else "" - _value = re.sub(r"^(?:\{model\}|\$\{TABLE\}|\w+)\.", "", _value).strip("`\"[]'") - if not (time_dim_names and _value in time_dim_names): + _value = re.sub(r"^(?:\{model\}|\$\{TABLE\}|\w+)\.", "", _value_raw).strip("`\"[]'") + _is_date_value = bool(time_dim_names and _value in time_dim_names) + if not _is_date_value: + # A date EXPRESSION over a time dimension -- e.g. TRUNC(CAST({model}.created_at AS + # DATE), month) -- is still the date overload even though the value is not a bare + # dimension name. Recognize it when the value references a known time dimension as a + # whole word, or is wrapped in an explicit date/time cast (never a numeric scale). + _refs_time_dim = bool(time_dim_names) and any( + re.search(rf"(?= 0: return arg_index == want diff --git a/sidemantic/sql/generator.py b/sidemantic/sql/generator.py index a34a1ab1..8511384f 100644 --- a/sidemantic/sql/generator.py +++ b/sidemantic/sql/generator.py @@ -2410,6 +2410,12 @@ def _add_partition(alias: str) -> None: qualify_clause = f"\n QUALIFY {time_col} = {window_expr}" # Build CTE + if not select_cols: + # A complete-SQL measure whose SQL references no columns (e.g. a bare COUNT(*)) + # contributes no projection, so select_cols is empty here. `SELECT FROM t` is a + # syntax error, so emit a constant instead. One constant per source row keeps the + # outer COUNT(*) (and any other row-count aggregate) correct. + select_cols.append(f"1 AS {self._quote_alias('_sidemantic_row')}") select_str = ",\n ".join(select_cols) cte_sql = ( f"{self._quote_identifier(self._cte_name(model_name))} AS " diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index ff6284dc..24ecb92f 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -2970,6 +2970,47 @@ def test_lookml_number_measure_list_aggregate_is_scope_safe(): assert con.execute(layer.compile(metrics=["orders.n_amounts"])).fetchall() == [(3,)] +def test_lookml_number_measure_zero_column_aggregate_compiles(): + """A complete-SQL measure whose SQL references no columns (bare COUNT(*)) must still compile. + + ``has_inline_agg`` marks ``type: number sql: COUNT(*)`` as complete, but the complete-SQL CTE + only projects columns from _complete_sql_columns(), which is empty for COUNT(*). Without a + fallback the CTE becomes ``SELECT FROM orders``, which every engine rejects. The + generator must emit a constant projection so the outer COUNT(*) still counts source rows. + """ + import duckdb + + from sidemantic import SemanticLayer + + graph = _parse_lkml( + """ +view: orders { + sql_table_name: orders ;; + dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } + dimension: status { type: string sql: ${TABLE}.status ;; } + measure: cnt { type: number sql: COUNT(*) ;; } + measure: cnt_const { type: number sql: COUNT(DISTINCT 1) ;; } +} +""" + ) + model = graph.get_model("orders") + assert model.get_metric("cnt") is not None + layer = SemanticLayer(auto_register=False) + layer.add_model(model) + con = duckdb.connect() + con.execute("create table orders(id int, status text)") + con.execute("insert into orders values (1,'a'),(2,'b'),(3,'a')") + # Ungrouped COUNT(*) over the CTE must equal the source row count, not raise a syntax error. + assert con.execute(layer.compile(metrics=["orders.cnt"])).fetchall() == [(3,)] + # Grouped by a real dimension it still counts per group. + assert sorted(con.execute(layer.compile(metrics=["orders.cnt"], dimensions=["orders.status"])).fetchall()) == [ + ("a", 2), + ("b", 1), + ] + # A zero-column DISTINCT constant is also valid. + assert con.execute(layer.compile(metrics=["orders.cnt_const"])).fetchall() == [(1,)] + + def test_lookml_number_measure_constant_dimension_is_aggregate_safe(): """A CONSTANT-valued dimension in a mixed number measure must not read as a raw column. @@ -4888,6 +4929,45 @@ def test_lookml_export_folded_filter_date_part_guard_is_position_aware(): ) +def test_lookml_export_folded_filter_trunc_date_expression_protects_part(): + """A TRUNC over a date EXPRESSION (not a bare dimension) is still the date overload. + + TRUNC is numeric-overloaded (TRUNC(number, scale)), so its part slot is only guarded when the + value is date-typed. The old guard only accepted a value that was EXACTLY a time dimension name, + so TRUNC(CAST({model}.created_at AS DATE), month) on a model that also has a `month` dimension + left `month` looking like a plain column: it got rewritten to the dimension SQL, corrupting the + exported unit. A value that references a time dimension, or is wrapped in an explicit date cast, + must be recognised as the date overload -- while a genuinely numeric TRUNC(amount, month) still + resolves `month` to its column. + """ + from sidemantic import Dimension, Model + + model = Model( + name="orders", + table="raw_orders", + primary_key="id", + dimensions=[ + Dimension(name="created_at", type="time", granularity="day", sql="created_at"), + Dimension(name="month", type="time", granularity="month", sql="order_month"), + Dimension(name="amount", type="numeric", sql="amount"), + ], + ) + conds = LookMLAdapter._fold_filter_conds + # CAST(... AS DATE) value: `month` stays the PART; the cast's `created_at` still resolves. + assert conds(["TRUNC(CAST({model}.created_at AS DATE), month) = DATE '2024-01-01'"], model) == ( + "(TRUNC(CAST((${TABLE}.created_at) AS DATE), month) = DATE '2024-01-01')" + ) + # Postgres `::date` shorthand is recognised the same way. + assert conds(["TRUNC({model}.created_at::date, month) = DATE '2024-01-01'"], model) == ( + "(TRUNC((${TABLE}.created_at)::date, month) = DATE '2024-01-01')" + ) + # A genuinely numeric TRUNC (value is a numeric dimension, no date cast) is untouched: `month` + # is a scale COLUMN and must resolve to its dimension SQL, not be protected as a part. + assert conds(["TRUNC({model}.amount, month) > 5"], model) == ( + "(TRUNC((${TABLE}.amount), (${TABLE}.order_month)) > 5)" + ) + + def test_lookml_export_folded_filter_date_diff_part_position_is_content_based(): """date_diff's part is at EITHER end depending on dialect, so decide by content, not position.