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