Repository navigation
perf(partition): reduce consecutive Top-1 candidates before copying - #31001
Open
LeftHandCold wants to merge 2 commits into
Open
LeftHandCold wants to merge 2 commits into
LeftHandCold wants to merge 2 commits into
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
LeftHandCold
commented
Oct 24, 2026
LeftHandCold
left a comment
Collaborator
Author
There was a problem hiding this comment.
代码复核:建议通过,未发现需要 Request Changes 的确定性问题。审查 HEAD:6fa077ce1fa51ab2c1dab4d5773ee75163e80ff4。
设计判断:这是在原 Top-1 执行器内减少无效行复制的局部优化,复用原比较器、hash admission 和 retain/replace 所有权;没有新增缓存或第二套聚合状态。
逐项核对:
- 仅 limit=1 且非 WITH TIES 生效;连续同组片段局限在原来的 256 行 hash chunk,不要求输入排序,也不跨 batch 保存借用行。
- 同片段使用现有 ORDER 比较器,只在严格更优时更新,保留第一条相同排序键记录;核对 NULL、升降序、复合键及浮点 NaN/±0 的比较路径。
- 所有输入仍参与分组 hash;每组最终赢家仍交给原 retain/replace,payload/metadata 的复制与释放责任没有变化。K>1、WITH TIES 保持原分支。
- 阅读了独立稳定排序 oracle、chunk/batch 边界、交错组、多阶段、latest NULL payload、元数据和复用测试。对全不同组的额外工作是有界相邻 group-id 判断;没有发现新的分配或累计状态。
验证边界:已核对当前 HEAD 的成功 CI 40018738556,并完成 diff --check。本次没有重跑本机全套 UT 或 TSBS。PR 的 1.272s→0.115s 属于作者给定布局的实测,不能外推为任意窗口查询的收益。
当前认证账号也是 PR 作者,故以 COMMENT 记录建议通过;这不是 GitHub 正式 Approval。
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
Which issue(s) this PR fixes:
Fixes #31002.
What this PR does / why we need it:
For
ROW_NUMBER() ... rn = 1, an ordered group can repeatedly replace the retained row for every incoming row. On the original TSBS CPU-only latest-point query (8.64 million rows, 1,000 hosts),replaceRowaccounted for 89% of sampled CPU, mostly copying payload vectors and metadata.For Top-1 without ties, select the first best row in each consecutive same-group run within the existing hash chunk, then use the existing retain/replace path once. This needs no sorted-input assumption and adds no allocation or retained state. Existing SQL comparators preserve NULL, composite order and first-peer behavior; K > 1 and WITH TIES keep their existing path. Payload ownership, metadata copying, hash admission, cancellation and cleanup remain unchanged.
Validation:
Performance A/B uses the same frozen main base
b78794175e65f8cf5a4a4ac1fcd2715534f48552plus this patch. The delivery branch is rebased on156b7cc6f8; partition, comparator, vector, types and hashmap dependencies are unchanged between those bases. On the final clean delivery head, the full owningpartitionpackage, gofmt/diff checks,go vet, configured incrementalgolangci-lint, and native artifact provenance checks all pass.This is a draft for review. The measured gain applies to the stated CPU-only query and layout; DevOps/IoT and the broader AgentLogs/JSON comparison remain separate work.