Skip to content

Commit

Permalink
Rename SqlTimeDeltaExpression to SqlSubtractTimeIntervalExpression
Browse files Browse the repository at this point in the history
The original name for this class was misleading, as a time delta is
effectively an interval computed as a difference. This suggested either
that the rendering should be a date_diff to produce an interval between
two timestamps, or that it should be an interval expression for use
in some other operation.

In reality, this class provides the functionality of a date_subtract,
where we subtract an interval value from a given input timestamp.
  • Loading branch information
tlento committed Oct 5, 2023
1 parent 1db231a commit 2fdc42a
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 18 deletions.
6 changes: 3 additions & 3 deletions metricflow/plan_conversion/sql_join_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
SqlIsNullExpression,
SqlLogicalExpression,
SqlLogicalOperator,
SqlTimeDeltaExpression,
SqlSubtractTimeIntervalExpression,
)
from metricflow.sql.sql_plan import SqlExpressionNode, SqlJoinDescription, SqlJoinType, SqlSelectStatementNode

Expand Down Expand Up @@ -441,7 +441,7 @@ def make_cumulative_metric_time_range_join_description(
start_of_range_comparison_expr = SqlComparisonExpression(
left_expr=metric_time_column_expr,
comparison=SqlComparison.GREATER_THAN,
right_expr=SqlTimeDeltaExpression(
right_expr=SqlSubtractTimeIntervalExpression(
arg=time_spine_column_expr,
count=node.window.count,
granularity=node.window.granularity,
Expand Down Expand Up @@ -481,7 +481,7 @@ def make_join_to_time_spine_join_description(
col_ref=SqlColumnReference(table_alias=time_spine_alias, column_name=metric_time_dimension_column_name)
)
if node.offset_window:
left_expr = SqlTimeDeltaExpression(
left_expr = SqlSubtractTimeIntervalExpression(
arg=left_expr, count=node.offset_window.count, granularity=node.offset_window.granularity
)
elif node.offset_to_grain:
Expand Down
4 changes: 2 additions & 2 deletions metricflow/sql/render/big_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
SqlGenerateUuidExpression,
SqlPercentileExpression,
SqlPercentileFunctionType,
SqlTimeDeltaExpression,
SqlSubtractTimeIntervalExpression,
)
from metricflow.sql.sql_plan import SqlSelectColumn
from metricflow.time.date_part import DatePart
Expand Down Expand Up @@ -142,7 +142,7 @@ def render_date_part(self, date_part: DatePart) -> str:
return super().render_date_part(date_part)

@override
def visit_time_delta_expr(self, node: SqlTimeDeltaExpression) -> SqlExpressionRenderResult:
def visit_time_delta_expr(self, node: SqlSubtractTimeIntervalExpression) -> SqlExpressionRenderResult:
"""Render time delta for BigQuery, which requires ISO prefixing for the WEEK granularity value."""
column = node.arg.accept(self)

Expand Down
4 changes: 2 additions & 2 deletions metricflow/sql/render/duckdb_renderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
SqlGenerateUuidExpression,
SqlPercentileExpression,
SqlPercentileFunctionType,
SqlTimeDeltaExpression,
SqlSubtractTimeIntervalExpression,
)


Expand All @@ -34,7 +34,7 @@ def supported_percentile_function_types(self) -> Collection[SqlPercentileFunctio
}

@override
def visit_time_delta_expr(self, node: SqlTimeDeltaExpression) -> SqlExpressionRenderResult:
def visit_time_delta_expr(self, node: SqlSubtractTimeIntervalExpression) -> SqlExpressionRenderResult:
"""Render time delta expression for DuckDB, which requires slightly different syntax from other engines."""
arg_rendered = node.arg.accept(self)

Expand Down
4 changes: 2 additions & 2 deletions metricflow/sql/render/expr_renderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
SqlRatioComputationExpression,
SqlStringExpression,
SqlStringLiteralExpression,
SqlTimeDeltaExpression,
SqlSubtractTimeIntervalExpression,
SqlWindowFunctionExpression,
)
from metricflow.sql.sql_plan import SqlSelectColumn
Expand Down Expand Up @@ -281,7 +281,7 @@ def render_date_part(self, date_part: DatePart) -> str:
"""Render DATE PART for an EXTRACT expression."""
return date_part.value

def visit_time_delta_expr(self, node: SqlTimeDeltaExpression) -> SqlExpressionRenderResult: # noqa: D
def visit_time_delta_expr(self, node: SqlSubtractTimeIntervalExpression) -> SqlExpressionRenderResult: # noqa: D
arg_rendered = node.arg.accept(self)

count = node.count
Expand Down
4 changes: 2 additions & 2 deletions metricflow/sql/render/postgres.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
SqlGenerateUuidExpression,
SqlPercentileExpression,
SqlPercentileFunctionType,
SqlTimeDeltaExpression,
SqlSubtractTimeIntervalExpression,
)


Expand All @@ -37,7 +37,7 @@ def supported_percentile_function_types(self) -> Collection[SqlPercentileFunctio
return {SqlPercentileFunctionType.CONTINUOUS, SqlPercentileFunctionType.DISCRETE}

@override
def visit_time_delta_expr(self, node: SqlTimeDeltaExpression) -> SqlExpressionRenderResult:
def visit_time_delta_expr(self, node: SqlSubtractTimeIntervalExpression) -> SqlExpressionRenderResult:
"""Render time delta operations for PostgreSQL, which needs custom support for quarterly granularity."""
arg_rendered = node.arg.accept(self)

Expand Down
16 changes: 11 additions & 5 deletions metricflow/sql/sql_exprs.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ def visit_extract_expr(self, node: SqlExtractExpression) -> VisitorOutputT: # n
pass

@abstractmethod
def visit_time_delta_expr(self, node: SqlTimeDeltaExpression) -> VisitorOutputT: # noqa: D
def visit_time_delta_expr(self, node: SqlSubtractTimeIntervalExpression) -> VisitorOutputT: # noqa: D
pass

@abstractmethod
Expand Down Expand Up @@ -1243,8 +1243,14 @@ def matches(self, other: SqlExpressionNode) -> bool: # noqa: D
return self._parents_match(other)


class SqlTimeDeltaExpression(SqlExpressionNode):
"""create time delta between eg `DATE_SUB(ds, 2, month)`."""
class SqlSubtractTimeIntervalExpression(SqlExpressionNode):
"""Represents an interval subtraction from a given timestamp.
This node contains the information required to produce a SQL statement which subtracts an interval with the given
count and granularity (which together define the interval duration) from the input timestamp expression. The return
value from the SQL rendering for this expression should be a timestamp expression offset from the initial input
value.
"""

def __init__( # noqa: D
self,
Expand Down Expand Up @@ -1289,7 +1295,7 @@ def rewrite( # noqa: D
column_replacements: Optional[SqlColumnReplacements] = None,
should_render_table_alias: Optional[bool] = None,
) -> SqlExpressionNode:
return SqlTimeDeltaExpression(
return SqlSubtractTimeIntervalExpression(
arg=self.arg.rewrite(column_replacements, should_render_table_alias),
count=self.count,
granularity=self.granularity,
Expand All @@ -1302,7 +1308,7 @@ def lineage(self) -> SqlExpressionTreeLineage: # noqa: D
)

def matches(self, other: SqlExpressionNode) -> bool: # noqa: D
if not isinstance(other, SqlTimeDeltaExpression):
if not isinstance(other, SqlSubtractTimeIntervalExpression):
return False
return self.count == other.count and self.granularity == other.granularity and self._parents_match(other)

Expand Down
4 changes: 2 additions & 2 deletions metricflow/test/integration/test_configured_cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
SqlPercentileExpressionArgument,
SqlPercentileFunctionType,
SqlStringExpression,
SqlTimeDeltaExpression,
SqlSubtractTimeIntervalExpression,
)
from metricflow.test.compare_df import assert_dataframes_equal
from metricflow.test.fixtures.setup_fixtures import MetricFlowTestSessionState
Expand Down Expand Up @@ -85,7 +85,7 @@ def render_date_sub(
granularity: TimeGranularity,
) -> str:
"""Renders a date subtract expression."""
expr = SqlTimeDeltaExpression(
expr = SqlSubtractTimeIntervalExpression(
arg=SqlColumnReferenceExpression(SqlColumnReference(table_alias, column_alias)),
count=count,
granularity=granularity,
Expand Down

0 comments on commit 2fdc42a

Please sign in to comment.