Skip to content

Commit

Permalink
Remove unused grain_to_date from SqlTimeDeltaExpression
Browse files Browse the repository at this point in the history
The original implementation of SqlTimeDeltaExpression had a custom
override for grain_to_date intended to support cumulative metrics.
This parameter would change the expression from a time delta to a
date_trunc. At some point we cleaned up the callsites to invoke
date_trunc directly instead of offloading this work to an overloaded
class.

This commit simply removes the confusing parameter in order to streamline
the expression rendering for time delta operations.
  • Loading branch information
tlento committed Oct 5, 2023
1 parent 32f8ae0 commit 1db231a
Show file tree
Hide file tree
Showing 5 changed files with 1 addition and 38 deletions.
10 changes: 0 additions & 10 deletions metricflow/sql/render/big_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,16 +145,6 @@ def render_date_part(self, date_part: DatePart) -> str:
def visit_time_delta_expr(self, node: SqlTimeDeltaExpression) -> SqlExpressionRenderResult:
"""Render time delta for BigQuery, which requires ISO prefixing for the WEEK granularity value."""
column = node.arg.accept(self)
if node.grain_to_date:
granularity = node.granularity
if granularity == TimeGranularity.WEEK:
granularity_value = "ISO" + granularity.value.upper()
else:
granularity_value = granularity.value
return SqlExpressionRenderResult(
sql=f"DATE_TRUNC({column.sql}, {granularity_value})",
bind_parameters=column.bind_parameters,
)

return SqlExpressionRenderResult(
sql=f"DATE_SUB(CAST({column.sql} AS {self.timestamp_data_type}), INTERVAL {node.count} {node.granularity.value})",
Expand Down
5 changes: 0 additions & 5 deletions metricflow/sql/render/duckdb_renderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,6 @@ def supported_percentile_function_types(self) -> Collection[SqlPercentileFunctio
def visit_time_delta_expr(self, node: SqlTimeDeltaExpression) -> SqlExpressionRenderResult:
"""Render time delta expression for DuckDB, which requires slightly different syntax from other engines."""
arg_rendered = node.arg.accept(self)
if node.grain_to_date:
return SqlExpressionRenderResult(
sql=f"DATE_TRUNC('{node.granularity.value}', {arg_rendered.sql}::timestamp)",
bind_parameters=arg_rendered.bind_parameters,
)

count = node.count
granularity = node.granularity
Expand Down
5 changes: 0 additions & 5 deletions metricflow/sql/render/expr_renderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,11 +283,6 @@ def render_date_part(self, date_part: DatePart) -> str:

def visit_time_delta_expr(self, node: SqlTimeDeltaExpression) -> SqlExpressionRenderResult: # noqa: D
arg_rendered = node.arg.accept(self)
if node.grain_to_date:
return SqlExpressionRenderResult(
sql=f"DATE_TRUNC('{node.granularity.value}', {arg_rendered.sql}::timestamp)",
bind_parameters=arg_rendered.bind_parameters,
)

count = node.count
granularity = node.granularity
Expand Down
5 changes: 0 additions & 5 deletions metricflow/sql/render/postgres.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,6 @@ def supported_percentile_function_types(self) -> Collection[SqlPercentileFunctio
def visit_time_delta_expr(self, node: SqlTimeDeltaExpression) -> SqlExpressionRenderResult:
"""Render time delta operations for PostgreSQL, which needs custom support for quarterly granularity."""
arg_rendered = node.arg.accept(self)
if node.grain_to_date:
return SqlExpressionRenderResult(
sql=f"DATE_TRUNC('{node.granularity.value}', {arg_rendered.sql}::timestamp)",
bind_parameters=arg_rendered.bind_parameters,
)

count = node.count
granularity = node.granularity
Expand Down
14 changes: 1 addition & 13 deletions metricflow/sql/sql_exprs.py
Original file line number Diff line number Diff line change
Expand Up @@ -1251,13 +1251,11 @@ def __init__( # noqa: D
arg: SqlExpressionNode,
count: int,
granularity: TimeGranularity,
grain_to_date: Optional[TimeGranularity] = None,
) -> None:
super().__init__(node_id=self.create_unique_id(), parent_nodes=[arg])
self._count = count
self._time_granularity = granularity
self._arg = arg
self._grain_to_date = grain_to_date

@classmethod
def id_prefix(cls) -> str: # noqa: D
Expand All @@ -1278,10 +1276,6 @@ def description(self) -> str: # noqa: D
def arg(self) -> SqlExpressionNode: # noqa: D
return self._arg

@property
def grain_to_date(self) -> Optional[TimeGranularity]: # noqa: D
return self._grain_to_date

@property
def count(self) -> int: # noqa: D
return self._count
Expand All @@ -1299,7 +1293,6 @@ def rewrite( # noqa: D
arg=self.arg.rewrite(column_replacements, should_render_table_alias),
count=self.count,
granularity=self.granularity,
grain_to_date=self.grain_to_date,
)

@property
Expand All @@ -1311,12 +1304,7 @@ def lineage(self) -> SqlExpressionTreeLineage: # noqa: D
def matches(self, other: SqlExpressionNode) -> bool: # noqa: D
if not isinstance(other, SqlTimeDeltaExpression):
return False
return (
self.count == other.count
and self.granularity == other.granularity
and self.grain_to_date == other.grain_to_date
and self._parents_match(other)
)
return self.count == other.count and self.granularity == other.granularity and self._parents_match(other)


class SqlCastToTimestampExpression(SqlExpressionNode):
Expand Down

0 comments on commit 1db231a

Please sign in to comment.