From 589cca7e79bf38693753626e476f4c4fe226467b Mon Sep 17 00:00:00 2001 From: Nico Ritschel Date: Fri, 26 Jun 2026 10:58:43 -0700 Subject: [PATCH] Default LookML view table to the view name when unspecified A LookML view with no sql_table_name and no derived_table implicitly uses a table named after the view (Looker's default behavior), but the adapter left table unset, so importing such a view crashed layer validation with "Model must have a table". Default table to the view name in that case, while leaving refinements, extends-based views, and abstract (extension: required) views untouched. --- sidemantic/adapters/lookml.py | 140 +++++- sidemantic/loaders.py | 92 +++- tests/adapters/lookml/test_edge_cases.py | 455 ++++++++++++++++++ tests/adapters/test_added_fixture_coverage.py | 3 +- .../test_fixture_functionality_contracts.py | 5 +- 5 files changed, 691 insertions(+), 4 deletions(-) diff --git a/sidemantic/adapters/lookml.py b/sidemantic/adapters/lookml.py index fcae0758..976edfd2 100644 --- a/sidemantic/adapters/lookml.py +++ b/sidemantic/adapters/lookml.py @@ -35,7 +35,12 @@ class LookMLAdapter(BaseAdapter): """ def parse(self, source: str | Path) -> SemanticGraph: - """Parse LookML files into semantic graph. + """Parse LookML files into a semantic graph. + + Auto-registration to an active SemanticLayer is suppressed while the graph is + built, so a tableless view that only gets its default table after refinements/ + extends are resolved is not validated mid-parse (which would raise inside a + ``with SemanticLayer():`` block). The finalized models are registered once. Args: source: Path to .lkml file or directory @@ -43,6 +48,40 @@ def parse(self, source: str | Path) -> SemanticGraph: Returns: Semantic graph with imported models """ + from sidemantic.core.registry import get_current_layer, set_current_layer + + prev_layer = get_current_layer() + set_current_layer(None) + try: + graph = self._build_graph(source) + finally: + set_current_layer(prev_layer) + # Defer auto-registration until models are complete (tables defaulted). Skip ONLY + # intentional non-queryable templates -- abstract extension:required bases and + # unsupported derived tables, marked in meta -- which add_model's validation would + # reject; mirrors loaders._is_registerable_model. A merely-broken tableless view + # (e.g. extends:[missing], left unresolved) is NOT skipped, so add_model still + # surfaces the real "no table/sql" error instead of silently dropping it. Strip + # survivors' relationships pointing at a skipped template (e.g. an explore join to + # it) so the active layer is not left with a dangling relationship validation flags. + if prev_layer is not None: + skipped = { + name + for name, m in graph.models.items() + if not (m.table or m.sql or getattr(m, "dax", None) or getattr(m, "source_uri", None)) + and (m.meta or {}).get("lookml_template") + } + for model in graph.models.values(): + if model.name in skipped or model.name in prev_layer.graph.models: + continue + rels = getattr(model, "relationships", None) + if rels: + model.relationships = [r for r in rels if r.name not in skipped] + prev_layer.add_model(model) + return graph + + def _build_graph(self, source: str | Path) -> SemanticGraph: + """Build the semantic graph from LookML files (see :meth:`parse`).""" graph = SemanticGraph() source_path = Path(source) @@ -58,17 +97,50 @@ def parse(self, source: str | Path) -> SemanticGraph: for lkml_file in lkml_files: self._parse_views_from_file(lkml_file, graph, refinements) + # Snapshot abstract / unsupported-derived_table flags BEFORE refinement merge: + # merge_model REPLACES a base view's meta when the refinement carries metadata, + # which would otherwise drop these markers. + abstract_pre = {n for n, m in graph.models.items() if (m.meta or {}).get("extension_required")} + unsupported_pre = {n for n, m in graph.models.items() if (m.meta or {}).get("unsupported_derived_table")} + # Apply refinements: merge each refinement into its base view from sidemantic.core.inheritance import merge_model, resolve_model_inheritance + refinement_abstract: set[str] = set() + refinement_unsupported_dt: set[str] = set() for refinement in refinements: base_name = refinement.name.lstrip("+") + # Record flags from EACH refinement's own meta: a later refinement's merge + # can replace the base meta and drop a flag an earlier refinement added. + rmeta = refinement.meta or {} + if rmeta.get("extension_required"): + refinement_abstract.add(base_name) + if rmeta.get("unsupported_derived_table"): + refinement_unsupported_dt.add(base_name) if base_name in graph.models: # Create a copy with the base name for merging refinement_for_merge = refinement.model_copy(update={"name": base_name}) merged = merge_model(refinement_for_merge, graph.models[base_name]) graph.models[base_name] = merged + # Union the pre-merge snapshot, every refinement's own flags, and the post-merge + # state (so a flag added by ANY refinement is caught even if a later refinement + # replaced the meta). Resolve this BEFORE extends so a concrete child that only + # INHERITS abstractness is not treated as abstract. Record extends parents too, + # so descendants of an unsupported derived table are detectable after resolution + # clears `extends`. + abstract_views = ( + abstract_pre + | refinement_abstract + | {n for n, m in graph.models.items() if (m.meta or {}).get("extension_required")} + ) + unsupported_dt_views = ( + unsupported_pre + | refinement_unsupported_dt + | {n for n, m in graph.models.items() if (m.meta or {}).get("unsupported_derived_table")} + ) + extends_parent = {n: m.extends for n, m in graph.models.items() if m.extends} + # Resolve extends chains. Pre-filter to models whose full chain # is present so one broken/missing parent doesn't block valid ones. def _chain_resolvable(name: str, visited: set[str] | None = None) -> bool: @@ -92,10 +164,68 @@ def _chain_resolvable(name: str, visited: set[str] | None = None) -> bool: resolved.update(unresolvable) graph.models = resolved + def _extends_chain_has(name: str, flagset: set[str]) -> bool: + """True if name or any of its extends-ancestors is in flagset.""" + seen: set[str] = set() + cur: str | None = name + while cur is not None and cur not in seen: + if cur in flagset: + return True + seen.add(cur) + cur = extends_parent.get(cur) + return False + + # Apply the implicit "table = view name" default AFTER refinements and extends + # are resolved, so a view whose fields/name come from a refinement or whose + # parent was tableless still gets its OWN name as the table (Looker's behavior). + # Skip abstract views, still-unresolved-extends, and views that are (or extend) an + # unsupported derived table. Do NOT require parsed fields: Looker defaults the table + # for an ordinary fieldless view (`view: orders {}`, or one with only adapter-ignored + # fields) too, so leaving it tableless would wrongly fail CLI validation/registration. + # Abstractness is NOT inherited through extends (a concrete child of an abstract base + # gets its own table), but an unsupported derived table IS inherited by descendants. + def _apply_default_tables(): + for model_name, model in graph.models.items(): + if ( + model.table is None + and model.sql is None + and not model.extends + and model_name not in abstract_views + and not _extends_chain_has(model_name, unsupported_dt_views) + and not (model.meta or {}).get("unsupported_derived_table") + ): + model.table = model_name + + _apply_default_tables() + # Second pass: parse explores and add relationships for lkml_file in lkml_files: self._parse_explores_from_file(lkml_file, graph) + # Re-apply the default: explores can add segments (sql_always_where / + # always_filter) to an otherwise-fieldless view, which only now makes it + # eligible for the implicit table default. + _apply_default_tables() + + # Re-assert non-queryable markers on models intentionally left tableless. A + # refinement can OVERWRITE a view's meta (dropping an `extension_required` flag an + # earlier refinement added) even though abstractness is tracked in side-sets, so + # without this the loader (which keys off final meta) would try to register and + # reject them. Set the flag definitively now, after all merges. Also stamp a + # PARSER-OWNED `lookml_template` marker so registration skips (here and in the + # loader) key off a sidemantic-internal flag, not the public `extension_required`/ + # `unsupported_derived_table` keys -- a native/other-format model that happens to + # carry those user-facing keys must still surface its missing-source error. + for model_name, model in graph.models.items(): + if model.table is not None or model.sql is not None: + continue + if model_name in abstract_views: + model.meta = {**(model.meta or {}), "extension_required": True, "lookml_template": True} + elif _extends_chain_has(model_name, unsupported_dt_views) or (model.meta or {}).get( + "unsupported_derived_table" + ): + model.meta = {**(model.meta or {}), "unsupported_derived_table": True, "lookml_template": True} + # Rebuild adjacency graph now that relationships have been added graph.build_adjacency() @@ -1103,6 +1233,14 @@ def _folded_measure_filter(m_def): elif isinstance(extends_list, str): extends = extends_list + # A LookML view with no sql_table_name/derived_table implicitly uses a table + # named after the view (Looker's default). A view with a derived_table the + # adapter cannot turn into SQL must NOT get such a default; mark it so the + # post-merge pass skips it (the raw derived_table dict is not retained). + unsupported_derived_table = bool(view_def.get("derived_table")) and sql is None + if unsupported_derived_table: + model_meta["unsupported_derived_table"] = True + # Build kwargs conditionally so that unset scalars don't appear in # model_fields_set. This matters for refinements: merge_model treats # every field in model_fields_set as an explicit child override, so diff --git a/sidemantic/loaders.py b/sidemantic/loaders.py index e17df1f0..326c1dae 100644 --- a/sidemantic/loaders.py +++ b/sidemantic/loaders.py @@ -13,6 +13,88 @@ from sidemantic.core.semantic_layer import SemanticLayer +def _is_registerable_model(model) -> bool: + """True unless ``model`` is an INTENTIONAL non-queryable template that should be + skipped rather than registered/validated. + + Only LookML's ``extension: required`` abstract bases and unsupported derived tables + (kept in the parsed graph as tableless templates) are skipped -- ``add_model`` would + reject them and abort loading an otherwise-valid project. The skip keys off the + PARSER-OWNED ``lookml_template`` marker (set by the LookML adapter), NOT the public + ``extension_required``/``unsupported_derived_table`` keys, so a native or other-format + tableless model that merely carries those user-facing keys still surfaces its real + missing-source validation error. Any OTHER tableless model (e.g. an erroneous Hex view + missing its base) is likewise left in. + """ + if model.table or model.sql or getattr(model, "dax", None) or getattr(model, "source_uri", None): + return True + return not (model.meta or {}).get("lookml_template") + + +def _drop_non_registerable_models(all_models: dict, all_metrics: dict | None = None) -> dict: + """Filter to registerable models AND strip any relationship targeting a dropped one. + + An explore may have already added a relationship pointing at a now-skipped template + (e.g. an `extension: required` join target); leaving it would dangle and fail + ``sidemantic validate``. Only relationships to models THIS function drops are removed + -- a relationship to a never-defined model is a real error that validation must still + surface, so it is left intact. Returns the filtered dict (mutating survivors' rels). + + When ``all_metrics`` is given, also drop graph-level metrics contributed by a dropped + model -- ``SemanticGraph.add_model()`` auto-registers a model's ``time_comparison`` / + ``conversion`` measures into the graph, so a ``period_over_period`` measure on an + ``extension: required`` template would otherwise linger as a graph metric whose base + model is gone, breaking ``compile``/``info``. A metric still defined on a SURVIVING + model is kept. + """ + kept = {name: model for name, model in all_models.items() if _is_registerable_model(model)} + dropped = set(all_models) - set(kept) + for model in kept.values(): + rels = getattr(model, "relationships", None) + if rels: + model.relationships = [r for r in rels if r.name not in dropped] + if all_metrics is not None and dropped: + # add_model auto-registers a model's GRAPH-LEVEL measures (time_comparison/conversion) + # into the graph; a dropped template's such measure would linger in all_metrics with + # its base model gone. Decide orphan-ness by whether the metric's BASE reference still + # resolves to a surviving model -- NOT object identity (a refinement reconstructs the + # Metric object, so the original auto-registered one no longer matches) and NOT bare + # name (a same-named standalone from another file would be wrongly dropped). A metric + # whose base measure lives only on dropped models is the orphan; one pointing at a + # surviving model (e.g. a standalone `${orders.total}`) is kept. + _ref_fields = ("base_metric", "numerator", "denominator", "base_event", "conversion_event") + _surviving_measures = {mm.name for model in kept.values() for mm in (getattr(model, "metrics", None) or [])} + + def _resolves_to_surviving(metric) -> bool: + for f in _ref_fields: + ref = getattr(metric, f, None) + if not isinstance(ref, str) or not ref: + continue + if "." in ref: + if ref.split(".", 1)[0] in kept: # qualified ref to a surviving model + return True + elif ref in _surviving_measures: # unqualified ref a surviving model provides + return True + return False + + # Names of graph-level measures contributed by dropped templates (orphan candidates). + candidates = { + mm.name + for name in dropped + for mm in (getattr(all_models[name], "metrics", None) or []) + if mm.type in ("time_comparison", "conversion") + } + for mn in candidates: + m = all_metrics.get(mn) + if ( + m is not None + and getattr(m, "type", None) in ("time_comparison", "conversion") + and not _resolves_to_surviving(m) + ): + all_metrics.pop(mn, None) + return kept + + def load_from_directory( layer: "SemanticLayer", directory: str | Path, @@ -331,10 +413,16 @@ def load_from_directory( # table pair. _apply_snowflake_pending_relationships(all_models, all_pending_relationships) + # Drop non-queryable template models (LookML `extension: required` abstract bases / + # unsupported derived tables) BEFORE inference + registration, so FK inference never + # targets a model that won't be registered (which would leave a dangling relationship) + # and add_model never rejects a template that was never meant to be queried. + all_models = _drop_non_registerable_models(all_models, all_metrics) + # Infer cross-model relationships based on naming conventions _infer_relationships(all_models) - # Add all models to the layer (now with relationships) + # Add all models to the layer (now with relationships). for model in all_models.values(): if model.name not in layer.graph.models: layer.add_model(model) @@ -420,6 +508,8 @@ def _load_sml_directory(layer: "SemanticLayer", directory: Path, all_models: dic if not hasattr(model, "_source_file"): model._source_file = str(directory) all_models.update(graph.models) + # Drop non-queryable templates (+ their dangling rels) before inference + registration. + all_models = _drop_non_registerable_models(all_models) _infer_relationships(all_models) for model in all_models.values(): if model.name not in layer.graph.models: diff --git a/tests/adapters/lookml/test_edge_cases.py b/tests/adapters/lookml/test_edge_cases.py index d30999af..235a869c 100644 --- a/tests/adapters/lookml/test_edge_cases.py +++ b/tests/adapters/lookml/test_edge_cases.py @@ -2947,6 +2947,461 @@ def test_lookml_self_referential_dimension_terminates(): assert selfd.sql.count("+ 1") <= 2 +def test_lookml_view_with_unsupported_derived_table_not_defaulted(): + """A view with a derived_table the adapter can't read must NOT get a default physical table.""" + adapter = LookMLAdapter() + model = adapter._parse_view( + { + "name": "ndt_unknown", + # derived_table with no sql / explore_source the adapter understands + "derived_table": {"persist_for": "1 hour"}, + "dimensions": [{"name": "id", "type": "number", "sql": "${TABLE}.id"}], + } + ) + assert model.table is None # not fabricated as a physical table named after the view + + +def test_lookml_view_without_table_defaults_to_view_name(): + """A view with no sql_table_name/derived_table should default its table to the view name.""" + from sidemantic import SemanticLayer + + graph = _parse_lkml( + "view: just_fields { " + "dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } " + "measure: c { type: count } }" + ) + model = graph.get_model("just_fields") + assert model.table == "just_fields" + # And it must be a valid, queryable model (previously raised ModelValidationError). + layer = SemanticLayer() + layer.add_model(model) + assert "just_fields" in layer.compile(metrics=["just_fields.c"]) + + +def test_lookml_parse_tableless_view_inside_layer_context(): + """Parsing a tableless view inside a `with SemanticLayer()` block must not crash. + + Auto-registration fires during Model construction; the default table is applied + after parse, so parsing must suppress registration until models are complete. + """ + import tempfile + + from sidemantic import SemanticLayer + + with tempfile.NamedTemporaryFile(mode="w", suffix=".lkml", delete=False) as f: + f.write( + "view: just_fields { dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } measure: c { type: count } }" + ) + f.flush() + path = Path(f.name) + + with SemanticLayer() as layer: + LookMLAdapter().parse(path) + # The model is registered to the context layer with its defaulted table. + assert layer.graph.get_model("just_fields").table == "just_fields" + + +def test_lookml_fieldless_ordinary_view_gets_default_table(): + """An ordinary fieldless view (or one with only adapter-ignored fields) still defaults. + + Looker defaults the table name to the view name regardless of parsed fields, so a + `view: orders {}` must not be left tableless (which would fail CLI load/registration). + Abstract/unsupported templates are still skipped; this only covers ordinary views. + """ + import tempfile + + from sidemantic import SemanticLayer + from sidemantic.loaders import load_from_directory + + d = tempfile.mkdtemp() + with open(Path(d) / "v.lkml", "w") as f: + f.write("view: orders {}\nview: just_set { set: foo { fields: [a, b] } }\n") + + layer = SemanticLayer() + load_from_directory(layer, d) # must not raise the no-table ModelValidationError + assert layer.graph.get_model("orders").table == "orders" + assert layer.graph.get_model("just_set").table == "just_set" + + +def test_lookml_abstract_base_not_registered_inside_layer_context(): + """An abstract (extension: required) base must NOT be registered inside a `with` layer. + + It's intentionally tableless; deferred registration must skip it (not raise) while the + concrete child still registers with its defaulted table. + """ + import tempfile + + from sidemantic import SemanticLayer + + with tempfile.NamedTemporaryFile(mode="w", suffix=".lkml", delete=False) as f: + f.write( + "view: base { extension: required dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } " + "dimension: x { type: number sql: ${TABLE}.x ;; } } " + "view: orders { extends: [base] dimension: y { type: number sql: ${TABLE}.y ;; } }" + ) + f.flush() + path = Path(f.name) + + with SemanticLayer() as layer: + LookMLAdapter().parse(path) # must not raise ModelValidationError + assert "base" not in layer.graph.models # abstract base skipped + assert layer.graph.get_model("orders").table == "orders" # concrete child registered + + +def test_lookml_broken_tableless_view_surfaces_error_inside_layer_context(): + """A genuinely-broken tableless view (unresolved extends) must NOT be silently skipped. + + Deferred registration skips only INTENTIONAL templates (extension:required / unsupported + derived tables). A `view: child { extends: [missing] }` stays tableless because its parent + can't resolve -- that's a real error, so add_model must still raise rather than drop it. + """ + import tempfile + + import pytest + + from sidemantic import SemanticLayer + from sidemantic.validation import ModelValidationError + + with tempfile.NamedTemporaryFile(mode="w", suffix=".lkml", delete=False) as f: + f.write( + "view: child { extends: [missing] dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } }" + ) + f.flush() + path = Path(f.name) + + with pytest.raises(ModelValidationError): + with SemanticLayer(): + LookMLAdapter().parse(path) + + +def test_lookml_abstract_base_does_not_break_directory_load(): + """The CLI path (load_from_directory) must skip the abstract base, not abort the project.""" + import tempfile + + from sidemantic import SemanticLayer + from sidemantic.loaders import load_from_directory + + d = tempfile.mkdtemp() + with open(Path(d) / "v.lkml", "w") as f: + f.write( + "view: base { extension: required dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } " + "dimension: x { type: number sql: ${TABLE}.x ;; } } " + "view: orders { extends: [base] dimension: y { type: number sql: ${TABLE}.y ;; } }" + ) + + layer = SemanticLayer() + load_from_directory(layer, d) # must not raise ModelValidationError on `base` + assert "base" not in layer.graph.models # non-queryable abstract base skipped + assert layer.graph.get_model("orders").table == "orders" + + +def test_lookml_drop_metrics_uses_base_resolution_not_identity_or_name(): + """Orphan-metric drop keys off whether the BASE resolves to a surviving model. + + NOT object identity (a refinement reconstructs the Metric object) and NOT bare name (a + same-named standalone from another file must survive). The orphan is the one whose base + measure lives only on dropped models. + """ + from sidemantic import Metric, Model + from sidemantic.loaders import _drop_non_registerable_models + + def pop(bm): + return Metric( + name="pop", type="time_comparison", base_metric=bm, comparison_type="yoy", calculation="difference" + ) + + template = Model(name="base", meta={"lookml_template": True}, metrics=[pop("total")]) + orders = Model(name="orders", table="orders") + + # A standalone `pop` over a SURVIVING model's measure (qualified ref) must be preserved. + standalone = pop("orders.total") + metrics = {"pop": standalone} + _drop_non_registerable_models({"base": template, "orders": orders}, metrics) + assert metrics.get("pop") is standalone + + # A refinement reconstructs the template's `pop` (DIFFERENT object than template.metrics[0]), + # base `total` resolves to no surviving model -> orphan, must be dropped (identity would miss it). + metrics2 = {"pop": pop("total")} + _drop_non_registerable_models({"base": template, "orders": orders}, metrics2) + assert "pop" not in metrics2 + + +def test_lookml_dropped_template_metric_dropped_despite_samename_local_measure(): + """A dropped template's graph metric is removed even if a surviving model has a same-named measure. + + The orphan check matches graph-level metric types (time_comparison/conversion), not bare + names: a surviving `orders.pop` SIMPLE measure must not keep the template's `pop` + period_over_period alive (which would expose a metric whose base model is gone). + """ + import tempfile + + from sidemantic import SemanticLayer + from sidemantic.loaders import load_from_directory + + d = tempfile.mkdtemp() + with open(Path(d) / "v.lkml", "w") as f: + f.write( + "view: base { extension: required " + " dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } " + " dimension: amt { type: number sql: ${TABLE}.amt ;; } " + " measure: total { type: sum sql: ${amt} ;; } " + " measure: pop { type: period_over_period based_on: total period: year kind: difference } } " + "view: orders { sql_table_name: orders ;; " + " dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } " + " measure: pop { type: count } }" + ) + + layer = SemanticLayer() + load_from_directory(layer, d) + assert "base" not in layer.graph.models + assert "pop" not in layer.graph.metrics # template graph metric dropped despite orders.pop + assert layer.graph.get_model("orders").get_metric("pop") is not None # local measure survives + + +def test_lookml_dropped_template_graph_metric_not_orphaned(): + """A graph metric (period_over_period) on a dropped template must not linger after CLI load. + + add_model auto-registers time_comparison/conversion measures as graph metrics; when the + only model carrying it is a skipped extension:required template, the metric must be + dropped too, else compile/info expose a metric whose base model is missing. + """ + import tempfile + + from sidemantic import SemanticLayer + from sidemantic.loaders import load_from_directory + + d = tempfile.mkdtemp() + with open(Path(d) / "v.lkml", "w") as f: + f.write( + "view: base { extension: required " + " dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } " + " dimension: amt { type: number sql: ${TABLE}.amt ;; } " + " measure: total { type: sum sql: ${amt} ;; } " + " measure: pop { type: period_over_period based_on: total period: year kind: difference } }" + ) + + layer = SemanticLayer() + load_from_directory(layer, d) # must not raise + assert "base" not in layer.graph.models # template skipped + assert "pop" not in layer.graph.metrics # orphaned graph metric dropped too + + +def test_lookml_registerability_keys_off_parser_marker_not_user_meta(): + """A tableless model with user `extension_required` meta but no parser marker must surface error. + + The skip keys off the parser-owned `lookml_template` marker so a native/other-format + model that merely carries the public `extension_required` key is still registered (and + its missing-source error surfaced), not silently dropped. + """ + from types import SimpleNamespace + + from sidemantic.loaders import _is_registerable_model + + user_meta = SimpleNamespace(table=None, sql=None, dax=None, source_uri=None, meta={"extension_required": True}) + assert _is_registerable_model(user_meta) is True # surfaces error, not dropped + template = SimpleNamespace( + table=None, sql=None, dax=None, source_uri=None, meta={"extension_required": True, "lookml_template": True} + ) + assert _is_registerable_model(template) is False # genuine LookML template skipped + + +def test_lookml_abstract_marker_survives_later_refinement_meta_overwrite(): + """A refinement overwriting meta must not strip the abstract marker the loader keys off. + + `+base { extension: required }` then `+base { label: ... }` leaves base tableless but + its final meta only has the label; the marker is re-asserted so the CLI loader still + skips it instead of raising the no-table validation error. + """ + import tempfile + + from sidemantic import SemanticLayer + from sidemantic.loaders import load_from_directory + + d = tempfile.mkdtemp() + with open(Path(d) / "v.lkml", "w") as f: + f.write( + "view: base { dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } " + "dimension: x { type: number sql: ${TABLE}.x ;; } } " + "view: +base { extension: required } " + 'view: +base { label: "Base" } ' + "view: orders { extends: [base] dimension: y { type: number sql: ${TABLE}.y ;; } }" + ) + + layer = SemanticLayer() + load_from_directory(layer, d) # must not raise even though `extension_required` was overwritten + assert "base" not in layer.graph.models + assert layer.graph.get_model("orders").table == "orders" + + +def test_lookml_fk_inference_skips_abstract_template_no_dangling_rel(): + """FK inference must not target a skipped abstract template, leaving a dangling relationship.""" + import tempfile + + from sidemantic import SemanticLayer + from sidemantic.loaders import load_from_directory + + d = tempfile.mkdtemp() + with open(Path(d) / "v.lkml", "w") as f: + f.write( + "view: customer { extension: required dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } } " + "view: customers { extends: [customer] sql_table_name: customers ;; " + "dimension: name { type: string sql: ${TABLE}.name ;; } } " + "view: orders { sql_table_name: orders ;; dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } " + "dimension: customer_id { type: number sql: ${TABLE}.customer_id ;; } }" + ) + + layer = SemanticLayer() + load_from_directory(layer, d) + assert "customer" not in layer.graph.models # abstract template not registered + # No model carries a relationship pointing at the skipped template. + for model in layer.graph.models.values(): + for rel in getattr(model, "relationships", []) or []: + assert rel.name in layer.graph.models, f"dangling relationship to {rel.name}" + + +def test_lookml_explore_join_to_template_no_dangling_rel(): + """An explore join to a skipped template must not leave a dangling relationship.""" + import tempfile + + from sidemantic import SemanticLayer + from sidemantic.loaders import load_from_directory + + d = tempfile.mkdtemp() + with open(Path(d) / "v.lkml", "w") as f: + f.write( + "view: customer { extension: required dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } } " + "view: orders { sql_table_name: orders ;; dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } " + "dimension: customer_id { type: number sql: ${TABLE}.customer_id ;; } } " + "explore: orders { join: customer { sql_on: ${orders.customer_id} = ${customer.id} ;; relationship: many_to_one } }" + ) + + layer = SemanticLayer() + load_from_directory(layer, d) + assert "customer" not in layer.graph.models + for model in layer.graph.models.values(): + for rel in getattr(model, "relationships", []) or []: + assert rel.name in layer.graph.models, f"dangling relationship to {rel.name}" + + +def test_lookml_active_layer_explore_join_to_template_no_dangling_rel(): + """Same as above but via the ACTIVE-layer parse() path (with SemanticLayer()).""" + import tempfile + + from sidemantic import SemanticLayer + + with tempfile.NamedTemporaryFile(mode="w", suffix=".lkml", delete=False) as f: + f.write( + "view: customer { extension: required dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } } " + "view: orders { sql_table_name: orders ;; dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } " + "dimension: customer_id { type: number sql: ${TABLE}.customer_id ;; } } " + "explore: orders { join: customer { sql_on: ${orders.customer_id} = ${customer.id} ;; relationship: many_to_one } }" + ) + f.flush() + path = Path(f.name) + + with SemanticLayer() as layer: + LookMLAdapter().parse(path) + assert "customer" not in layer.graph.models + for model in layer.graph.models.values(): + for rel in getattr(model, "relationships", []) or []: + assert rel.name in layer.graph.models, f"dangling relationship to {rel.name}" + + +def test_lookml_extends_tableless_view_keeps_own_table(): + """A child extending a tableless base must default to its OWN name, not inherit the base's default.""" + graph = _parse_lkml( + "view: base { dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } " + "dimension: x { type: number sql: ${TABLE}.x ;; } } " + "view: child { extends: [base] dimension: y { type: number sql: ${TABLE}.y ;; } }" + ) + assert graph.get_model("base").table == "base" + assert graph.get_model("child").table == "child" + + +def test_lookml_refinement_adds_fields_then_defaults_table(): + """A fieldless base whose fields come from a +refinement still defaults its table after merge.""" + graph = _parse_lkml( + "view: orders {} view: +orders { dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } }" + ) + assert graph.get_model("orders").table == "orders" + + +def test_lookml_filter_only_view_gets_default_table(): + """A view that declares only LookML `filter` fields (-> segments) still defaults its table. + + Such a view is not 'fieldless' -- without the default it fails validation (no table/sql). + """ + graph = _parse_lkml("view: ff { filter: recent { type: yesno sql: ${TABLE}.x ;; } }") + model = graph.get_model("ff") + assert model.table == "ff" + assert [s.name for s in model.segments] == ["recent"] + + +def test_lookml_concrete_child_of_abstract_view_gets_table(): + """A concrete view extending an extension:required base must default to its OWN name, not be treated as abstract.""" + graph = _parse_lkml( + "view: base { extension: required " + "dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } " + "dimension: x { type: number sql: ${TABLE}.x ;; } } " + "view: orders { extends: [base] dimension: y { type: number sql: ${TABLE}.y ;; } }" + ) + assert graph.get_model("base").table is None # abstract base stays table-less + assert graph.get_model("orders").table == "orders" # concrete child defaults to its name + + +def test_lookml_unsupported_derived_table_marker_survives_refinement(): + """The unsupported-derived_table marker must survive a refinement that carries meta (e.g. label).""" + graph = _parse_lkml( + "view: ndt { derived_table: { sql_trigger_value: SELECT CURRENT_DATE() ;; } " + "dimension: id { type: number sql: ${TABLE}.id ;; } } " + 'view: +ndt { label: "NDT View" }' + ) + ndt = graph.get_model("ndt") + # Not defaulted to a physical table even though the refinement replaced its meta. + assert ndt.table is None + assert ndt.sql is None + + +def test_lookml_child_inheriting_unsupported_derived_table_not_defaulted(): + """A child extending a parent with an unsupported derived_table must stay table-less too.""" + graph = _parse_lkml( + "view: base_ndt { derived_table: { sql_trigger_value: SELECT 1 ;; } " + "dimension: id { type: number sql: ${TABLE}.id ;; } } " + "view: child_ndt { extends: [base_ndt] }" + ) + # Inherited the unsupported-derived_table marker via extends -> not defaulted. + assert graph.get_model("child_ndt").table is None + + +def test_lookml_refinement_added_extension_required_stays_abstract(): + """A +refinement that adds extension: required must keep the refined view tableless.""" + graph = _parse_lkml( + "view: base { dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } } " + "view: +base { extension: required }" + ) + assert graph.get_model("base").table is None + + +def test_lookml_abstract_flag_survives_later_refinement(): + """extension: required added by one refinement survives a later refinement that replaces meta.""" + graph = _parse_lkml( + "view: base { dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; } } " + "view: +base { extension: required } " + 'view: +base { label: "Base" }' + ) + assert graph.get_model("base").table is None + + +def test_lookml_child_of_unsupported_dt_with_own_meta_not_defaulted(): + """A child extending an unsupported derived_table base must stay tableless even with its own meta.""" + graph = _parse_lkml( + "view: base { derived_table: { sql_trigger_value: SELECT 1 ;; } " + "dimension: id { type: number sql: ${TABLE}.id ;; } } " + 'view: child { extends: [base] label: "Child" }' + ) + assert graph.get_model("child").table is None + + def test_lookml_export_unmapped_aggregations_not_count(): """Unmapped aggregations / complex types must not be silently exported as COUNT.""" import tempfile diff --git a/tests/adapters/test_added_fixture_coverage.py b/tests/adapters/test_added_fixture_coverage.py index 8fb423ab..ea08d3d5 100644 --- a/tests/adapters/test_added_fixture_coverage.py +++ b/tests/adapters/test_added_fixture_coverage.py @@ -227,7 +227,8 @@ "tests/fixtures/lookml/ga360_ga_block.view.lkml", "tests/fixtures/lookml/lkml_model_all_fields.model.lkml", "tests/fixtures/lookml/lkml_parameter_join.model.lkml", - "tests/fixtures/lookml/node_lookml_refinement_merging.model.lkml", + # node_lookml_refinement_merging now compiles: its `deep_merging` view has a + # dimension and (per Looker semantics) defaults its table to the view name. "tests/fixtures/lookml/node_lookml_refinement_sequencing.model.lkml", "tests/fixtures/lookml/pylookml_aggregate_tables.model.lkml", "tests/fixtures/lookml/pylookml_manifest.lkml", diff --git a/tests/adapters/test_fixture_functionality_contracts.py b/tests/adapters/test_fixture_functionality_contracts.py index 60e20953..71613933 100644 --- a/tests/adapters/test_fixture_functionality_contracts.py +++ b/tests/adapters/test_fixture_functionality_contracts.py @@ -147,7 +147,10 @@ "RillAdapter", "ThoughtSpotAdapter", }, - "source_fragments_without_fields": {"AtScaleSMLAdapter", "MalloyAdapter", "RillAdapter"}, + # LookML: a fieldless view now gets its implicit "table = view name" default (Looker + # behavior), so a refinement-fragment fixture with field-less views is a source fragment + # without fields rather than a tableless semantic-only model. + "source_fragments_without_fields": {"AtScaleSMLAdapter", "LookMLAdapter", "MalloyAdapter", "RillAdapter"}, "semantic_only_no_sources": { "AtScaleSMLAdapter", "HolisticsAdapter",