gianm commented on a change in pull request #9986:
URL: https://github.com/apache/druid/pull/9986#discussion_r435531102
##########
File path: sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
##########
@@ -13419,6 +13420,73 @@ public void testNvlColumns() throws Exception
);
}
+ @Test
+ public void testGroupByWithLiteralInSubqueryGrouping() throws Exception
+ {
+ testQuery(
+ "SELECT \n"
+ + " t1, t2\n"
+ + " FROM\n"
+ + " ( SELECT\n"
+ + " 'dummy' as t1,\n"
+ + " CASE\n"
+ + " WHEN \n"
+ + " dim4 = 'b'\n"
+ + " THEN dim4\n"
+ + " ELSE NULL\n"
+ + " END AS t2\n"
+ + " FROM\n"
+ + " numfoo\n"
+ + " GROUP BY\n"
+ + " dim4\n"
+ + " )\n"
+ + " GROUP BY\n"
+ + " t1,t2\n",
+ ImmutableList.of(
+ GroupByQuery.builder()
+ .setDataSource(
+ GroupByQuery.builder()
+
.setDataSource(CalciteTests.DATASOURCE3)
+
.setInterval(querySegmentSpec(Filtration.eternity()))
+ .setGranularity(Granularities.ALL)
+ .setDimensions(new
DefaultDimensionSpec("dim4", "_d0", ValueType.STRING))
+ .setContext(QUERY_CONTEXT_DEFAULT)
+ .build()
+ )
+ .setVirtualColumns(
+ expressionVirtualColumn(
+ "v0",
+ "\'dummy\'",
+ ValueType.STRING
+ ),
+ expressionVirtualColumn(
+ "v1",
+ "case_searched((\"_d0\" == 'b'),\"_d0\",null)",
+ ValueType.STRING
+ )
+ )
.setInterval(querySegmentSpec(Filtration.eternity()))
Review comment:
This indentation is kind of weird. I'm not sure if checkstyle will flag
it, but watch out.
##########
File path: sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
##########
@@ -13419,6 +13420,73 @@ public void testNvlColumns() throws Exception
);
}
+ @Test
+ public void testGroupByWithLiteralInSubqueryGrouping() throws Exception
+ {
+ testQuery(
+ "SELECT \n"
+ + " t1, t2\n"
+ + " FROM\n"
+ + " ( SELECT\n"
+ + " 'dummy' as t1,\n"
+ + " CASE\n"
+ + " WHEN \n"
+ + " dim4 = 'b'\n"
+ + " THEN dim4\n"
+ + " ELSE NULL\n"
+ + " END AS t2\n"
+ + " FROM\n"
+ + " numfoo\n"
+ + " GROUP BY\n"
+ + " dim4\n"
+ + " )\n"
+ + " GROUP BY\n"
+ + " t1,t2\n",
+ ImmutableList.of(
+ GroupByQuery.builder()
+ .setDataSource(
+ GroupByQuery.builder()
+
.setDataSource(CalciteTests.DATASOURCE3)
+
.setInterval(querySegmentSpec(Filtration.eternity()))
+ .setGranularity(Granularities.ALL)
+ .setDimensions(new
DefaultDimensionSpec("dim4", "_d0", ValueType.STRING))
+ .setContext(QUERY_CONTEXT_DEFAULT)
+ .build()
+ )
+ .setVirtualColumns(
+ expressionVirtualColumn(
+ "v0",
+ "\'dummy\'",
+ ValueType.STRING
+ ),
+ expressionVirtualColumn(
+ "v1",
+ "case_searched((\"_d0\" == 'b'),\"_d0\",null)",
+ ValueType.STRING
+ )
+ )
.setInterval(querySegmentSpec(Filtration.eternity()))
+ .setDimensions(
+ dimensions(
+ new DefaultDimensionSpec("v0", "d0",
ValueType.STRING),
+ new DefaultDimensionSpec("v1", "d1",
ValueType.STRING)
+ )
+ )
+ .setGranularity(Granularities.ALL)
+ .setContext(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ NullHandling.replaceWithDefault() ?
+ ImmutableList.of(
+ new Object[]{"dummy", ""},
+ new Object[]{"dummy", "b"}
+ ) :
+ ImmutableList.of(
+ new Object[]{"dummy", null},
Review comment:
FYI: You could simplify this by using `NULL_STRING` for the null / ""
and avoiding the ternary.
##########
File path: sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java
##########
@@ -424,16 +425,39 @@ private static Grouping computeGrouping(
/**
* Builds a {@link Subtotals} object based on {@link
Aggregate#getGroupSets()}.
+ *
+ * @throws CannotBuildQueryException if subtotals cannot be computed
*/
private static Subtotals computeSubtotals(
final PartialDruidQuery partialQuery,
+ final PlannerContext plannerContext,
final RowSignature rowSignature
)
{
final Aggregate aggregate = partialQuery.getAggregate();
// dimBitMapping maps from input field position to group set position
(dimension number).
- final int[] dimBitMapping = new int[rowSignature.size()];
+ final int[] dimBitMapping;
+ if (partialQuery.getSelectProject() != null) {
+ int fieldCount = 0;
+ for (final RexNode rexNode :
partialQuery.getSelectProject().getChildExps()) {
+ final DruidExpression expression = Expressions.toDruidExpression(
+ plannerContext,
+ rowSignature,
+ rexNode
+ );
+
+ if (expression == null) {
+ throw new CannotBuildQueryException(partialQuery.getSelectProject(),
rexNode);
Review comment:
I think there's no need to do this check, because it isn't needed to
build the Subtotals object, and `computeDimensions` will handle the case where
we're grouping on something where dimensions are based on expressions that are
not translatable.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
