scarlin-cloudera commented on code in PR #4442:
URL: https://github.com/apache/hive/pull/4442#discussion_r1286168676


##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/PlanModifierForASTConv.java:
##########
@@ -382,6 +387,11 @@ private static boolean validExchangeChild(HiveSortExchange 
sortNode) {
     return sortNode.getInput() instanceof Project;
   }
 
+  private static boolean validTableFunctionScanChild(HiveTableFunctionScan 
htfsNode) {
+    return htfsNode.getInputs().size() == 1 &&
+        (htfsNode.getInput(0) instanceof Project || htfsNode.getInput(0) 
instanceof HiveTableScan);

Review Comment:
   I found this code kinda weird as I went through it.  
   
   My observation, and perhaps it is wrong, is that this module makes up for 
some defects in the ASTConverter.  The ASTConverter requires that the level 
underneath it be a Project or HiveTableFunctionScan.  If it isn't, the caller 
"introduces" a Project so that the ASTConverter can handle it properly.
   
   I potentially misunderstood this code, but while I was testing, I hit this 
issue in the ASTConverter and this seems to follow the trend of already 
existing code.  I do think ASTConverter should be rewritten, but if we went 
that far, we should perhaps go to the physical operator directly rather than 
convert to AST and then convert to the physical operator.



-- 
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: gitbox-unsubscr...@hive.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org

Reply via email to