Skip to content

Avoid q2 per-row global code remap - #3327

Closed
snissn wants to merge 3 commits into
codex/q2-post-prepare-diagnostics-3324from
codex/q2-post-prepare-lazy-3324
Closed

Avoid q2 per-row global code remap#3327
snissn wants to merge 3 commits into
codex/q2-post-prepare-diagnostics-3324from
codex/q2-post-prepare-lazy-3324

Conversation

@snissn

@snissn snissn commented Jun 29, 2026

Copy link
Copy Markdown
Owner

Summary

  • replace q2 sorted grouped-distinct per-row global code materialization with per-part local-code-to-global-rank maps
  • translate local dictionary codes to global ranks at iterator boundaries
  • keep grouped output dictionary available while avoiding distinct global dictionary/global code arrays

Production-path caveat

A 1M JSONBench full-data q2 one_shot_end_to_end / no_aggregate_metadata run against this branch showed the production q2 cell currently uses dense grouped-count-distinct (dense_group_count_distinct_used=true, sort_layout=time_us), not the sorted grouped-distinct path changed here.

Latest artifact:

  • /tmp/jsonbench_q2_lazy_1m_pUFYuebb/report.json
  • total 85.240ms, setup 65.209ms, run 19.986ms, post-prepare 40.849ms
  • dense q2 split: group rank 0.061ms, distinct rank 34.566ms, local rank 6.221ms
  • rows scanned/matched/reduced 1,000,000 / 954,611 / 954,611
  • result hash b63b8e1013c918fcda7c299ef64beb20c4988854cf0671a83803b6951d795860

So this PR is a valid sorted grouped-distinct algorithmic experiment, but it should not be presented as the production JSONBench full-data q2 improvement. The production q2 follow-up now targets dense grouped-count-distinct distinct-rank/local-rank setup; see #3324.

Claim boundary

This is no-aggregate one-shot typed-column q2 setup work only. It is not metadata acceleration evidence and is not a ClickHouse superiority claim. The PR stays draft until a production or harness q2 cell that actually exercises the sorted grouped-distinct path has current 1M evidence.

Validation

  • git diff --check
  • GOWORK=off go test ./TreeDB/collections -run 'TestColumnPhysicalTypedColumnOneShotCacheQ2NoMetadata3123|TestTypedColumnQ2.*1950|TestTypedColumnQ2DenseGroupCountDistinctRankMapCapacity3158' -count=1
  • GOWORK=off go test ./TreeDB/collections -count=1

Evidence still required

  • 1M q2 cell that exercises the sorted grouped-distinct path: setup, q2 post-prepare split, run, total
  • preserve rows scanned/matched/reduced and result hash
  • include q1/q3 no-regression samples if shared setup behavior is affected

Refs #3324. Refs #3070.

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 40fe45ec-8676-41e8-af8c-ddcbf0445c01

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/q2-post-prepare-lazy-3324

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@snissn

snissn commented Jul 1, 2026

Copy link
Copy Markdown
Owner Author

Closing as superseded by the refreshed sorted-path q2 draft #3413 and the current dense grouped-count-distinct production lane in #3324. This PR remains useful historical/reference evidence, but it is not merge-ready production q2 one-shot/no-metadata evidence.

@snissn snissn closed this Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant