Add backward-incompatibility check for upsert deleteRecordColumn#18552
Merged
Conversation
mayankshriv
force-pushed
the
upsert-backward-compat-checks
branch
from
9cacd82 to
bd1c600
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #18552 +/- ##
============================================
+ Coverage 64.24% 64.31% +0.07%
Complexity 1126 1126
============================================
Files 3311 3311
Lines 203820 203829 +9
Branches 31720 31722 +2
============================================
+ Hits 130949 131102 +153
+ Misses 62356 62206 -150
- Partials 10515 10521 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
deepthi912
reviewed
May 21, 2026
deepthi912
reviewed
May 21, 2026
mayankshriv
force-pushed
the
upsert-backward-compat-checks
branch
from
bd1c600 to
a00b68a
Compare
The deleteRecordColumn field was documented in the javadoc as being checked during table config updates, but the code was never implemented. Changing it mid-flight causes deletes to silently use the wrong column since the value is cached at partition manager initialization. Not adding checks for enabling/disabling upsert or consistencyMode changes — these are allowed operations. Toggling upsert on/off is a supported workflow (e.g., enabling upsert on an existing table after schema changes). Changing consistencyMode is an operational tuning decision that takes effect on server restart without data corruption. Tests added: - Rejects changing, adding, or removing deleteRecordColumn - Verifies identical upsert config produces no violations - Verifies non-upsert tables produce no violations Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mayankshriv
changed the title
mayankshriv
force-pushed
the
upsert-backward-compat-checks
branch
from
a00b68a to
1ce6305
Compare
Contributor
Author
|
Thanks for the review @deepthi912! Re: upsert toggle (comment) — Agreed, this is a supported workflow. Removed the upsert toggle violation in the latest push; it's back to the original INFO-level logging. Re: consistencyMode (comment) — Makes sense — consistencyMode change is an operational tuning decision, not data-corrupting. Removed this check in the latest push. The PR now only adds the missing |
mayankshriv
added
the
backward-incompat
KKcorps
approved these changes
May 21, 2026
Contributor
|
Docs PR opened: pinot-contrib/pinot-docs#814 |
xiangfu0
added a commit
to pinot-contrib/pinot-docs
that referenced
this pull request
May 21, 2026
## Summary - document that `deleteRecordColumn` is immutable for existing upsert tables unless the update is forced - add `deleteRecordColumn` to the protected-field lists on the upsert guide and controller API reference ## Source cross-check - `pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java` - `pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/TableConfigUtilsTest.java` - `apache/pinot#18552` ## Validation - `git diff --check`
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
5 participants
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.
Summary
The
deleteRecordColumnfield inUpsertConfigwas documented in the javadoc ofvalidateUpsertConfigUpdateas being checked during table config updates, but the check was never implemented. Changing it mid-flight causes deletes to silently use the wrong column, since the value is cached at partition manager initialization.This PR adds the missing backward-compatibility check, following the same pattern as the existing checks for
mode,hashFunction,comparisonColumns, etc. — rejected by default, bypassable withforce=trueon the update API.Why only deleteRecordColumn?
consistencyModeis an operational tuning decision that takes effect on server restart without data corruption — not a backward-incompatible change.Tests added
deleteRecordColumnTest plan
TableConfigUtilsTest#testValidateBackwardCompatibility*)testValidateBackwardCompatibilityAllowsPartialUpsertStrategyChangesstill passes🤖 Generated with Claude Code