[GitHub] [druid] gianm commented on a change in pull request #11959: SQL INSERT planner support.

[GitBox <gi...@apache.org>]

gianm commented on a change in pull request #11959:
URL: https://github.com/apache/druid/pull/11959#discussion_r756218455



##########
File path: sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidOuterQueryRel.java
##########
@@ -85,21 +84,6 @@ public PartialDruidQuery getPartialDruidQuery()
     return partialQuery;
   }
 
-  @Override
-  public Sequence<Object[]> runQuery()
-  {
-    // runQuery doesn't need to finalize aggregations, because the fact that runQuery is happening suggests this
-    // is the outermost query and it will actually get run as a native query. Druid's native query layer will
-    // finalize aggregations for the outermost query even if we don't explicitly ask it to.
-
-    final DruidQuery query = toDruidQuery(false);
-    if (query != null) {

Review comment:
       It can't be null; that was a hold-over from an earlier time when `null` was used by the old semi-join rel to mean "known to be empty". The new join rel doesn't work that way, since it pushes computation down into the native layer.




-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org