Skip to content

Commit e305772

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 e305772

2 files changed

Lines changed: 73 additions & 20 deletions

File tree

sqlalchemy_kusto/dialect_kql.py

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

145-
# Add summarize first if it exists
145+
# Add pre-extend (calculated columns) before summarize
146+
if "pre_extend" in projections_parts_dict:
147+
compiled_query_lines.append(projections_parts_dict.pop("pre_extend"))
148+
149+
# Add summarize
146150
if "summarize" in projections_parts_dict:
147151
compiled_query_lines.append(projections_parts_dict.pop("summarize"))
148152

149-
# Then add extend after summarize
150-
if "extend" in projections_parts_dict:
151-
compiled_query_lines.append(projections_parts_dict.pop("extend"))
153+
# Add post-extend (calculated measures) after summarize
154+
if "post_extend" in projections_parts_dict:
155+
compiled_query_lines.append(projections_parts_dict.pop("post_extend"))
152156

153157
# Add remaining parts (project, sort)
154158
for statement_part in projections_parts_dict.values():
@@ -204,7 +208,8 @@ def _get_projection_or_summarize(self, select: selectable.Select) -> dict[str, s
204208
group_by_cols = select._group_by_clauses
205209
order_by_cols = select._order_by_clauses
206210
summarize_statement = ""
207-
extend_statement = ""
211+
pre_extend_statement = ""
212+
post_extend_statement = ""
208213
project_statement = ""
209214
has_aggregates = False
210215
# The following is the logic
@@ -219,8 +224,10 @@ def _get_projection_or_summarize(self, select: selectable.Select) -> dict[str, s
219224
# N---> Add to projection
220225
if columns is not None:
221226
summarize_columns = set()
222-
extend_columns = set()
227+
pre_extend_columns = []
228+
post_extend_columns = []
223229
projection_columns = []
230+
all_agg_aliases: set[str] = set()
224231
for column in [c for c in columns if c.name != "*"]:
225232
column_name, column_alias = self._extract_column_name_and_alias(column)
226233
column_alias = self._escape_and_quote_columns(column_alias, True)
@@ -232,13 +239,24 @@ def _get_projection_or_summarize(self, select: selectable.Select) -> dict[str, s
232239
summarize_columns.add(
233240
self._build_column_projection(kql_agg, column_alias)
234241
)
242+
if column_alias:
243+
all_agg_aliases.add(column_alias)
235244
# No group by clause
236245
# Do the columns have aliases ?
237246
# Add additional and to handle case where : SELECT column_name as column_name
238247
elif column_alias and column_alias != column_name:
239-
extend_columns.add(
240-
self._build_column_projection(column_name, column_alias, True)
248+
extend_entry = self._build_column_projection(
249+
column_name, column_alias, True
241250
)
251+
escaped_expr = self._escape_and_quote_columns(column_name)
252+
253+
# Calculated measures (aggregate-dependent) go after summarize;
254+
# calculated columns (no aggregate dependency) go before summarize
255+
if any(alias in escaped_expr for alias in all_agg_aliases):
256+
post_extend_columns.append(extend_entry)
257+
else:
258+
pre_extend_columns.append(extend_entry)
259+
242260
if column_alias:
243261
projection_columns.append(
244262
self._escape_and_quote_columns(column_alias, True)
@@ -257,8 +275,10 @@ def _get_projection_or_summarize(self, select: selectable.Select) -> dict[str, s
257275
summarize_statement = (
258276
f"{summarize_statement} by {', '.join(by_columns)}"
259277
)
260-
if extend_columns:
261-
extend_statement = f"| extend {', '.join(sorted(extend_columns))}"
278+
if pre_extend_columns:
279+
pre_extend_statement = f"| extend {', '.join(pre_extend_columns)}"
280+
if post_extend_columns:
281+
post_extend_statement = f"| extend {', '.join(post_extend_columns)}"
262282
project_statement = (
263283
f"| project {', '.join(projection_columns)}"
264284
if projection_columns
@@ -269,7 +289,8 @@ def _get_projection_or_summarize(self, select: selectable.Select) -> dict[str, s
269289
f"| order by {', '.join(unwrapped_order_by)}" if unwrapped_order_by else ""
270290
)
271291
return {
272-
"extend": extend_statement,
292+
"pre_extend": pre_extend_statement,
293+
"post_extend": post_extend_statement,
273294
"summarize": summarize_statement,
274295
"project": project_statement,
275296
"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)