Skip to content

Commit 417f230

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 83a40fa commit 417f230

4 files changed

Lines changed: 438 additions & 3 deletions

File tree

sidemantic/adapters/lookml.py

Lines changed: 120 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,39 @@ 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
60+
# non-queryable models (abstract extension:required bases, unsupported derived
61+
# tables) -- they intentionally have no table/sql, so add_model's validation
62+
# would reject them; they are templates, not registerable models.
63+
if prev_layer is not None:
64+
for model in graph.models.values():
65+
if model.name not in prev_layer.graph.models and (model.table or model.sql):
66+
prev_layer.add_model(model)
67+
return graph
68+
69+
def _build_graph(self, source: str | Path) -> SemanticGraph:
70+
"""Build the semantic graph from LookML files (see :meth:`parse`)."""
4671
graph = SemanticGraph()
4772
source_path = Path(source)
4873

@@ -58,17 +83,50 @@ def parse(self, source: str | Path) -> SemanticGraph:
5883
for lkml_file in lkml_files:
5984
self._parse_views_from_file(lkml_file, graph, refinements)
6085

86+
# Snapshot abstract / unsupported-derived_table flags BEFORE refinement merge:
87+
# merge_model REPLACES a base view's meta when the refinement carries metadata,
88+
# which would otherwise drop these markers.
89+
abstract_pre = {n for n, m in graph.models.items() if (m.meta or {}).get("extension_required")}
90+
unsupported_pre = {n for n, m in graph.models.items() if (m.meta or {}).get("unsupported_derived_table")}
91+
6192
# Apply refinements: merge each refinement into its base view
6293
from sidemantic.core.inheritance import merge_model, resolve_model_inheritance
6394

95+
refinement_abstract: set[str] = set()
96+
refinement_unsupported_dt: set[str] = set()
6497
for refinement in refinements:
6598
base_name = refinement.name.lstrip("+")
99+
# Record flags from EACH refinement's own meta: a later refinement's merge
100+
# can replace the base meta and drop a flag an earlier refinement added.
101+
rmeta = refinement.meta or {}
102+
if rmeta.get("extension_required"):
103+
refinement_abstract.add(base_name)
104+
if rmeta.get("unsupported_derived_table"):
105+
refinement_unsupported_dt.add(base_name)
66106
if base_name in graph.models:
67107
# Create a copy with the base name for merging
68108
refinement_for_merge = refinement.model_copy(update={"name": base_name})
69109
merged = merge_model(refinement_for_merge, graph.models[base_name])
70110
graph.models[base_name] = merged
71111

112+
# Union the pre-merge snapshot, every refinement's own flags, and the post-merge
113+
# state (so a flag added by ANY refinement is caught even if a later refinement
114+
# replaced the meta). Resolve this BEFORE extends so a concrete child that only
115+
# INHERITS abstractness is not treated as abstract. Record extends parents too,
116+
# so descendants of an unsupported derived table are detectable after resolution
117+
# clears `extends`.
118+
abstract_views = (
119+
abstract_pre
120+
| refinement_abstract
121+
| {n for n, m in graph.models.items() if (m.meta or {}).get("extension_required")}
122+
)
123+
unsupported_dt_views = (
124+
unsupported_pre
125+
| refinement_unsupported_dt
126+
| {n for n, m in graph.models.items() if (m.meta or {}).get("unsupported_derived_table")}
127+
)
128+
extends_parent = {n: m.extends for n, m in graph.models.items() if m.extends}
129+
72130
# Resolve extends chains. Pre-filter to models whose full chain
73131
# is present so one broken/missing parent doesn't block valid ones.
74132
def _chain_resolvable(name: str, visited: set[str] | None = None) -> bool:
@@ -92,10 +150,63 @@ def _chain_resolvable(name: str, visited: set[str] | None = None) -> bool:
92150
resolved.update(unresolvable)
93151
graph.models = resolved
94152

153+
def _extends_chain_has(name: str, flagset: set[str]) -> bool:
154+
"""True if name or any of its extends-ancestors is in flagset."""
155+
seen: set[str] = set()
156+
cur: str | None = name
157+
while cur is not None and cur not in seen:
158+
if cur in flagset:
159+
return True
160+
seen.add(cur)
161+
cur = extends_parent.get(cur)
162+
return False
163+
164+
# Apply the implicit "table = view name" default AFTER refinements and extends
165+
# are resolved, so a view whose fields/name come from a refinement or whose
166+
# parent was tableless still gets its OWN name as the table (Looker's behavior).
167+
# Skip abstract views, still-unresolved-extends, fieldless views, and views that
168+
# are (or extend) an unsupported derived table. Abstractness is NOT inherited
169+
# through extends (a concrete child of an abstract base gets its own table), but
170+
# an unsupported derived table IS inherited by descendants.
171+
def _apply_default_tables():
172+
for model_name, model in graph.models.items():
173+
if (
174+
model.table is None
175+
and model.sql is None
176+
and not model.extends
177+
and model_name not in abstract_views
178+
and not _extends_chain_has(model_name, unsupported_dt_views)
179+
and not (model.meta or {}).get("unsupported_derived_table")
180+
and (model.dimensions or model.metrics or model.segments)
181+
):
182+
model.table = model_name
183+
184+
_apply_default_tables()
185+
95186
# Second pass: parse explores and add relationships
96187
for lkml_file in lkml_files:
97188
self._parse_explores_from_file(lkml_file, graph)
98189

190+
# Re-apply the default: explores can add segments (sql_always_where /
191+
# always_filter) to an otherwise-fieldless view, which only now makes it
192+
# eligible for the implicit table default.
193+
_apply_default_tables()
194+
195+
# Re-assert non-queryable markers on models intentionally left tableless. A
196+
# refinement can OVERWRITE a view's meta (dropping an `extension_required` flag an
197+
# earlier refinement added) even though abstractness is tracked in side-sets, so
198+
# without this the loader (which keys off final meta) would try to register and
199+
# reject them. Set the flag definitively now, after all merges.
200+
for model_name, model in graph.models.items():
201+
if model.table is not None or model.sql is not None:
202+
continue
203+
if model_name in abstract_views:
204+
model.meta = {**(model.meta or {}), "extension_required": True}
205+
elif _extends_chain_has(model_name, unsupported_dt_views) or (model.meta or {}).get(
206+
"unsupported_derived_table"
207+
):
208+
model.meta = {**(model.meta or {}), "unsupported_derived_table": True}
209+
99210
# Rebuild adjacency graph now that relationships have been added
100211
graph.build_adjacency()
101212

@@ -966,6 +1077,14 @@ def _folded_measure_filter(m_def):
9661077
elif isinstance(extends_list, str):
9671078
extends = extends_list
9681079

1080+
# A LookML view with no sql_table_name/derived_table implicitly uses a table
1081+
# named after the view (Looker's default). A view with a derived_table the
1082+
# adapter cannot turn into SQL must NOT get such a default; mark it so the
1083+
# post-merge pass skips it (the raw derived_table dict is not retained).
1084+
unsupported_derived_table = bool(view_def.get("derived_table")) and sql is None
1085+
if unsupported_derived_table:
1086+
model_meta["unsupported_derived_table"] = True
1087+
9691088
# Build kwargs conditionally so that unset scalars don't appear in
9701089
# model_fields_set. This matters for refinements: merge_model treats
9711090
# every field in model_fields_set as an explicit child override, so

sidemantic/loaders.py

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,40 @@
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, marked in ``meta``) are skipped --
22+
``add_model`` would reject them and abort loading an otherwise-valid project. Any
23+
OTHER tableless model (e.g. an erroneous Hex view missing its base) is left in so
24+
downstream validation still surfaces the real error.
25+
"""
26+
if model.table or model.sql or getattr(model, "dax", None) or getattr(model, "source_uri", None):
27+
return True
28+
meta = model.meta or {}
29+
return not (meta.get("extension_required") or meta.get("unsupported_derived_table"))
30+
31+
32+
def _drop_non_registerable_models(all_models: dict) -> dict:
33+
"""Filter to registerable models AND strip any relationship targeting a dropped one.
34+
35+
An explore may have already added a relationship pointing at a now-skipped template
36+
(e.g. an `extension: required` join target); leaving it would dangle and fail
37+
``sidemantic validate``. Only relationships to models THIS function drops are removed
38+
-- a relationship to a never-defined model is a real error that validation must still
39+
surface, so it is left intact. Returns the filtered dict (mutating survivors' rels).
40+
"""
41+
kept = {name: model for name, model in all_models.items() if _is_registerable_model(model)}
42+
dropped = set(all_models) - set(kept)
43+
for model in kept.values():
44+
rels = getattr(model, "relationships", None)
45+
if rels:
46+
model.relationships = [r for r in rels if r.name not in dropped]
47+
return kept
48+
49+
1650
def load_from_directory(
1751
layer: "SemanticLayer",
1852
directory: str | Path,
@@ -331,10 +365,16 @@ def load_from_directory(
331365
# table pair.
332366
_apply_snowflake_pending_relationships(all_models, all_pending_relationships)
333367

368+
# Drop non-queryable template models (LookML `extension: required` abstract bases /
369+
# unsupported derived tables) BEFORE inference + registration, so FK inference never
370+
# targets a model that won't be registered (which would leave a dangling relationship)
371+
# and add_model never rejects a template that was never meant to be queried.
372+
all_models = _drop_non_registerable_models(all_models)
373+
334374
# Infer cross-model relationships based on naming conventions
335375
_infer_relationships(all_models)
336376

337-
# Add all models to the layer (now with relationships)
377+
# Add all models to the layer (now with relationships).
338378
for model in all_models.values():
339379
if model.name not in layer.graph.models:
340380
layer.add_model(model)
@@ -420,6 +460,8 @@ def _load_sml_directory(layer: "SemanticLayer", directory: Path, all_models: dic
420460
if not hasattr(model, "_source_file"):
421461
model._source_file = str(directory)
422462
all_models.update(graph.models)
463+
# Drop non-queryable templates (+ their dangling rels) before inference + registration.
464+
all_models = _drop_non_registerable_models(all_models)
423465
_infer_relationships(all_models)
424466
for model in all_models.values():
425467
if model.name not in layer.graph.models:

0 commit comments

Comments
 (0)