abhishekagarwal87 commented on a change in pull request #12065:
URL: https://github.com/apache/druid/pull/12065#discussion_r777366385



##########
File path: sql/src/main/java/org/apache/druid/sql/calcite/rel/Grouping.java
##########
@@ -70,12 +71,25 @@ private Grouping(
       @Nullable final DimFilter havingFilter,
       final RowSignature outputRowSignature
   )
+  {
+    this(dimensions, subtotals, aggregations, havingFilter, 
outputRowSignature, false);
+  }
+
+  private Grouping(
+      final List<DimensionExpression> dimensions,
+      final Subtotals subtotals,
+      final List<Aggregation> aggregations,
+      @Nullable final DimFilter havingFilter,
+      final RowSignature outputRowSignature,
+      final boolean droppedDimensionsWhileApplyingProject
+  )
   {
     this.dimensions = ImmutableList.copyOf(dimensions);
     this.subtotals = Preconditions.checkNotNull(subtotals, "subtotals");
     this.aggregations = ImmutableList.copyOf(aggregations);
     this.havingFilter = havingFilter;
     this.outputRowSignature = Preconditions.checkNotNull(outputRowSignature, 
"outputRowSignature");
+    this.droppedDimensionsWhileApplyingProject = 
droppedDimensionsWhileApplyingProject;

Review comment:
       can we rename it to something else e.g. hadGroupingDimensions or 
groupDimensionsDropped or something? basically, we need not add 
`whileApplyingProject` here

##########
File path: sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
##########
@@ -13342,4 +13340,34 @@ public void 
testCommonVirtualExpressionWithDifferentValueType() throws Exception
         ImmutableList.of()
     );
   }
+
+  // When optimization in Grouping#applyProject is applied, and it reduces a 
Group By query to a timeseries, we
+  // want it to return empty bucket if no row matches
+  @Test
+  public void testReturnEmptyRowWhenGroupByIsConvertedToTimeseries() throws 
Exception

Review comment:
       can you add few more queries such as one that groups on a dummy dimension

##########
File path: sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
##########
@@ -13342,4 +13340,34 @@ public void 
testCommonVirtualExpressionWithDifferentValueType() throws Exception
         ImmutableList.of()
     );
   }
+
+  // When optimization in Grouping#applyProject is applied, and it reduces a 
Group By query to a timeseries, we
+  // want it to return empty bucket if no row matches
+  @Test
+  public void testReturnEmptyRowWhenGroupByIsConvertedToTimeseries() throws 
Exception
+  {
+    skipVectorize();
+    testQuery(
+        "SELECT 'A', dim1 from foo WHERE m1 = 50 AND dim1 = 'wat' GROUP BY 
dim1",

Review comment:
       btw I noticed that it runs as a group-by query if we remove the `m1 = 
50` clause. do you know why? 




-- 
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]

Reply via email to