Skip to content

Commit

Permalink
Fix time constraints not being passed for saved queries.
Browse files Browse the repository at this point in the history
  • Loading branch information
plypaul committed Dec 9, 2023
1 parent c814d43 commit 24288ae
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 2 deletions.
2 changes: 2 additions & 0 deletions metricflow/engine/metricflow_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,8 @@ def _create_execution_plan(self, mf_query_request: MetricFlowQueryRequest) -> Me
else None
),
limit=mf_query_request.limit,
time_constraint_start=mf_query_request.time_constraint_start,
time_constraint_end=mf_query_request.time_constraint_end,
order_by_names=mf_query_request.order_by_names,
order_by_parameters=mf_query_request.order_by,
)
Expand Down
4 changes: 4 additions & 0 deletions metricflow/query/query_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ def parse_and_validate_saved_query(
saved_query_parameter: SavedQueryParameter,
where_filter: Optional[WhereFilter],
limit: Optional[int],
time_constraint_start: Optional[datetime.datetime],
time_constraint_end: Optional[datetime.datetime],
order_by_names: Optional[Sequence[str]],
order_by_parameters: Optional[Sequence[OrderByQueryParameter]],
) -> MetricFlowQuerySpec:
Expand All @@ -211,6 +213,8 @@ def parse_and_validate_saved_query(
for group_by_item_name in saved_query.query_params.group_by
),
where_constraint=merge_to_single_where_filter(PydanticWhereFilterIntersection(where_filters=where_filters)),
time_constraint_start=time_constraint_start,
time_constraint_end=time_constraint_end,
limit=limit,
order_by_names=order_by_names,
order_by=order_by_parameters,
Expand Down
14 changes: 12 additions & 2 deletions metricflow/test/cli/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def test_validate_configs(cli_context: CLIContext) -> None:
This test is special, because the CLI bypasses the semantic manifest read into the CLIContext and
validates the config files on disk. It's not entirely clear why we do this, so we should probably
figure that out and, if possible, stop doing it so we can have this test depend on an injectable
figure that out and, if possible, stop doing it so we can have this test depen on an injectable
in-memory semantic manifest instead of whatever is stored in the filesystem.
At any rate, due to the direct read from disk, we have to store a serialized semantic manifest
Expand Down Expand Up @@ -271,7 +271,17 @@ def test_saved_query_with_cumulative_metric( # noqa: D
sql_client: SqlClient,
) -> None:
resp = cli_runner.run(
query, args=["--saved-query", "saved_query_with_cumulative_metric", "--order", "metric_time__day"]
query,
args=[
"--saved-query",
"saved_query_with_cumulative_metric",
"--order",
"metric_time__day",
"--start-time",
"2020-01-01",
"--end-time",
"2020-01-01",
],
)

assert_object_snapshot_equal(
Expand Down

0 comments on commit 24288ae

Please sign in to comment.