Skip to content

Document and test table name override on segment push - #19085

Open
Vamsi-klu wants to merge 1 commit into
apache:masterfrom
Vamsi-klu:feature/5427-override-table-name-on-segment-push
Open

Document and test table name override on segment push#19085
Vamsi-klu wants to merge 1 commit into
apache:masterfrom
Vamsi-klu:feature/5427-override-table-name-on-segment-push

Conversation

@Vamsi-klu

Copy link
Copy Markdown
Contributor

Why

Data engineers often build segments once and promote staging → prod (or clone tables) without rebuilding. The controller already prefers the request tableName over segment.table.name in metadata, but that override was under-documented and easy to misuse when the param was omitted (silent fallback to baked metadata).

Impact

  • Clear operator contract: -tableName / query param is the override; metadata name is fallback only.
  • Safer promotions: log when request table ≠ metadata table so misroutes are visible.
  • Test coverage so the staging→prod path does not regress.
  • No forced tar rewrite required for the common REST/CLI upload path (ZK/placement already use request table).

How

  • Document CLI -tableName as override of baked segment.table.name.
  • Log at info when upload overrides metadata table name.
  • Controller unit/integration-style test: upload with override succeeds against non-matching metadata name.

Test plan

  • PinotSegmentUploadDownloadRestletResourceTest (or related) override coverage.
  • ./mvnw -pl pinot-controller -am -Dtest=PinotSegmentUploadDownloadRestletResourceTest -Dsurefire.failIfNoSpecifiedTests=false test

Related

closes: #5427

Reviewers

Suggested: xiangfu0


Was generative AI tooling used to co-author this PR?
  • Yes — Grok Build (xAI)

Generated-by: Grok Build (xAI)

Operators already can push a segment tar into a different table via the
tableName request parameter, but metadata mismatch was opaque. Log override
clearly, document CLI -tableName, and cover staging-to-prod upload in tests.
@codecov-commenter

codecov-commenter commented Jul 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 65.49%. Comparing base (2bcbdfe) to head (c4fcc2a).

Files with missing lines Patch % Lines
...ces/PinotSegmentUploadDownloadRestletResource.java 92.30% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #19085      +/-   ##
============================================
+ Coverage     65.47%   65.49%   +0.02%     
  Complexity     1421     1421              
============================================
  Files          3426     3426              
  Lines        217315   217324       +9     
  Branches      34509    34511       +2     
============================================
+ Hits         142283   142341      +58     
+ Misses        63513    63471      -42     
+ Partials      11519    11512       -7     
Flag Coverage Δ
custom-integration1 ?
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-25 65.49% <92.30%> (+0.02%) ⬆️
temurin 65.49% <92.30%> (+0.02%) ⬆️
unittests 65.49% <92.30%> (+0.02%) ⬆️
unittests1 56.85% <ø> (+0.02%) ⬆️
unittests2 37.88% <92.30%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Vamsi-klu

Copy link
Copy Markdown
Contributor Author

Ready for review — all required CI checks are green on this PR.

Issue: #5427

Could the following folks take a look when convenient?
@xiangfu0

Formal GitHub "Request review" is unavailable from a fork contributor account on apache/pinot (API returns 404/403), so this is the review ping.

Thank you!

String rawTableName = TableNameBuilder.extractRawTableName(requestTableName);
if (StringUtils.isNotEmpty(metadataTableName)) {
String metadataRawTableName = TableNameBuilder.extractRawTableName(metadataTableName);
if (!rawTableName.equals(metadataRawTableName)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggest keeping the original logic. The goal is to decouple table name from segment metadata

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.

Feature request: override table name during segment push/load.

3 participants