HIVE-29322: Avoid TopNKeyOperator When ReduceSink TopNkey Filtering Provides Better Pruning for ORDER BY LIMIT Queries

[Indhumathi27]

What changes were proposed in this pull request?

This PR updates TopNKeyProcessor to skip creating TopNKeyOperator when ReduceSinkDesc.topN is already set by LIMIT pushdown for ORDER BY LIMIT case. This prevents TopNKey from overriding pushdown and ensures the Reduce sink TopNkey filtering is used.

Why are the changes needed?

Currently, when a query includes ORDER BY + LIMIT, LIMIT pushdown is generated during planning but is effectively overridden by the subsequent TopNKey rewrite. As a result, TopNKey operator receives full input rather than a reduced data set, leading to worse performance (e.g., 16M rows forwarded to reducer instead of a few). In cases where global ordering uses a single reducer, LIMIT pushdown is sufficient and far more efficient. This fix prevents unnecessary TopNKey creation so that pushdown can reduce shuffle and significantly improve execution time.

Test reports:

For query: select * from table order by h limit 100;
Total num of rows: 67764224

with fix:

without fix:

Does this PR introduce any user-facing change?

No

How was this patch tested?

Manual testing + existing testcases

[Indhumathi27]

@kasakrisz @deniskuzZ Can you help to review this PR. Thanks

[Indhumathi27]

@ayushtkn @zabetak @kasakrisz @deniskuzZ Can you help to review the PR. thanks

[zabetak]

I left some questions under the JIRA ticket to better understand the issue that we are trying to solve here.

[Indhumathi27]

@zabetak @okumin Added the analysis in the jira. Please check

[zabetak]

I went over the plan changes and they look reasonable based on the goal of this PR. In terms of changes in the optimizer I left a few comments that could make the code a bit easier to follow.

[zabetak]

When the plan contains a Limit operator all subsequent TopN optimizations in the same mapper/reducer seem redundant. It's out of scope of the current PR but it may be worth logging a separate JIRA ticket as a potential improvement.

[zabetak]

It seems that we don't want to introduce a TopNKeyOperator if the path between TS and RS does not contain a JOIN or GBY operator. We can add such checks at some point here or maybe better modify the respective RuleRegExp to only trigger when there is a GBY or JOIN in the path.

[Indhumathi27]

@zabetak I have handled in TopNKeyProcessor to disable TopNkey for cases without JOIN/GROUP BY. can you look at the changes. Thanks

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[zabetak]

I have a feeling that the traversal logic here could be skipped by properly setting up the RuleRegExp in org.apache.hadoop.hive.ql.parse.TezCompiler#runTopNKeyOptimization.

Something like:

new RuleRegExp("Top n key optimization", "(GBY%|JOIN%).*RS%")

The expression above is not tested but I have the impression that with some tuning we can get the desired matching scope.