Skip to content

Commit 958456a

Browse files
committed
Preserve sub-hour precision for LookML 'time' timeframe
Looker's 'time' timeframe keeps full timestamp precision (to the second), but the adapter mapped it to hour granularity, silently collapsing all sub-hour rows. Map 'time' to second granularity, and add second/minute to the export granularity->timeframe map so they round-trip instead of degrading to 'date'. Update the four tests that asserted the old hour grain.
1 parent 0966fe7 commit 958456a

5 files changed

Lines changed: 172 additions & 12 deletions

File tree

sidemantic/adapters/lookml.py

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -916,7 +916,9 @@ def _parse_dimension_group(
916916
# Timeframes that truncate a timestamp to a coarser time grain. These keep
917917
# type="time" with a Sidemantic granularity so they behave as time dimensions.
918918
_TIME_GRANULARITY_TIMEFRAMES = {
919-
"time": "hour",
919+
# Looker's "time" timeframe keeps full timestamp precision (to the second);
920+
# truncating to the hour silently collapses sub-hour rows.
921+
"time": "second",
920922
"time_of_day": "hour",
921923
"hour": "hour",
922924
"minute": "minute",
@@ -976,7 +978,10 @@ def _build_timeframe_dimension(
976978
label = dim_group_def.get("label")
977979
description = dim_group_def.get("description")
978980

979-
# Time-truncation timeframes -> time dimension with granularity.
981+
# Time-truncation timeframes -> time dimension with granularity. Remember the
982+
# original LookML timeframe so export can round-trip it exactly: several
983+
# timeframes (e.g. "time" and "second") map to the same sidemantic granularity
984+
# and would otherwise collapse to one on export.
980985
granularity = self._TIME_GRANULARITY_TIMEFRAMES.get(timeframe)
981986
if granularity is not None:
982987
return Dimension(
@@ -986,6 +991,7 @@ def _build_timeframe_dimension(
986991
granularity=granularity,
987992
label=label,
988993
description=description,
994+
meta={"lookml_timeframe": timeframe},
989995
)
990996

991997
# Fiscal quarter/year truncations honoring fiscal_month_offset. The base
@@ -2054,30 +2060,52 @@ def _export_view(self, model: Model, graph: SemanticGraph) -> dict:
20542060
for dim in time_dims:
20552061
# Extract base name (remove _date, _week, etc suffix)
20562062
base_name = dim.name
2057-
for suffix in ["_date", "_week", "_month", "_quarter", "_year", "_time", "_hour"]:
2063+
for suffix in ["_date", "_week", "_month", "_quarter", "_year", "_time", "_hour", "_minute", "_second"]:
20582064
if dim.name.endswith(suffix):
20592065
base_name = dim.name[: -len(suffix)]
20602066
break
20612067
base_name_groups[base_name].append(dim)
20622068

20632069
dimension_groups = []
20642070
for base_name, dims in base_name_groups.items():
2065-
# Map granularity to timeframe
2071+
# Map granularity to timeframe. Used as a fallback for dimensions not
2072+
# imported from LookML (which carry no meta['lookml_timeframe']); a
2073+
# second-grain dimension maps to the LookML `second` timeframe so native
2074+
# `*_second` fields round-trip instead of collapsing to `time`.
20662075
granularity_mapping = {
2067-
"hour": "time",
2076+
"second": "second",
2077+
"minute": "minute",
2078+
"hour": "hour",
20682079
"day": "date",
20692080
"week": "week",
20702081
"month": "month",
20712082
"quarter": "quarter",
20722083
"year": "year",
20732084
}
20742085

2075-
# Collect all timeframes for this base name
2086+
# Collect all timeframes for this base name, de-duplicated. LookML's
2087+
# "time" and "second" timeframes both import to second granularity
2088+
# (sidemantic has no separate "time" grain), so without dedup a group
2089+
# containing both would emit duplicate timeframes (e.g. [time, time])
2090+
# and drop a field on re-import.
20762091
timeframes = []
2092+
seen_timeframes = set()
20772093
sql = None
20782094
for dim in dims:
2079-
timeframe = granularity_mapping.get(dim.granularity, "date")
2080-
timeframes.append(timeframe)
2095+
# Prefer the original LookML timeframe captured at import (so
2096+
# "time"/"second"/etc. round-trip distinctly).
2097+
timeframe = (dim.meta or {}).get("lookml_timeframe")
2098+
if timeframe is None:
2099+
# For native (non-import) dims, "time" and "second" share the
2100+
# second granularity; use the name suffix to pick the one that
2101+
# round-trips the field name, else fall back to the grain map.
2102+
if dim.granularity == "second" and dim.name.endswith("_time"):
2103+
timeframe = "time"
2104+
else:
2105+
timeframe = granularity_mapping.get(dim.granularity, "date")
2106+
if timeframe not in seen_timeframes:
2107+
seen_timeframes.add(timeframe)
2108+
timeframes.append(timeframe)
20812109
if dim.sql and not sql:
20822110
sql = dim.sql
20832111

tests/adapters/lookml/test_advanced_measure_types.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ class TestDimensionGroupTimeframes:
129129
def test_time_truncation_timeframes_are_time(self, graph):
130130
model = graph.get_model("events_calendar")
131131
for tf, gran in [
132-
("occurred_time", "hour"),
132+
("occurred_time", "second"),
133133
("occurred_date", "day"),
134134
("occurred_week", "week"),
135135
("occurred_month", "month"),

tests/adapters/lookml/test_edge_cases.py

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2290,6 +2290,137 @@ def test_lookml_view_with_unsupported_derived_table_not_defaulted():
22902290
assert model.table is None # not fabricated as a physical table named after the view
22912291

22922292

2293+
def test_lookml_time_timeframe_keeps_second_precision():
2294+
"""Looker's "time" timeframe keeps to-the-second precision, not hour."""
2295+
graph = _parse_lkml(
2296+
"""
2297+
view: v {
2298+
sql_table_name: t ;;
2299+
dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; }
2300+
dimension_group: created {
2301+
type: time
2302+
timeframes: [time, date, minute]
2303+
sql: ${TABLE}.created_at ;;
2304+
}
2305+
}
2306+
"""
2307+
)
2308+
model = graph.get_model("v")
2309+
assert model.get_dimension("created_time").granularity == "second"
2310+
assert model.get_dimension("created_minute").granularity == "minute"
2311+
assert model.get_dimension("created_date").granularity == "day"
2312+
2313+
2314+
def test_lookml_time_grain_roundtrip():
2315+
"""time/hour/minute/date grains round-trip through export without grain or suffix corruption."""
2316+
import tempfile
2317+
2318+
graph = _parse_lkml(
2319+
"""
2320+
view: v {
2321+
sql_table_name: t ;;
2322+
dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; }
2323+
dimension_group: created {
2324+
type: time
2325+
timeframes: [time, hour, minute, date]
2326+
sql: ${TABLE}.created_at ;;
2327+
}
2328+
}
2329+
"""
2330+
)
2331+
out = tempfile.mktemp(suffix=".lkml")
2332+
LookMLAdapter().export(graph, out)
2333+
grains = {
2334+
d.name: d.granularity for d in LookMLAdapter().parse(Path(out)).get_model("v").dimensions if d.type == "time"
2335+
}
2336+
assert grains == {
2337+
"created_time": "second",
2338+
"created_hour": "hour",
2339+
"created_minute": "minute",
2340+
"created_date": "day",
2341+
}
2342+
2343+
2344+
def test_lookml_native_second_grain_roundtrips():
2345+
"""A second-grain dimension not imported from LookML (no meta) exports as `second`, not `time`."""
2346+
import tempfile
2347+
2348+
from sidemantic import Dimension, Model
2349+
from sidemantic.core.semantic_graph import SemanticGraph
2350+
2351+
graph = SemanticGraph()
2352+
graph.add_model(
2353+
Model(
2354+
name="v",
2355+
table="t",
2356+
primary_key="id",
2357+
dimensions=[
2358+
Dimension(name="id", type="numeric", sql="id"),
2359+
Dimension(name="created_second", type="time", granularity="second", sql="{model}.created_at"),
2360+
],
2361+
)
2362+
)
2363+
out = tempfile.mktemp(suffix=".lkml")
2364+
LookMLAdapter().export(graph, out)
2365+
names = {d.name for d in LookMLAdapter().parse(Path(out)).get_model("v").dimensions if d.type == "time"}
2366+
assert "created_second" in names
2367+
2368+
2369+
def test_lookml_native_time_named_second_grain_roundtrips():
2370+
"""A native second-grain dim named *_time must export as `time` so the name round-trips."""
2371+
import tempfile
2372+
2373+
from sidemantic import Dimension, Model
2374+
from sidemantic.core.semantic_graph import SemanticGraph
2375+
2376+
graph = SemanticGraph()
2377+
graph.add_model(
2378+
Model(
2379+
name="v",
2380+
table="t",
2381+
primary_key="id",
2382+
dimensions=[
2383+
Dimension(name="id", type="numeric", sql="id"),
2384+
Dimension(name="created_time", type="time", granularity="second", sql="{model}.created_at"),
2385+
],
2386+
)
2387+
)
2388+
out = tempfile.mktemp(suffix=".lkml")
2389+
LookMLAdapter().export(graph, out)
2390+
names = {d.name for d in LookMLAdapter().parse(Path(out)).get_model("v").dimensions if d.type == "time"}
2391+
assert "created_time" in names # not renamed to created_second
2392+
2393+
2394+
def test_lookml_time_and_second_timeframes_no_duplicate_export():
2395+
"""A group with both `time` and `second` (both -> second grain) must not emit duplicate timeframes."""
2396+
import re
2397+
import tempfile
2398+
2399+
graph = _parse_lkml(
2400+
"""
2401+
view: v {
2402+
sql_table_name: t ;;
2403+
dimension: id { primary_key: yes type: number sql: ${TABLE}.id ;; }
2404+
dimension_group: created {
2405+
type: time
2406+
timeframes: [time, second, hour]
2407+
sql: ${TABLE}.created_at ;;
2408+
}
2409+
}
2410+
"""
2411+
)
2412+
out = tempfile.mktemp(suffix=".lkml")
2413+
LookMLAdapter().export(graph, out)
2414+
text = open(out).read()
2415+
timeframes = re.search(r"timeframes:\s*\[([^\]]*)\]", text).group(1)
2416+
parts = [p.strip() for p in timeframes.split(",")]
2417+
assert len(parts) == len(set(parts)), f"duplicate timeframes exported: {parts}"
2418+
# Both `time` and `second` are preserved (no collapse/drop) via the stored
2419+
# original timeframe, so re-import keeps every member.
2420+
names = {d.name for d in LookMLAdapter().parse(Path(out)).get_model("v").dimensions if d.type == "time"}
2421+
assert {"created_time", "created_second", "created_hour"} <= names
2422+
2423+
22932424
def test_lookml_view_without_table_defaults_to_view_name():
22942425
"""A view with no sql_table_name/derived_table should default its table to the view name."""
22952426
from sidemantic import SemanticLayer

tests/adapters/lookml/test_fixtures.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ def test_redshift_etl_errors_time_dimensions(self):
6969
model = self.graph.get_model("redshift_etl_errors")
7070
assert model.get_dimension("error_time") is not None
7171
assert model.get_dimension("error_time").type == "time"
72-
assert model.get_dimension("error_time").granularity == "hour"
72+
assert model.get_dimension("error_time").granularity == "second"
7373
assert model.get_dimension("error_date") is not None
7474
assert model.get_dimension("error_date").type == "time"
7575
assert model.get_dimension("error_date").granularity == "day"
@@ -332,7 +332,7 @@ def test_created_time_group(self):
332332
assert model.get_dimension("created_month") is not None
333333
assert model.get_dimension("created_year") is not None
334334
assert model.get_dimension("created_time") is not None
335-
assert model.get_dimension("created_time").granularity == "hour"
335+
assert model.get_dimension("created_time").granularity == "second"
336336

337337
def test_shipped_time_group(self):
338338
model = self.graph.get_model("order_items")

tests/adapters/lookml/test_parsing.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,8 @@ def test_lookml_adapter_ecommerce():
119119
# Test dimension group with multiple timeframes
120120
created_time = orders.get_dimension("created_time")
121121
assert created_time is not None
122-
assert created_time.granularity == "hour"
122+
# Looker's "time" timeframe keeps to-the-second precision.
123+
assert created_time.granularity == "second"
123124

124125
created_date = orders.get_dimension("created_date")
125126
assert created_date is not None

0 commit comments

Comments
 (0)