scarlin-cloudera commented on code in PR #3706:
URL: https://github.com/apache/hive/pull/3706#discussion_r1006962170
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/topnkey/TopNKeyPushdownProcessor.java:
##########
@@ -347,14 +347,12 @@ private void pushdownThroughTopNKey(TopNKeyOperator
topNKey) throws SemanticExce
if (topNKeyDesc.getKeyColumns().size() == commonKeyPrefix.size()) {
// TNK keys are subset of the parent TNK keys
pushdownThroughParent(topNKey);
- if (topNKey.getChildOperators().get(0).getType() ==
OperatorType.TOPNKEY) {
- LOG.debug("Removing {} since child {} supersedes it",
parent.getName(), topNKey.getName());
-
topNKey.getParentOperators().get(0).removeChildAndAdoptItsChildren(topNKey);
- }
+ LOG.debug("Removing {} since child {} supersedes it",
parent.getName(), topNKey.getName());
+
parent.getParentOperators().get(0).removeChildAndAdoptItsChildren(parent);
Review Comment:
I see what you're saying here and that does make sense.
The reason I removed it is because: While I think it's a valid check, is it
necessary?
This code assumes that while we are pushing down, we have the TNK -> TNK in
our subtree, but I'm not sure how that would be possible since the TNK is only
generated above a RS, and a TNK can only be a child (or parent) of a TNK while
it is temporarily going through this pushdown code, and it will always be
resolved by this pushdown code.
But I'm ok with keeping this code there for safety reasons.
--
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]
