Skip to content

Commit

Permalink
Remove inconsistent rendering of time constraints
Browse files Browse the repository at this point in the history
render_time_constraint() and render_between_time_constraint() result in the same output, but use different SQL syntax. The first one matches our actual rendered syntax, so use that one everywhere.
  • Loading branch information
courtneyholcomb committed Aug 13, 2024
1 parent b9195a6 commit 76ac912
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 24 deletions.
8 changes: 4 additions & 4 deletions tests_metricflow/integration/test_cases/itest_metrics.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1704,7 +1704,7 @@ integration_test:
SELECT
ds AS metric_time__day
FROM {{ source_schema }}.mf_time_spine subq_6
WHERE {{ render_between_time_constraint("ds", "2019-12-19", "2020-01-02") }}
WHERE {{ render_time_constraint("ds", "2019-12-19", "2020-01-02") }}
) subq_5
INNER JOIN (
SELECT
Expand Down Expand Up @@ -2192,7 +2192,7 @@ integration_test:
SELECT
SUM(1) AS bookings_join_to_time_spine
FROM {{ source_schema }}.fct_bookings
WHERE {{ render_between_time_constraint("ds", "2020-01-03", "2020-01-03") }}
WHERE {{ render_time_constraint("ds", "2020-01-03", "2020-01-03") }}
---
integration_test:
name: join_to_time_spine_with_queried_time_constraint
Expand All @@ -2211,10 +2211,10 @@ integration_test:
{{ render_date_trunc("ds", TimeGranularity.DAY) }} AS ds
, SUM(1) AS bookings
FROM {{ source_schema }}.fct_bookings
WHERE {{ render_between_time_constraint("ds", "2020-01-03", "2020-01-03") }}
WHERE {{ render_time_constraint("ds", "2020-01-03", "2020-01-03") }}
GROUP BY {{ render_date_trunc("ds", TimeGranularity.DAY) }}
) a ON b.ds = a.ds
WHERE {{ render_between_time_constraint("b.ds", "2020-01-03", "2020-01-03") }}
WHERE {{ render_time_constraint("b.ds", "2020-01-03", "2020-01-03") }}
---
integration_test:
name: simple_metric_with_sub_daily_dimension
Expand Down
2 changes: 1 addition & 1 deletion tests_metricflow/integration/test_cases/itest_simple.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ integration_test:
SUM(booking_value) AS booking_value
, ds AS metric_time__day
FROM {{ source_schema }}.fct_bookings
WHERE {{ render_between_time_constraint("ds", "2020-01-01", "2020-01-01") }}
WHERE {{ render_time_constraint("ds", "2020-01-01", "2020-01-01") }}
GROUP BY
ds
---
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ integration_test:
FROM {{ source_schema }}.fct_bookings_dt b
LEFT OUTER JOIN {{ source_schema }}.dim_listings_latest l
ON b.listing_id = l.listing_id
WHERE {{ render_between_time_constraint("dt", "2020-01-01", "2020-01-01") }}
WHERE {{ render_time_constraint("dt", "2020-01-01", "2020-01-01") }}
GROUP BY
b.is_instant
, b.dt
Expand Down
18 changes: 0 additions & 18 deletions tests_metricflow/integration/test_configured_cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,22 +65,6 @@ def render_time_constraint(
stop_expr = self.cast_to_ts(f"{stop_time_plus_one_day}")
return f"{self.cast_expr_to_ts(expr)} >= {start_expr} AND {self.cast_expr_to_ts(expr)} < {stop_expr}"

def render_between_time_constraint(
self,
expr: str,
start_time: str,
stop_time: str,
) -> str:
"""Render an expression like "ds between timestamp '2020-01-01' AND timestamp '2020-01-02'".
This will cast the literals as needed for each engine, and provide an alternative to incrementing
the date as we do in render_time_constraint. Using BETWEEN is more robust for cases involving potentially
mixed granularities.
"""
start_expr = self.cast_to_ts(f"{start_time}")
stop_expr = self.cast_to_ts(f"{stop_time}")
return f"{expr} BETWEEN {start_expr} AND {stop_expr}"

def cast_expr_to_ts(self, expr: str) -> str:
"""Returns the expression as a new expression cast to the timestamp type, if applicable for the DB."""
return f"CAST({expr} AS {self._sql_client.sql_query_plan_renderer.expr_renderer.timestamp_data_type})"
Expand Down Expand Up @@ -295,7 +279,6 @@ def test_case(
).render(
source_schema=mf_test_configuration.mf_source_schema,
render_time_constraint=check_query_helpers.render_time_constraint,
render_between_time_constraint=check_query_helpers.render_between_time_constraint,
TimeGranularity=TimeGranularity,
DatePart=DatePart,
render_date_sub=check_query_helpers.render_date_sub,
Expand Down Expand Up @@ -328,7 +311,6 @@ def test_case(
).render(
source_schema=mf_test_configuration.mf_source_schema,
render_time_constraint=check_query_helpers.render_time_constraint,
render_between_time_constraint=check_query_helpers.render_between_time_constraint,
TimeGranularity=TimeGranularity,
DatePart=DatePart,
render_date_sub=check_query_helpers.render_date_sub,
Expand Down

0 comments on commit 76ac912

Please sign in to comment.