Skip to content

Commit 6cff341

Browse files
authored
[BugFix] Preserve SQL aggregate aliases in plan (#5509)
SELECT col, COUNT(*) AS cnt only renames a column without changing the set or order, so RelBuilder.project (force=false) skips emitting the LogicalProject and the alias is dropped. Force the projection when an AS renames a field; the PPL path is unaffected since it never produces an AS RexCall through visitProject. Signed-off-by: Chen Dai <daichen@amazon.com>
1 parent ea39ffd commit 6cff341

2 files changed

Lines changed: 71 additions & 1 deletion

File tree

api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlV2Test.java

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,57 @@ public void selectExpressionWithoutFrom() {
277277
""");
278278
}
279279

280+
@Test
281+
public void testGroupByAggregateAlias() {
282+
givenQuery(
283+
"""
284+
SELECT department, SUM(age) AS total FROM catalog.employees GROUP BY department
285+
""")
286+
.assertPlan(
287+
"""
288+
LogicalProject(department=[$0], total=[$1])
289+
LogicalAggregate(group=[{0}], SUM(age)=[SUM($1)])
290+
LogicalProject(department=[$3], age=[$2])
291+
LogicalTableScan(table=[[catalog, employees]])
292+
""");
293+
}
294+
295+
@Test
296+
public void testOrderByAggregateAlias() {
297+
givenQuery(
298+
"""
299+
SELECT department, COUNT(*) AS cnt FROM catalog.employees
300+
GROUP BY department ORDER BY cnt DESC LIMIT 3
301+
""")
302+
.assertPlan(
303+
"""
304+
LogicalSort(sort0=[$1], dir0=[DESC-nulls-last])
305+
LogicalProject(department=[$1], cnt=[$0])
306+
LogicalSort(sort0=[$0], dir0=[DESC-nulls-last], fetch=[3])
307+
LogicalProject(COUNT(*)=[$1], department=[$0])
308+
LogicalAggregate(group=[{0}], COUNT(*)=[COUNT()])
309+
LogicalProject(department=[$3])
310+
LogicalTableScan(table=[[catalog, employees]])
311+
""");
312+
}
313+
314+
@Test
315+
public void testAliasPreservedInOutputSchema() {
316+
givenQuery("SELECT COUNT(*) AS cnt FROM catalog.employees").assertFields("cnt");
317+
318+
givenQuery("SELECT department, COUNT(*) AS cnt FROM catalog.employees GROUP BY department")
319+
.assertFields("department", "cnt");
320+
321+
givenQuery("SELECT department, COUNT(*) FROM catalog.employees GROUP BY department")
322+
.assertFields("department", "COUNT(*)");
323+
324+
givenQuery("SELECT MAX(age) + MIN(age) AS range_sum FROM catalog.employees")
325+
.assertFields("range_sum");
326+
327+
givenQuery("SELECT id, name, age AS years, department FROM catalog.employees")
328+
.assertFields("id", "name", "years", "department");
329+
}
330+
280331
@Test
281332
public void testHavingMaxCol() {
282333
givenQuery(

core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -481,11 +481,30 @@ public RelNode visitProject(Project node, CalcitePlanContext context) {
481481
if (!context.isResolvingSubquery()) {
482482
context.setProjectVisited(true);
483483
}
484-
context.relBuilder.project(expandedFields);
484+
485+
// Force the projection on a rename: without it, Calcite omits the project node when the
486+
// columns are unchanged (same fields and order), so an alias like COUNT(*) AS cnt is lost.
487+
boolean force = isRenameFieldsProject(expandedFields, currentFields);
488+
context.relBuilder.project(expandedFields, ImmutableList.of(), force);
485489
}
486490
return context.relBuilder.peek();
487491
}
488492

493+
private static boolean isRenameFieldsProject(List<RexNode> fields, List<String> currentFields) {
494+
for (RexNode r : fields) {
495+
if (r.getKind() == AS) {
496+
RexCall as = (RexCall) r;
497+
if (as.getOperands().get(0) instanceof RexInputRef ref) {
498+
String name = ((RexLiteral) as.getOperands().get(1)).getValueAs(String.class);
499+
if (!name.equals(currentFields.get(ref.getIndex()))) {
500+
return true;
501+
}
502+
}
503+
}
504+
}
505+
return false;
506+
}
507+
489508
private boolean isSingleAllFieldsProject(Project node) {
490509
return node.getProjectList().size() == 1
491510
&& node.getProjectList().getFirst() instanceof AllFields;

0 commit comments

Comments
 (0)