[GitHub] [hive] scarlin-cloudera commented on a diff in pull request #3706: HIVE-26671: Incorrect results with Top N Key optimization

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

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

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


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