Skip to content

[Enhancement] Pre-split base and rollup for INSERT-from-table into a range table#76392

Open
xiangguangyxg wants to merge 1 commit into
StarRocks:mainfrom
xiangguangyxg:presplit-insert-from-table-rollup
Open

[Enhancement] Pre-split base and rollup for INSERT-from-table into a range table#76392
xiangguangyxg wants to merge 1 commit into
StarRocks:mainfrom
xiangguangyxg:presplit-insert-from-table-rollup

Conversation

@xiangguangyxg

@xiangguangyxg xiangguangyxg commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Why I'm doing:

Sample-Based Tablet Pre-Split previously skipped INSERT ... SELECT ... FROM <single OLAP table> whenever the range-distribution target carried a rollup, so on such a load neither the base index nor the rollup got pre-split (they stayed at one tablet and relied on post-load auto-merge/compaction). PR #76233 shipped multi-index pre-split for FILES/Broker loads but documented INSERT-from-table + rollup as a descope, because the sampler could only project the base sort key and only by target column name, whereas INSERT-from-table must project by the source column name via the target->source column remap.

What I'm doing:

Make INSERT-from-table split every visible index (base plus each rollup):

  • InsertFromTableScanContext now carries the full target->source column-name map instead of a base-only source-column list. The data-tier executor maps whatever sort key it is handed through that map: request.getSortKey() for the base (in the multi-partition single-request flow, and per-index in the single-partition flow), and each rollup spec's sort key for the secondary projection.
  • The shared secondary-index projection becomes an overridable executor seam (AbstractSqlSampleSubqueryExecutor.secondaryIndexProjectionIdents). The default projects by target column name, so FILES/Broker generate byte-identical SQL; INSERT-from-table overrides it to remap to source names.
  • TablePreSplitSource now builds one secondary index spec per visible rollup.
  • The two guards that skipped multi-index INSERT-from-table are removed.

Safety: single-index (no-rollup) INSERT-from-table is byte-identical (the base sort key maps to the same source names). FILES/Broker and the ALTER-side pre-split path are untouched. Any unmapped sort-key column fails the sample so the load simply proceeds without pre-split (a base-only or no split is data-safe, since the BE routes each row by its true value). Data tier only -- an OLAP source has no Parquet/ORC footer, so the meta tier is already bypassed for this load kind.

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Does this PR entail a change in behavior?

  • Yes, this PR will result in a change in behavior.
  • No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

  • Interface/UI changes: syntax, type conversion, expression evaluation, display information
  • Parameter changes: default values, similar parameters but with different default values
  • Policy changes: use new policy to replace old one, functionality automatically enabled
  • Feature removed
  • Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function
    • This pr needs auto generate documentation
  • This is a backport pr

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto-backported to the target branch
    • 4.1
    • 4.0
    • 3.5

@CelerData-Reviewer

CelerData-Reviewer commented Jul 15, 2026

Copy link
Copy Markdown

@codex review

@github-actions github-actions Bot requested review from luohaha and meegoo July 15, 2026 04:04
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: 3c8caaffbb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@xiangguangyxg

xiangguangyxg commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Shared-data e2e validation (TSP build 2611, 1 FE + 3 CN, ubuntu)

Range table DUPLICATE KEY(k1,k2) ORDER BY(k1,k2) + divergent rollup ADD ROLLUP r(k1,k2,v) ORDER BY(k2), 3,000,000-row OLAP source. All CNs: tablet_reshard_split_external_boundaries_fallback_total Δ=0 throughout.

Case Source SELECT Result
Primary SELECT * FROM src (names match, by-position) base 1→2 (composite (k1,k2) ranges) and rollup 1→2 (single (k2) ranges); COUNT=3,000,000, SUM(k2)=4,499,998,500,000 exact; tier_used{data_tier}+1; fallback Δ=0
Non-identity remap SELECT a, b, c FROM src2 (bare cols, k1←a, k2←b) base 1→2 and rollup 1→2 with source-derived boundaries; COUNT/SUM(k2) exact; fallback Δ=0
Renamed source under * SELECT * FROM src2 (names differ) skips pre-split (by-position SELECT * name-match gate); data loaded correctly
Aggregation SELECT k1,k2,sum(v) ... GROUP BY k1,k2 skips gracefully (no split, no fallback); data loaded
Expression SELECT k1, k2+1, v skips gracefully (no split); data loaded

Both the base index and the divergent rollup pre-split from the same sample, with per-index boundaries computed from the correctly-remapped source columns; every non-eligible source skips without a fallback. FE unit suite (presplit + broker) 636/0/0, checkstyle clean.

@xiangguangyxg xiangguangyxg force-pushed the presplit-insert-from-table-rollup branch from 3c8caaf to c1a6bbe Compare July 15, 2026 07:06
@CelerData-Reviewer

CelerData-Reviewer commented Jul 15, 2026

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: c1a6bbefd4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@xiangguangyxg xiangguangyxg force-pushed the presplit-insert-from-table-rollup branch 2 times, most recently from bc7cb88 to b89c10a Compare July 15, 2026 08:05
@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

[Java-Extensions Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

[FE Incremental Coverage Report]

✅ pass : 47 / 47 (100.00%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 com/starrocks/alter/reshard/presplit/AbstractSqlSampleSubqueryExecutor.java 5 5 100.00% []
🔵 com/starrocks/alter/reshard/presplit/InsertFromTableSampleSubqueryExecutor.java 14 14 100.00% []
🔵 com/starrocks/alter/reshard/presplit/BrokerLoadPreSplitHook.java 1 1 100.00% []
🔵 com/starrocks/alter/reshard/presplit/FilesPreSplitSource.java 1 1 100.00% []
🔵 com/starrocks/alter/reshard/presplit/InsertFromTableScanContext.java 1 1 100.00% []
🔵 com/starrocks/alter/reshard/presplit/TablePreSplitSource.java 12 12 100.00% []
🔵 com/starrocks/alter/reshard/presplit/InsertSelectSourceColumns.java 5 5 100.00% []
🔵 com/starrocks/alter/reshard/presplit/SecondaryIndexSpec.java 8 8 100.00% []

@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

[BE Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

@xiangguangyxg xiangguangyxg force-pushed the presplit-insert-from-table-rollup branch from b89c10a to cd4619c Compare July 15, 2026 10:23
@CelerData-Reviewer

CelerData-Reviewer commented Jul 15, 2026

Copy link
Copy Markdown

@codex review

@sonarqubecloud

sonarqubecloud Bot commented Jul 15, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
6 New issues
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

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: cd4619c51e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@xiangguangyxg @claude
[Enhancement] Pre-split base and rollup for INSERT-from-table into a …
f9476c2 
…range table

Sample-Based Tablet Pre-Split previously skipped INSERT ... SELECT ... FROM a
single OLAP table when the range-distribution target carried a rollup, because
the sampler could only project the base sort key and only by target column name.

This makes INSERT-from-table split every visible index (base plus each rollup):

- InsertFromTableScanContext carries the full target->source column-name map
  instead of a base-only source-column list, and the data-tier executor maps
  whatever sort key it is handed (request.getSortKey() for the base; each rollup
  spec's sort key for the secondary projection) through that map.
- The shared secondary-index projection becomes an overridable executor seam
  whose default projects by target column name (FILES / Broker byte-identical);
  INSERT-from-table overrides it to remap to source names.
- TablePreSplitSource builds one secondary index spec per visible rollup.
- The two guards that skipped multi-index INSERT-from-table are removed.

Single-index INSERT-from-table is byte-identical (the base sort key maps to the
same source names). FILES / Broker and the ALTER-side pre-split path are
untouched. Any unmapped sort-key column fails the sample so the load proceeds
without pre-split. Data tier only (an OLAP source has no Parquet/ORC footer).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: xiangguangyxg <xiangguangyxg@gmail.com>
@xiangguangyxg xiangguangyxg force-pushed the presplit-insert-from-table-rollup branch from cd4619c to f9476c2 Compare July 15, 2026 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@luohaha luohaha Awaiting requested review from luohaha
@meegoo meegoo Awaiting requested review from meegoo

At least 2 approving reviews are required to merge this pull request.

Assignees

@xiangguangyxg xiangguangyxg

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@xiangguangyxg @CelerData-Reviewer