kfaraz commented on a change in pull request #12047:
URL: https://github.com/apache/druid/pull/12047#discussion_r766306455
##########
File path: sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
##########
@@ -13342,4 +13343,45 @@ public void
testCommonVirtualExpressionWithDifferentValueType() throws Exception
ImmutableList.of()
);
}
+
+ @Test
+ public void testRemovalOfRedundantLiteralsInGroupBy() throws Exception
Review comment:
Maybe add some simpler test cases too where
a) the literal is not a part of the projection and should be removed
b) the literal is a part of the projection and should not be removed
##########
File path: sql/src/main/java/org/apache/druid/sql/calcite/rel/Grouping.java
##########
@@ -182,16 +185,31 @@ public Grouping applyProject(final PlannerContext
plannerContext, final Project
postAggregator ->
newAggregations.add(Aggregation.create(postAggregator))
);
+ final int[] newDimIndexes = new int[dimensions.size()];
+
// Remove literal dimensions that did not appear in the projection. This
is useful for queries
// like "SELECT COUNT(*) FROM tbl GROUP BY 'dummy'" which some tools can
generate, and for which we don't
// actually want to include a dimension 'dummy'.
- final ImmutableBitSet aggregateProjectBits =
RelOptUtil.InputFinder.bits(project.getChildExps(), null);
- final int[] newDimIndexes = new int[dimensions.size()];
+ Set<DruidExpression> literalsInInput = new HashSet<>();
Review comment:
If it's only a literal, could we just keep a Set of String here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
