You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "kgyrtkirk (via GitHub)" <gi...@apache.org> on 2024/08/22 10:52:55 UTC

[PR] Planning could have failed if UNION ALL operator was completely removed (druid)

kgyrtkirk opened a new pull request, #16946:
URL: https://github.com/apache/druid/pull/16946

   For a query in which the *UNION* could be removed; the planning may lead to an exception - because after the removal of the union to retain the rowType of the union; a `Project` is added - which just casts the columns to the right types.
   If the remaining branch of the union selects some `null`-s the incoming type will be undefined.
   
   ```sql
   SELECT COUNT(*), "channel" FROM "wikipedia"
   	WHERE false
   	GROUP BY "channel"
   UNION ALL
   SELECT null,null;
   ```
   
   After optimization
   ```
   DruidProject(EXPR$0=[CAST($0):BIGINT], channel=[CAST($1):VARCHAR], druid=[logical]): rowcount = 1.0, cumulative cost = {1.0 rows, 1.0E-6 cpu, 0.0 io}, id = 355
     DruidValues(tuples=[[{ null, null }]]): rowcount = 1.0, cumulative cost = {1.0 rows, 0.0 cpu, 0.0 io}, id = 352
   ```
   
   <details>
   <summary>Exception</summary>
   
   ```
   2024-08-22T10:43:09,169 ERROR [qtp533769848-25] org.apache.druid.sql.avatica.DruidMeta - Cannot translate reference[CAST($0):BIGINT] from rel[rel#2315:DruidProject.DRUID_LOGICAL.[](input=DruidValues#2312,exprs=[CAST($0):BIGINT, CAST($1):VARCHAR],druid=logical)]
   org.apache.druid.error.DruidException: Cannot translate reference[CAST($0):BIGINT] from rel[rel#2315:DruidProject.DRUID_LOGICAL.[](input=DruidValues#2312,exprs=[CAST($0):BIGINT, CAST($1):VARCHAR],druid=logical)]
   	at org.apache.druid.sql.calcite.planner.DruidPlanner.translateException(DruidPlanner.java:395) ~[classes/:?]
   	at org.apache.druid.sql.calcite.planner.QueryHandler.plan(QueryHandler.java:258) ~[classes/:?]
   	at org.apache.druid.sql.calcite.planner.DruidPlanner.plan(DruidPlanner.java:245) ~[classes/:?]
   	at org.apache.druid.sql.DirectStatement.createPlan(DirectStatement.java:255) ~[classes/:?]
   	at org.apache.druid.sql.DirectStatement.plan(DirectStatement.java:223) ~[classes/:?]
   	at org.apache.druid.sql.DirectStatement.execute(DirectStatement.java:184) ~[classes/:?]
   	at org.apache.druid.sql.avatica.DruidJdbcResultSet.lambda$0(DruidJdbcResultSet.java:239) ~[classes/:?]
   	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264) ~[?:?]
   	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) ~[?:?]
   	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) ~[?:?]
   	at java.base/java.lang.Thread.run(Thread.java:829) [?:?]
   Caused by: org.apache.druid.sql.calcite.rel.CannotBuildQueryException: Cannot translate reference[CAST($0):BIGINT] from rel[rel#2315:DruidProject.DRUID_LOGICAL.[](input=DruidValues#2312,exprs=[CAST($0):BIGINT, CAST($1):VARCHAR],druid=logical)]
   	at org.apache.druid.sql.calcite.rel.Projection.preAggregation(Projection.java:243) ~[classes/:?]
   	at org.apache.druid.sql.calcite.rel.DruidQuery.computeSelectProjection(DruidQuery.java:382) ~[classes/:?]
   	at org.apache.druid.sql.calcite.rel.DruidQuery.fromPartialQuery(DruidQuery.java:246) ~[classes/:?]
   	at org.apache.druid.sql.calcite.rel.PartialDruidQuery.build(PartialDruidQuery.java:525) ~[classes/:?]
   	at org.apache.druid.sql.calcite.planner.querygen.DruidQueryGenerator$PDQVertexFactory$PDQVertex.buildQuery(DruidQueryGenerator.java:170) ~[classes/:?]
   	at org.apache.druid.sql.calcite.planner.querygen.DruidQueryGenerator.buildQuery(DruidQueryGenerator.java:66) ~[classes/:?]
   	at org.apache.druid.sql.calcite.planner.QueryHandler.planWithDruidConvention(QueryHandler.java:572) ~[classes/:?]
   	at org.apache.druid.sql.calcite.planner.QueryHandler$SelectHandler.planForDruid(QueryHandler.java:746) ~[classes/:?]
   	at org.apache.druid.sql.calcite.planner.QueryHandler.plan(QueryHandler.java:225) ~[classes/:?]
   	... 9 more
   ```
   </details>


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


Re: [PR] Planning could have failed if UNION ALL operator was completely removed (druid)

Posted by "clintropolis (via GitHub)" <gi...@apache.org>.
clintropolis commented on code in PR #16946:
URL: https://github.com/apache/druid/pull/16946#discussion_r1745208899


##########
sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/CastOperatorConversion.java:
##########
@@ -92,10 +92,13 @@ public DruidExpression toDruidExpression(
                                               ? ExpressionType.fromColumnType(fromDruidType)
                                               : ExpressionType.fromColumnType(toDruidType);
 
-      if (fromExpressionType == null || toExpressionType == null) {
+      if (toExpressionType == null) {
         // We have no runtime type for these SQL types.
         return null;
       }
+      if (fromExpressionType == null) {
+        return DruidExpression.ofLiteral(toDruidType, DruidExpression.nullLiteral());
+      }

Review Comment:
   this isn't really a cool change to make as is at least, `Calcites.getColumnTypeForRelDataType` returns null for any SQL type we are unable to convert to a native druid type, it doesn't necessarily mean that the type is a null literal. see #17011



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


Re: [PR] Planning could have failed if UNION ALL operator was completely removed (druid)

Posted by "rohangarg (via GitHub)" <gi...@apache.org>.
rohangarg merged PR #16946:
URL: https://github.com/apache/druid/pull/16946


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