Skip to content

Commit 589cca7

Browse files
committed
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.
1 parent 6416688 commit 589cca7

5 files changed

Lines changed: 691 additions & 4 deletions

File tree

sidemantic/adapters/lookml.py

Lines changed: 139 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,53 @@ class LookMLAdapter(BaseAdapter):
3535
"""
3636

3737
def parse(self, source: str | Path) -> SemanticGraph:
38-
"""Parse LookML files into semantic graph.
38+
"""Parse LookML files into a semantic graph.
39+
40+
Auto-registration to an active SemanticLayer is suppressed while the graph is
41+
built, so a tableless view that only gets its default table after refinements/
42+
extends are resolved is not validated mid-parse (which would raise inside a
43+
``with SemanticLayer():`` block). The finalized models are registered once.
3944
4045
Args:
4146
source: Path to .lkml file or directory
4247
4348
Returns:
4449
Semantic graph with imported models
4550
"""
51+
from sidemantic.core.registry import get_current_layer, set_current_layer
52+
53+
prev_layer = get_current_layer()
54+
set_current_layer(None)
55+
try:
56+
graph = self._build_graph(source)
57+
finally:
58+
set_current_layer(prev_layer)
59+
# Defer auto-registration until models are complete (tables defaulted). Skip ONLY
60+
# intentional non-queryable templates -- abstract extension:required bases and
61+
# unsupported derived tables, marked in meta -- which add_model's validation would
62+
# reject; mirrors loaders._is_registerable_model. A merely-broken tableless view
63+
# (e.g. extends:[missing], left unresolved) is NOT skipped, so add_model still
64+
# surfaces the real "no table/sql" error instead of silently dropping it. Strip
65+
# survivors' relationships pointing at a skipped template (e.g. an explore join to
66+
# it) so the active layer is not left with a dangling relationship validation flags.
67+
if prev_layer is not None:
68+
skipped = {
69+
name
70+
for name, m in graph.models.items()
71+
if not (m.table or m.sql or getattr(m, "dax", None) or getattr(m, "source_uri", None))
72+
and (m.meta or {}).get("lookml_template")
73+
}
74+
for model in graph.models.values():
75+
if model.name in skipped or model.name in prev_layer.graph.models:
76+
continue
77+
rels = getattr(model, "relationships", None)
78+
if rels:
79+
model.relationships = [r for r in rels if r.name not in skipped]
80+
prev_layer.add_model(model)
81+
return graph
82+
83+
def _build_graph(self, source: str | Path) -> SemanticGraph:
84+
"""Build the semantic graph from LookML files (see :meth:`parse`)."""
4685
graph = SemanticGraph()
4786
source_path = Path(source)
4887

@@ -58,17 +97,50 @@ def parse(self, source: str | Path) -> SemanticGraph:
5897
for lkml_file in lkml_files:
5998
self._parse_views_from_file(lkml_file, graph, refinements)
6099

100+
# Snapshot abstract / unsupported-derived_table flags BEFORE refinement merge:
101+
# merge_model REPLACES a base view's meta when the refinement carries metadata,
102+
# which would otherwise drop these markers.
103+
abstract_pre = {n for n, m in graph.models.items() if (m.meta or {}).get("extension_required")}
104+
unsupported_pre = {n for n, m in graph.models.items() if (m.meta or {}).get("unsupported_derived_table")}
105+
61106
# Apply refinements: merge each refinement into its base view
62107
from sidemantic.core.inheritance import merge_model, resolve_model_inheritance
63108

109+
refinement_abstract: set[str] = set()
110+
refinement_unsupported_dt: set[str] = set()
64111
for refinement in refinements:
65112
base_name = refinement.name.lstrip("+")
113+
# Record flags from EACH refinement's own meta: a later refinement's merge
114+
# can replace the base meta and drop a flag an earlier refinement added.
115+
rmeta = refinement.meta or {}
116+
if rmeta.get("extension_required"):
117+
refinement_abstract.add(base_name)
118+
if rmeta.get("unsupported_derived_table"):
119+
refinement_unsupported_dt.add(base_name)
66120
if base_name in graph.models:
67121
# Create a copy with the base name for merging
68122
refinement_for_merge = refinement.model_copy(update={"name": base_name})
69123
merged = merge_model(refinement_for_merge, graph.models[base_name])
70124
graph.models[base_name] = merged
71125

126+
# Union the pre-merge snapshot, every refinement's own flags, and the post-merge
127+
# state (so a flag added by ANY refinement is caught even if a later refinement
128+
# replaced the meta). Resolve this BEFORE extends so a concrete child that only
129+
# INHERITS abstractness is not treated as abstract. Record extends parents too,
130+
# so descendants of an unsupported derived table are detectable after resolution
131+
# clears `extends`.
132+
abstract_views = (
133+
abstract_pre
134+
| refinement_abstract
135+
| {n for n, m in graph.models.items() if (m.meta or {}).get("extension_required")}
136+
)
137+
unsupported_dt_views = (
138+
unsupported_pre
139+
| refinement_unsupported_dt
140+
| {n for n, m in graph.models.items() if (m.meta or {}).get("unsupported_derived_table")}
141+
)
142+
extends_parent = {n: m.extends for n, m in graph.models.items() if m.extends}
143+
72144
# Resolve extends chains. Pre-filter to models whose full chain
73145
# is present so one broken/missing parent doesn't block valid ones.
74146
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:
92164
resolved.update(unresolvable)
93165
graph.models = resolved
94166

167+
def _extends_chain_has(name: str, flagset: set[str]) -> bool:
168+
"""True if name or any of its extends-ancestors is in flagset."""
169+
seen: set[str] = set()
170+
cur: str | None = name
171+
while cur is not None and cur not in seen:
172+
if cur in flagset:
173+
return True
174+
seen.add(cur)
175+
cur = extends_parent.get(cur)
176+
return False
177+
178+
# Apply the implicit "table = view name" default AFTER refinements and extends
179+
# are resolved, so a view whose fields/name come from a refinement or whose
180+
# parent was tableless still gets its OWN name as the table (Looker's behavior).
181+
# Skip abstract views, still-unresolved-extends, and views that are (or extend) an
182+
# unsupported derived table. Do NOT require parsed fields: Looker defaults the table
183+
# for an ordinary fieldless view (`view: orders {}`, or one with only adapter-ignored
184+
# fields) too, so leaving it tableless would wrongly fail CLI validation/registration.
185+
# Abstractness is NOT inherited through extends (a concrete child of an abstract base
186+
# gets its own table), but an unsupported derived table IS inherited by descendants.
187+
def _apply_default_tables():
188+
for model_name, model in graph.models.items():
189+
if (
190+
model.table is None
191+
and model.sql is None
192+
and not model.extends
193+
and model_name not in abstract_views
194+
and not _extends_chain_has(model_name, unsupported_dt_views)
195+
and not (model.meta or {}).get("unsupported_derived_table")
196+
):
197+
model.table = model_name
198+
199+
_apply_default_tables()
200+
95201
# Second pass: parse explores and add relationships
96202
for lkml_file in lkml_files:
97203
self._parse_explores_from_file(lkml_file, graph)
98204

205+
# Re-apply the default: explores can add segments (sql_always_where /
206+
# always_filter) to an otherwise-fieldless view, which only now makes it
207+
# eligible for the implicit table default.
208+
_apply_default_tables()
209+
210+
# Re-assert non-queryable markers on models intentionally left tableless. A
211+
# refinement can OVERWRITE a view's meta (dropping an `extension_required` flag an
212+
# earlier refinement added) even though abstractness is tracked in side-sets, so
213+
# without this the loader (which keys off final meta) would try to register and
214+
# reject them. Set the flag definitively now, after all merges. Also stamp a
215+
# PARSER-OWNED `lookml_template` marker so registration skips (here and in the
216+
# loader) key off a sidemantic-internal flag, not the public `extension_required`/
217+
# `unsupported_derived_table` keys -- a native/other-format model that happens to
218+
# carry those user-facing keys must still surface its missing-source error.
219+
for model_name, model in graph.models.items():
220+
if model.table is not None or model.sql is not None:
221+
continue
222+
if model_name in abstract_views:
223+
model.meta = {**(model.meta or {}), "extension_required": True, "lookml_template": True}
224+
elif _extends_chain_has(model_name, unsupported_dt_views) or (model.meta or {}).get(
225+
"unsupported_derived_table"
226+
):
227+
model.meta = {**(model.meta or {}), "unsupported_derived_table": True, "lookml_template": True}
228+
99229
# Rebuild adjacency graph now that relationships have been added
100230
graph.build_adjacency()
101231

@@ -1103,6 +1233,14 @@ def _folded_measure_filter(m_def):
11031233
elif isinstance(extends_list, str):
11041234
extends = extends_list
11051235

1236+
# A LookML view with no sql_table_name/derived_table implicitly uses a table
1237+
# named after the view (Looker's default). A view with a derived_table the
1238+
# adapter cannot turn into SQL must NOT get such a default; mark it so the
1239+
# post-merge pass skips it (the raw derived_table dict is not retained).
1240+
unsupported_derived_table = bool(view_def.get("derived_table")) and sql is None
1241+
if unsupported_derived_table:
1242+
model_meta["unsupported_derived_table"] = True
1243+
11061244
# Build kwargs conditionally so that unset scalars don't appear in
11071245
# model_fields_set. This matters for refinements: merge_model treats
11081246
# every field in model_fields_set as an explicit child override, so

sidemantic/loaders.py

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,88 @@
1313
from sidemantic.core.semantic_layer import SemanticLayer
1414

1515

16+
def _is_registerable_model(model) -> bool:
17+
"""True unless ``model`` is an INTENTIONAL non-queryable template that should be
18+
skipped rather than registered/validated.
19+
20+
Only LookML's ``extension: required`` abstract bases and unsupported derived tables
21+
(kept in the parsed graph as tableless templates) are skipped -- ``add_model`` would
22+
reject them and abort loading an otherwise-valid project. The skip keys off the
23+
PARSER-OWNED ``lookml_template`` marker (set by the LookML adapter), NOT the public
24+
``extension_required``/``unsupported_derived_table`` keys, so a native or other-format
25+
tableless model that merely carries those user-facing keys still surfaces its real
26+
missing-source validation error. Any OTHER tableless model (e.g. an erroneous Hex view
27+
missing its base) is likewise left in.
28+
"""
29+
if model.table or model.sql or getattr(model, "dax", None) or getattr(model, "source_uri", None):
30+
return True
31+
return not (model.meta or {}).get("lookml_template")
32+
33+
34+
def _drop_non_registerable_models(all_models: dict, all_metrics: dict | None = None) -> dict:
35+
"""Filter to registerable models AND strip any relationship targeting a dropped one.
36+
37+
An explore may have already added a relationship pointing at a now-skipped template
38+
(e.g. an `extension: required` join target); leaving it would dangle and fail
39+
``sidemantic validate``. Only relationships to models THIS function drops are removed
40+
-- a relationship to a never-defined model is a real error that validation must still
41+
surface, so it is left intact. Returns the filtered dict (mutating survivors' rels).
42+
43+
When ``all_metrics`` is given, also drop graph-level metrics contributed by a dropped
44+
model -- ``SemanticGraph.add_model()`` auto-registers a model's ``time_comparison`` /
45+
``conversion`` measures into the graph, so a ``period_over_period`` measure on an
46+
``extension: required`` template would otherwise linger as a graph metric whose base
47+
model is gone, breaking ``compile``/``info``. A metric still defined on a SURVIVING
48+
model is kept.
49+
"""
50+
kept = {name: model for name, model in all_models.items() if _is_registerable_model(model)}
51+
dropped = set(all_models) - set(kept)
52+
for model in kept.values():
53+
rels = getattr(model, "relationships", None)
54+
if rels:
55+
model.relationships = [r for r in rels if r.name not in dropped]
56+
if all_metrics is not None and dropped:
57+
# add_model auto-registers a model's GRAPH-LEVEL measures (time_comparison/conversion)
58+
# into the graph; a dropped template's such measure would linger in all_metrics with
59+
# its base model gone. Decide orphan-ness by whether the metric's BASE reference still
60+
# resolves to a surviving model -- NOT object identity (a refinement reconstructs the
61+
# Metric object, so the original auto-registered one no longer matches) and NOT bare
62+
# name (a same-named standalone from another file would be wrongly dropped). A metric
63+
# whose base measure lives only on dropped models is the orphan; one pointing at a
64+
# surviving model (e.g. a standalone `${orders.total}`) is kept.
65+
_ref_fields = ("base_metric", "numerator", "denominator", "base_event", "conversion_event")
66+
_surviving_measures = {mm.name for model in kept.values() for mm in (getattr(model, "metrics", None) or [])}
67+
68+
def _resolves_to_surviving(metric) -> bool:
69+
for f in _ref_fields:
70+
ref = getattr(metric, f, None)
71+
if not isinstance(ref, str) or not ref:
72+
continue
73+
if "." in ref:
74+
if ref.split(".", 1)[0] in kept: # qualified ref to a surviving model
75+
return True
76+
elif ref in _surviving_measures: # unqualified ref a surviving model provides
77+
return True
78+
return False
79+
80+
# Names of graph-level measures contributed by dropped templates (orphan candidates).
81+
candidates = {
82+
mm.name
83+
for name in dropped
84+
for mm in (getattr(all_models[name], "metrics", None) or [])
85+
if mm.type in ("time_comparison", "conversion")
86+
}
87+
for mn in candidates:
88+
m = all_metrics.get(mn)
89+
if (
90+
m is not None
91+
and getattr(m, "type", None) in ("time_comparison", "conversion")
92+
and not _resolves_to_surviving(m)
93+
):
94+
all_metrics.pop(mn, None)
95+
return kept
96+
97+
1698
def load_from_directory(
1799
layer: "SemanticLayer",
18100
directory: str | Path,
@@ -331,10 +413,16 @@ def load_from_directory(
331413
# table pair.
332414
_apply_snowflake_pending_relationships(all_models, all_pending_relationships)
333415

416+
# Drop non-queryable template models (LookML `extension: required` abstract bases /
417+
# unsupported derived tables) BEFORE inference + registration, so FK inference never
418+
# targets a model that won't be registered (which would leave a dangling relationship)
419+
# and add_model never rejects a template that was never meant to be queried.
420+
all_models = _drop_non_registerable_models(all_models, all_metrics)
421+
334422
# Infer cross-model relationships based on naming conventions
335423
_infer_relationships(all_models)
336424

337-
# Add all models to the layer (now with relationships)
425+
# Add all models to the layer (now with relationships).
338426
for model in all_models.values():
339427
if model.name not in layer.graph.models:
340428
layer.add_model(model)
@@ -420,6 +508,8 @@ def _load_sml_directory(layer: "SemanticLayer", directory: Path, all_models: dic
420508
if not hasattr(model, "_source_file"):
421509
model._source_file = str(directory)
422510
all_models.update(graph.models)
511+
# Drop non-queryable templates (+ their dangling rels) before inference + registration.
512+
all_models = _drop_non_registerable_models(all_models)
423513
_infer_relationships(all_models)
424514
for model in all_models.values():
425515
if model.name not in layer.graph.models:

0 commit comments

Comments
 (0)