Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] bug in how AVG is decomposed into measures #1299

Merged
merged 6 commits into from
Feb 1, 2025
Merged

[fix] bug in how AVG is decomposed into measures #1299

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dd54bc2
[fix] bug in how AVG is decomposed into measures.
anhqle Jan 31, 2025
f190125
lint fixes to pass make check
anhqle Jan 31, 2025
9363cf7
Revert "lint fixes to pass make check"
anhqle Jan 31, 2025
b0dfb17
update test
anhqle Jan 31, 2025
dbfd71a
lint fix
anhqle Jan 31, 2025
7d79945
Fix measures SQL test
shangyian Feb 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions datajunction-server/datajunction_server/sql/decompose.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,15 +151,12 @@ def _avg(self, func) -> list[Measure]:
of the selected measure).
"""
arg = func.args[0]
measure_name = "_".join(
[str(col) for col in arg.find_all(ast.Column)]
+ [dj_functions.Sum.__name__.lower()],
)
measure_name = "_".join([str(col) for col in arg.find_all(ast.Column)])
expression = str(arg)
short_hash = hashlib.md5(expression.encode("utf-8")).hexdigest()[:8]
return [
Measure(
name=f"{measure_name}_{short_hash}",
name=f"{measure_name}_{dj_functions.Sum.__name__.lower()}_{short_hash}",
expression=expression,
aggregation=dj_functions.Sum.__name__.upper(),
rule=AggregationRule(
Expand All @@ -169,8 +166,8 @@ def _avg(self, func) -> list[Measure]:
),
),
Measure(
name="count",
expression="1",
name=f"{measure_name}_{dj_functions.Count.__name__.lower()}_{short_hash}",
expression=expression,
aggregation=dj_functions.Count.__name__.upper(),
rule=AggregationRule(
type=Aggregability.FULL
Expand Down
6 changes: 3 additions & 3 deletions datajunction-server/tests/api/graphql/measures_sql_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,10 @@ async def test_measures_sql(
"semanticType": "MEASURE",
},
{
"name": "count",
"name": "price_count_78a5eb43",
"semanticEntity": {
"column": "count",
"name": "default.repair_orders_fact.count",
"column": "price_count_78a5eb43",
"name": "default.repair_orders_fact.price_count_78a5eb43",
"node": "default.repair_orders_fact",
},
"semanticType": "MEASURE",
Expand Down
24 changes: 12 additions & 12 deletions datajunction-server/tests/api/sql_v2_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ async def test_measures_sql_with_filters__v2( # pylint: disable=too-many-argume
SELECT
default_DOT_repair_orders_fact_built.default_DOT_dispatcher_DOT_dispatcher_id,
SUM(price * discount) AS price_discount_sum_017d55a8,
COUNT(1) AS count
COUNT(price * discount) AS price_discount_count_017d55a8
FROM default_DOT_repair_orders_fact_built
GROUP BY default_DOT_repair_orders_fact_built.default_DOT_dispatcher_DOT_dispatcher_id
""",
Expand All @@ -660,10 +660,10 @@ async def test_measures_sql_with_filters__v2( # pylint: disable=too-many-argume
"type": "double",
},
{
"column": "count",
"name": "count",
"column": "price_discount_count_017d55a8",
"name": "price_discount_count_017d55a8",
"node": "default.repair_orders_fact",
"semantic_entity": "default.repair_orders_fact.count",
"semantic_entity": "default.repair_orders_fact.price_discount_count_017d55a8",
"semantic_type": "measure",
"type": "bigint",
},
Expand Down Expand Up @@ -762,7 +762,7 @@ async def test_measures_sql_with_filters__v2( # pylint: disable=too-many-argume
default_DOT_repair_orders_fact_built.default_DOT_us_state_DOT_state_name,
default_DOT_repair_orders_fact_built.default_DOT_dispatcher_DOT_company_name,
default_DOT_repair_orders_fact_built.default_DOT_hard_hat_DOT_last_name,
COUNT(1) AS count,
COUNT(CAST(time_to_dispatch AS INT)) AS time_to_dispatch_count_bf99afd6,
SUM(CAST(time_to_dispatch AS INT)) AS time_to_dispatch_sum_bf99afd6,
SUM(total_repair_cost) AS total_repair_cost_sum_9bdaf803
FROM default_DOT_repair_orders_fact_built
Expand Down Expand Up @@ -797,10 +797,10 @@ async def test_measures_sql_with_filters__v2( # pylint: disable=too-many-argume
"type": "string",
},
{
"column": "count",
"name": "count",
"column": "time_to_dispatch_count_bf99afd6",
"name": "time_to_dispatch_count_bf99afd6",
"node": "default.repair_orders_fact",
"semantic_entity": "default.repair_orders_fact.count",
"semantic_entity": "default.repair_orders_fact.time_to_dispatch_count_bf99afd6",
"semantic_type": "measure",
"type": "bigint",
},
Expand Down Expand Up @@ -891,7 +891,7 @@ async def test_measures_sql_with_filters__v2( # pylint: disable=too-many-argume
SELECT
default_DOT_repair_orders_fact_built.default_DOT_dispatcher_DOT_company_name,
default_DOT_repair_orders_fact_built.default_DOT_hard_hat_DOT_last_name,
COUNT(1) AS count,
COUNT(CAST(time_to_dispatch AS INT)) AS time_to_dispatch_count_bf99afd6,
SUM(CAST(time_to_dispatch AS INT)) AS time_to_dispatch_sum_bf99afd6
FROM default_DOT_repair_orders_fact_built
GROUP BY
Expand Down Expand Up @@ -971,7 +971,7 @@ async def test_measures_sql_with_filters__v2( # pylint: disable=too-many-argume
SELECT
default_DOT_repair_orders_fact_built.default_DOT_dispatcher_DOT_company_name,
default_DOT_repair_orders_fact_built.default_DOT_hard_hat_DOT_last_name,
COUNT(1) AS count,
COUNT(CAST(time_to_dispatch AS INT)) AS time_to_dispatch_count_bf99afd6,
SUM(CAST(time_to_dispatch AS INT)) AS time_to_dispatch_sum_bf99afd6
FROM default_DOT_repair_orders_fact_built
GROUP BY
Expand Down Expand Up @@ -1326,7 +1326,7 @@ async def test_measures_sql_preagg_incompatible( # pylint: disable=too-many-arg
)
SELECT
default_DOT_repair_orders_fact_built.default_DOT_dispatcher_DOT_company_name,
COUNT(1) AS count,
COUNT(price) AS price_count_78a5eb43,
SUM(price) AS price_sum_78a5eb43,
COUNT(repair_order_id) AS repair_order_id_count_0b7dfba0
FROM default_DOT_repair_orders_fact_built
Expand Down Expand Up @@ -1505,7 +1505,7 @@ async def test_measures_sql_local_dimensions(
)
SELECT
default_DOT_hard_hat_built.default_DOT_hard_hat_DOT_hire_date,
COUNT(1) AS count,
COUNT(CAST(NOW() AS DATE) - default_DOT_hard_hat_DOT_hire_date) AS hire_date_count_9b06ca5d,
SUM(CAST(NOW() AS DATE) - default_DOT_hard_hat_DOT_hire_date) AS hire_date_sum_9b06ca5d
FROM default_DOT_hard_hat_built
GROUP BY
Expand Down
15 changes: 9 additions & 6 deletions datajunction-server/tests/sql/decompose_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,8 @@ def test_average():

expected_measures = [
Measure(
name="count",
expression="1",
name="sales_amount_count_a1b27bc7",
expression="sales_amount",
aggregation="COUNT",
rule=AggregationRule(type=Aggregability.FULL),
),
Expand All @@ -222,7 +222,10 @@ def test_average():
]
assert measures == expected_measures
assert str(derived_sql) == str(
parse("SELECT SUM(sales_amount_sum_a1b27bc7) / SUM(count) FROM parent_node"),
parse(
"SELECT SUM(sales_amount_sum_a1b27bc7) / "
"SUM(sales_amount_count_a1b27bc7) FROM parent_node",
),
)


Expand Down Expand Up @@ -604,8 +607,8 @@ def test_metric_query_with_aliases():
measures, derived_sql = extractor.extract()
expected_measures = [
Measure(
name="count",
expression="1",
name="time_to_dispatch_count_bf99afd6",
expression="CAST(time_to_dispatch AS INT)",
aggregation="COUNT",
rule=AggregationRule(type=Aggregability.FULL, level=None),
),
Expand All @@ -620,6 +623,6 @@ def test_metric_query_with_aliases():
assert str(derived_sql) == str(
parse(
"SELECT SUM(time_to_dispatch_sum_bf99afd6) / "
"SUM(count) FROM default.repair_orders_fact",
"SUM(time_to_dispatch_count_bf99afd6) FROM default.repair_orders_fact",
),
)