Skip to content

[codex] Preserve fast grouper key order#82

Draft
zanmato1984 wants to merge 8 commits into
mainfrom
codex/group-key-ordering-check
Draft

[codex] Preserve fast grouper key order#82
zanmato1984 wants to merge 8 commits into
mainfrom
codex/group-key-ordering-check

Conversation

@zanmato1984

@zanmato1984 zanmato1984 commented Jul 14, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add focused row grouper coverage for first-seen key order on string keys.
  • Preserve first-seen group id order inside SwissTable new-key insertion when a pending stamp match requires equality comparison.
  • Avoid post-processing GetUniques() with a Take; the row table is populated in the intended group id order directly.

Validation

  • cmake --build .build-choose-test --target arrow-compute-row-test -j8
  • .build-choose-test/release/arrow-compute-row-test
  • cmake --build .build-choose-test --target arrow-compute-aggregate-test -j8
  • .build-choose-test/release/arrow-compute-aggregate-test

@zanmato1984
zanmato1984 force-pushed the codex/group-key-ordering-check branch from d361b5f to 8297c7e Compare July 14, 2026 20:06
@zanmato1984 zanmato1984 changed the title [codex] Add group key ordering coverage [codex] Preserve fast grouper key order Jul 14, 2026
@zanmato1984
zanmato1984 force-pushed the codex/group-key-ordering-check branch from 949c8b7 to 4a448e0 Compare July 14, 2026 20:31
@github-actions

Copy link
Copy Markdown

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

@zanmato1984
zanmato1984 force-pushed the codex/group-key-ordering-check branch from 26b59fd to 2d893ae Compare July 15, 2026 18:23
Signed-off-by: Rossi Sun <zanmato1984@gmail.com>
Signed-off-by: Rossi Sun <zanmato1984@gmail.com>
Signed-off-by: Rossi Sun <zanmato1984@gmail.com>
@zanmato1984
zanmato1984 force-pushed the codex/group-key-ordering-check branch from 2d893ae to 98fb89e Compare July 15, 2026 18:23
Signed-off-by: Rossi Sun <zanmato1984@gmail.com>
Signed-off-by: Rossi Sun <zanmato1984@gmail.com>
Signed-off-by: Rossi Sun <zanmato1984@gmail.com>
Signed-off-by: Rossi Sun <zanmato1984@gmail.com>
Signed-off-by: Rossi Sun <zanmato1984@gmail.com>
Comment on lines +590 to +595
if (num_inserted_ + num_pending_appends == num_groups_limit) {
// 中文说明:跳出当前 key 的 probing 循环,外层会保留剩余 selection。
break;
}
// 中文说明:当前 key 已作为新 group 处理完,继续下一个 selection 项。
break;

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这两个break不是重复了?上面的if没必要存在吧?

// 中文说明:接收“不相等”的数量;0 表示当前 key 命中已有 group。
int num_not_equal;
// 中文说明:只比较当前 row 和刚找到的候选 group。
run_comparisons(1, &temp_id, nullptr, out_group_ids, &num_not_equal, &temp_id,

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里不是退化成了单行比较?性能没问题?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant