Skip to content

Commit ec4c6a6

Browse files
authored
Fix yardstick rewriter for LookML-loaded models with {model} placeholder (#106)
The LookML adapter stores dimension/measure SQL with {model} placeholders (replacing ${TABLE}). The standard query rewriter handles this, but the yardstick rewriter passed these directly to sqlglot.parse_one(), which choked on the curly braces producing invalid SQL like {'_0': table.model}. Fixes: - Replace {model} in dimension SQL before parsing (5 call sites) - Skip dimension expansion when resolved SQL is just table.column (prevents unqualified outer references in correlation predicates)
1 parent 6ead68a commit ec4c6a6

1 file changed

Lines changed: 18 additions & 3 deletions

File tree

sidemantic/sql/query_rewriter.py

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -984,8 +984,16 @@ def _resolve_yardstick_dimension_expression(
984984
return None
985985

986986
dimension_sql = dimension.sql_expr.strip()
987+
988+
# Replace {model} placeholder (used by LookML adapter) with table alias
989+
dimension_sql = dimension_sql.replace('{model}', table_alias)
990+
991+
# If the resolved SQL is just the column name (possibly table-qualified),
992+
# no expansion is needed and the original column reference should be preserved
987993
if dimension_sql.lower() == column.name.lower():
988994
return None
995+
if dimension_sql.lower() == f'{table_alias}.{column.name}'.lower():
996+
return None
989997

990998
expr = sqlglot.parse_one(dimension_sql, dialect=self.dialect)
991999
expr = self._rewrite_tables(
@@ -1161,7 +1169,9 @@ def _build_yardstick_measure_sql(
11611169
)
11621170
if is_derived_formula and measure.sql:
11631171
visiting.add(visit_key)
1164-
formula_expr = sqlglot.parse_one(measure.sql, dialect=self.dialect)
1172+
# Replace {model} placeholder (used by LookML adapter) with model alias
1173+
_formula_sql = measure.sql.replace('{model}', model_alias)
1174+
formula_expr = sqlglot.parse_one(_formula_sql, dialect=self.dialect)
11651175

11661176
def replace_measure_refs(node: exp.Expression) -> exp.Expression:
11671177
if not isinstance(node, exp.Column):
@@ -1246,7 +1256,9 @@ def replace_measure_refs(node: exp.Expression) -> exp.Expression:
12461256
base_predicates.append(visible_expr.sql(dialect=self.dialect))
12471257

12481258
for measure_filter in measure.filters or []:
1249-
filter_expr = sqlglot.parse_one(measure_filter, dialect=self.dialect)
1259+
# Replace {model} placeholder (used by LookML adapter) with inner alias
1260+
_filter_sql = measure_filter.replace('{model}', '_inner')
1261+
filter_expr = sqlglot.parse_one(_filter_sql, dialect=self.dialect)
12501262
if default_alias and single_model_scope:
12511263
filter_expr = self._qualify_unaliased_columns(filter_expr, default_alias)
12521264
filter_expr = self._rewrite_tables(
@@ -1320,6 +1332,8 @@ def _build_yardstick_aggregation_expr(self, measure, model_alias: str, model_nam
13201332
def _rewrite_yardstick_measure_expression(
13211333
self, sql_expr: str, model_alias: str, model_name: str, target_alias: str
13221334
) -> str:
1335+
# Replace {model} placeholder (used by LookML adapter) with target alias
1336+
sql_expr = sql_expr.replace('{model}', target_alias)
13231337
parsed = sqlglot.parse_one(sql_expr, dialect=self.dialect)
13241338
parsed = self._rewrite_tables(
13251339
parsed,
@@ -1333,7 +1347,8 @@ def _rewrite_yardstick_measure_expression(
13331347
return parsed.sql(dialect=self.dialect)
13341348

13351349
def _is_window_measure_expression(self, sql_expr: str) -> bool:
1336-
parsed = sqlglot.parse_one(sql_expr, dialect=self.dialect)
1350+
# Strip {model} placeholder to avoid parse errors
1351+
parsed = sqlglot.parse_one(sql_expr.replace('{model}', '__model'), dialect=self.dialect)
13371352
return any(isinstance(node, exp.Window) for node in parsed.walk())
13381353

13391354
def _build_yardstick_context_dimensions(

0 commit comments

Comments
 (0)