Skip to content

Commit a6735e6

Browse files
committed
Fix inherited primary key and time_shift base_metric guard
Only include primary_key in Model kwargs when explicitly declared via primary_key:true on a dimension, so child cubes in an extends chain inherit the parent's key instead of silently defaulting to "id". Guard time_shift conversion: only set type=time_comparison when the base_metric reference can be parsed from sql. If sql is not a bare {metric_name} token, fall through to normal derived handling instead of raising a validation error.
1 parent a52767c commit a6735e6

1 file changed

Lines changed: 20 additions & 19 deletions

File tree

sidemantic/adapters/cube.py

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ def _parse_cube(self, cube_def: dict) -> Model | None:
172172

173173
# Parse dimensions and find primary key
174174
dimensions = []
175-
primary_key = "id" # default
175+
primary_key = None # Only set if explicitly declared
176176

177177
for dim_def in cube_def.get("dimensions") or []:
178178
dim = self._parse_dimension(dim_def, name)
@@ -246,16 +246,17 @@ def _parse_cube(self, cube_def: dict) -> Model | None:
246246
if preagg:
247247
pre_aggregations.append(preagg)
248248

249-
# Build kwargs, omitting None table/sql so inheritance can provide them
249+
# Build kwargs, omitting None table/sql/primary_key so inheritance can provide them
250250
model_kwargs = {
251251
"name": name,
252-
"primary_key": primary_key,
253252
"relationships": relationships,
254253
"dimensions": dimensions,
255254
"metrics": measures,
256255
"segments": segments,
257256
"pre_aggregations": pre_aggregations,
258257
}
258+
if primary_key is not None:
259+
model_kwargs["primary_key"] = primary_key
259260
if table is not None:
260261
model_kwargs["table"] = table
261262
if sql is not None:
@@ -424,6 +425,7 @@ def _parse_measure(self, measure_def: dict, cube_name: str) -> Metric | None:
424425
measure_sql = _normalize_cube_sql(measure_def.get("sql"), cube_name)
425426

426427
# Check for time_shift (period-over-period comparison)
428+
# Only convert to time_comparison if we can extract a base_metric reference
427429
base_metric = None
428430
comparison_type = None
429431
time_offset = None
@@ -432,23 +434,22 @@ def _parse_measure(self, measure_def: dict, cube_name: str) -> Metric | None:
432434
ts = time_shift_def[0]
433435
ts_interval = ts.get("interval")
434436
ts_type = ts.get("type")
435-
if ts_type == "prior" and ts_interval:
436-
metric_type = "time_comparison"
437-
time_offset = str(ts_interval)
438-
comparison_map = {
439-
"1 year": "yoy",
440-
"1 month": "mom",
441-
"1 week": "wow",
442-
"1 day": "dod",
443-
"1 quarter": "qoq",
444-
}
445-
comparison_type = comparison_map.get(ts_interval, "prior_period")
437+
if ts_type == "prior" and ts_interval and measure_sql:
446438
# Extract base_metric from sql like "{measure_name}"
447-
if measure_sql:
448-
base_match = re.match(r"^\s*\{(\w+)\}\s*$", measure_sql)
449-
if base_match:
450-
base_metric = f"{cube_name}.{base_match.group(1)}"
451-
measure_sql = None # Clear sql, base_metric carries the reference
439+
base_match = re.match(r"^\s*\{(\w+)\}\s*$", measure_sql)
440+
if base_match:
441+
base_metric = f"{cube_name}.{base_match.group(1)}"
442+
measure_sql = None # Clear sql, base_metric carries the reference
443+
metric_type = "time_comparison"
444+
time_offset = str(ts_interval)
445+
comparison_map = {
446+
"1 year": "yoy",
447+
"1 month": "mom",
448+
"1 week": "wow",
449+
"1 day": "dod",
450+
"1 quarter": "qoq",
451+
}
452+
comparison_type = comparison_map.get(ts_interval, "prior_period")
452453

453454
# Convert ${measure_name} references to model_name.measure_name format
454455
# This is needed for derived metrics that reference other measures

0 commit comments

Comments
 (0)