You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "terrymanu (via GitHub)" <gi...@apache.org> on 2026/08/23 04:58:44 UTC

[PR] Fix isSameGroupByAndOrderByItems to ignore order direction for group by and order by items [shardingsphere]

terrymanu opened a new pull request, #39574:
URL: https://github.com/apache/shardingsphere/pull/39574

   Judge group by and order by item sameness by select item identity, which is
   the resolved projection index or the segment text or column index, instead of
   the direction-sensitive OrderByItem equality. This keeps pagination rewrite
   using the original row count and group by stream merge eligible for queries
   like GROUP BY a, b ORDER BY a DESC, b DESC, and closes the latent wrong-result
   path where unresolved rewrite-time indexes made GROUP BY a ORDER BY SUM(c) ASC keep the per-shard limit.
   
   Fixes #23014


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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


Re: [PR] Fix isSameGroupByAndOrderByItems to ignore order direction for group by and order by items [shardingsphere]

Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu merged PR #39574:
URL: https://github.com/apache/shardingsphere/pull/39574


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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


Re: [PR] Fix isSameGroupByAndOrderByItems to ignore order direction for group by and order by items [shardingsphere]

Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu commented on PR #39574:
URL: https://github.com/apache/shardingsphere/pull/39574#issuecomment-5385051157

   ### Result
   
   **Review Result: Mergeable**
   
   No blocking issues passed the Finding Proof Gate. The same-key, different-order-direction scenario reported in #23014 is correctly fixed.
   
   ### Evidence
   
   - `SelectStatementBaseContext.java:369-441` accepts only identities proven by ordinals, complete bound-column information, or generated ordering. Expression collisions and explicit projection-alias ambiguities safely fall back.
   - `PaginationContext.java:230-239` preserves the per-shard Top-K limit of `110` for `OFFSET 100 / COUNT 10`; ASC/DESC does not affect the contiguity of identical grouping keys.
   - `ShardingDQLResultMerger.java:59-121` selects `GroupByStreamMergedResult` only after the grouping and ordering keys are proven identical.
   - Regression tests cover generated ordering, direction differences, expression/index collisions, parameter markers, quoted identifiers, projection-alias binding, pagination, and merge selection.
   - Local verification passed: Binder 45/45, SQL rewrite IT 773/773, and merge tests 37/37. Spotless and Checkstyle also passed.
   - Public requirements: [PR #39574](https://github.com/apache/shardingsphere/pull/39574) and [Issue #23014](https://github.com/apache/shardingsphere/issues/23014).
   
   ### Coverage
   
   - Reviewed head: `9f19c7b0b79d0fdee562226f0c735f926ce57bb4`
   - Review base: `103f81d84e09e23d2ee023edbb9e5b441063d6ba`
   - All six files in the GROUP BY/ORDER BY fix cluster were reviewed, and the GitHub file list matched the local triple-dot scope.
   - Root-cause, blast-radius, tests/runtime, and final convergence reviews completed with no unresolved evidence gaps.
   - This is a code-correctness-only result. Doris changes were excluded by explicit scope, and GitHub Actions were not reviewed.


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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