Skip to content

Commit e53f4e9

Browse files
Alison GimCopilot
andcommitted
Split extend into pre/post-summarize for calculated columns vs measures
Calculated columns (non-aggregate expressions like strcat, iff) now get their extend clause placed BEFORE summarize so they exist for group-by. Calculated measures (expressions referencing aggregate aliases) remain AFTER summarize where they can reference aggregated values. Tracks all aggregate aliases in a separate set to avoid overwrites when multiple measures use the same aggregate expression (e.g. two count(*)). Adds test_pre_aggregated_calculations from PR review comment to verify calculated columns used as group-by dimensions work correctly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent c72ee61 commit e53f4e9

2 files changed

Lines changed: 71 additions & 23 deletions

File tree

sqlalchemy_kusto/dialect_kql.py

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -142,13 +142,10 @@ def visit_select(
142142
)
143143
compiled_query_lines.append(f"| where {converted_where_clause}")
144144

145-
# Add summarize first if it exists
146-
if "summarize" in projections_parts_dict:
147-
compiled_query_lines.append(projections_parts_dict.pop("summarize"))
148-
149-
# Then add extend after summarize
150-
if "extend" in projections_parts_dict:
151-
compiled_query_lines.append(projections_parts_dict.pop("extend"))
145+
# Add clauses in correct order: pre-extend, summarize, post-extend
146+
for key in ("pre_extend", "summarize", "post_extend"):
147+
if key in projections_parts_dict:
148+
compiled_query_lines.append(projections_parts_dict.pop(key))
152149

153150
# Add remaining parts (project, sort)
154151
for statement_part in projections_parts_dict.values():
@@ -204,7 +201,8 @@ def _get_projection_or_summarize(self, select: selectable.Select) -> dict[str, s
204201
group_by_cols = select._group_by_clauses
205202
order_by_cols = select._order_by_clauses
206203
summarize_statement = ""
207-
extend_statement = ""
204+
pre_extend_statement = ""
205+
post_extend_statement = ""
208206
project_statement = ""
209207
has_aggregates = False
210208
# The following is the logic
@@ -219,8 +217,10 @@ def _get_projection_or_summarize(self, select: selectable.Select) -> dict[str, s
219217
# N---> Add to projection
220218
if columns is not None:
221219
summarize_columns = set()
222-
extend_columns = set()
220+
pre_extend_columns = []
221+
post_extend_columns = []
223222
projection_columns = []
223+
all_agg_aliases: set[str] = set()
224224
for column in [c for c in columns if c.name != "*"]:
225225
column_name, column_alias = self._extract_column_name_and_alias(column)
226226
column_alias = self._escape_and_quote_columns(column_alias, True)
@@ -232,13 +232,26 @@ def _get_projection_or_summarize(self, select: selectable.Select) -> dict[str, s
232232
summarize_columns.add(
233233
self._build_column_projection(kql_agg, column_alias)
234234
)
235+
if column_alias:
236+
all_agg_aliases.add(column_alias)
235237
# No group by clause
236238
# Do the columns have aliases ?
237239
# Add additional and to handle case where : SELECT column_name as column_name
238240
elif column_alias and column_alias != column_name:
239-
extend_columns.add(
240-
self._build_column_projection(column_name, column_alias, True)
241+
extend_entry = self._build_column_projection(
242+
column_name, column_alias, True
243+
)
244+
escaped_expr = self._escape_and_quote_columns(column_name)
245+
246+
# Calculated measures (aggregate-dependent) go after summarize;
247+
# calculated columns (no aggregate dependency) go before summarize
248+
target = (
249+
post_extend_columns
250+
if any(alias in escaped_expr for alias in all_agg_aliases)
251+
else pre_extend_columns
241252
)
253+
target.append(extend_entry)
254+
242255
if column_alias:
243256
projection_columns.append(
244257
self._escape_and_quote_columns(column_alias, True)
@@ -257,8 +270,10 @@ def _get_projection_or_summarize(self, select: selectable.Select) -> dict[str, s
257270
summarize_statement = (
258271
f"{summarize_statement} by {', '.join(by_columns)}"
259272
)
260-
if extend_columns:
261-
extend_statement = f"| extend {', '.join(sorted(extend_columns))}"
273+
if pre_extend_columns:
274+
pre_extend_statement = f"| extend {', '.join(pre_extend_columns)}"
275+
if post_extend_columns:
276+
post_extend_statement = f"| extend {', '.join(post_extend_columns)}"
262277
project_statement = (
263278
f"| project {', '.join(projection_columns)}"
264279
if projection_columns
@@ -269,7 +284,8 @@ def _get_projection_or_summarize(self, select: selectable.Select) -> dict[str, s
269284
f"| order by {', '.join(unwrapped_order_by)}" if unwrapped_order_by else ""
270285
)
271286
return {
272-
"extend": extend_statement,
287+
"pre_extend": pre_extend_statement,
288+
"post_extend": post_extend_statement,
273289
"summarize": summarize_statement,
274290
"project": project_statement,
275291
"sort": sort_statement,

tests/unit/test_dialect_kql.py

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -173,12 +173,12 @@ def test_group_by_text():
173173
query_compiled = str(
174174
query.compile(engine, compile_kwargs={"literal_binds": True})
175175
).replace("\n", "")
176-
# raw query text from query
176+
# raw query text from query - order matches column appearance
177177
query_expected = (
178178
'["ActiveUsersLastMonth"]'
179+
'| extend ["EventInfo_Time"] = ["EventInfo_Time"] / time(1d), '
180+
'["ActiveUserMetric"] = ["ActiveUsers"]'
179181
'| summarize by ["EventInfo_Time"] / time(1d)'
180-
'| extend ["ActiveUserMetric"] = ["ActiveUsers"], '
181-
'["EventInfo_Time"] = ["EventInfo_Time"] / time(1d)'
182182
'| project ["EventInfo_Time"], ["ActiveUserMetric"]'
183183
'| order by ["ActiveUserMetric"] desc'
184184
)
@@ -202,12 +202,13 @@ def test_function_text(f: str, expected: str):
202202
query_compiled = str(
203203
query.compile(engine, compile_kwargs={"literal_binds": True})
204204
).replace("\n", "")
205+
# Order matches column appearance in select
205206
query_expected = (
206207
'["ActiveUsersLastMonth"]'
207-
'| extend ["ActiveUserMetric"] = ["ActiveUsers"], '
208-
'["EventInfo_Time"] = '
208+
'| extend ["EventInfo_Time"] = '
209209
+ expected
210-
+ '| project ["EventInfo_Time"], ["ActiveUserMetric"]'
210+
+ ', ["ActiveUserMetric"] = ["ActiveUsers"]'
211+
'| project ["EventInfo_Time"], ["ActiveUserMetric"]'
211212
)
212213
assert query_compiled == query_expected
213214

@@ -226,8 +227,8 @@ def test_group_by_text_vaccine_dataset():
226227
).replace("\n", "")
227228
query_expected = (
228229
'database("superset").["CovidVaccineData"]'
229-
'| summarize by ["country_name"]'
230230
'| extend ["country_name"] = ["country_name"]'
231+
'| summarize by ["country_name"]'
231232
'| project ["country_name"]'
232233
'| order by ["country_name"] asc'
233234
)
@@ -328,8 +329,8 @@ def test_distinct_count_by_text():
328329
# raw query text from query
329330
query_expected = (
330331
'["ActiveUsersLastMonth"]'
331-
'| summarize ["DistinctUsers"] = dcount(["ActiveUsers"]) by ["EventInfo_Time"] / time(1d)'
332332
'| extend ["EventInfo_Time"] = ["EventInfo_Time"] / time(1d)'
333+
'| summarize ["DistinctUsers"] = dcount(["ActiveUsers"]) by ["EventInfo_Time"] / time(1d)'
333334
'| project ["EventInfo_Time"], ["DistinctUsers"]'
334335
'| order by ["ActiveUserMetric"] desc'
335336
)
@@ -354,8 +355,8 @@ def test_distinct_count_alt_by_text():
354355
# raw query text from query
355356
query_expected = (
356357
'["ActiveUsersLastMonth"]'
357-
'| summarize ["DistinctUsers"] = dcount(["ActiveUsers"]) by ["EventInfo_Time"] / time(1d)'
358358
'| extend ["EventInfo_Time"] = ["EventInfo_Time"] / time(1d)'
359+
'| summarize ["DistinctUsers"] = dcount(["ActiveUsers"]) by ["EventInfo_Time"] / time(1d)'
359360
'| project ["EventInfo_Time"], ["DistinctUsers"]'
360361
'| order by ["ActiveUserMetric"] desc'
361362
)
@@ -571,6 +572,37 @@ def test_calculated_measure_with_adhoc_measure_and_constant():
571572
assert query_compiled == query_expected
572573

573574

575+
def test_pre_aggregated_calculations():
576+
"""Test from PR #48 review: calculated column before aggregation.
577+
578+
A calculated column (strcat) is created and used as a group-by dimension.
579+
The extend must appear before summarize so the column exists for grouping.
580+
"""
581+
app_col = literal_column("App")
582+
ns_col = literal_column("Namespace")
583+
id_col = literal_column("_id")
584+
585+
app_namespace = sa.func.strcat(app_col, ns_col).label("App_Namespace")
586+
587+
query = (
588+
select(app_namespace, sa.func.count(id_col).label("TotalLogs"))
589+
.select_from(text("Logs"))
590+
.group_by(app_namespace)
591+
)
592+
593+
query_compiled = str(
594+
query.compile(engine, compile_kwargs={"literal_binds": True})
595+
).replace("\n", "")
596+
597+
query_expected = (
598+
'["Logs"]'
599+
'| extend ["App_Namespace"] = strcat(App, Namespace)'
600+
'| summarize ["TotalLogs"] = count(["_id"]) by ["App_Namespace"]'
601+
'| project ["App_Namespace"], ["TotalLogs"]'
602+
)
603+
assert query_compiled == query_expected
604+
605+
574606
@pytest.mark.parametrize(
575607
("query_table_name", "expected_table_name"),
576608
[

0 commit comments

Comments
 (0)