Skip to content

Add lifecycle hooks and configs for custom partial-upsert mergers - #19422

Open
xiangfu0 wants to merge 3 commits into
apache:masterfrom
xiangfu0:codex/structured-partial-upsert-plumbing
Open

Add lifecycle hooks and configs for custom partial-upsert mergers#19422
xiangfu0 wants to merge 3 commits into
apache:masterfrom
xiangfu0:codex/structured-partial-upsert-plumbing

Conversation

@xiangfu0

@xiangfu0 xiangfu0 commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Motivation

Custom partial-upsert mergers can own row merge behavior, but they currently cannot receive implementation-specific configuration or validate and normalize records that have no previously stored row. Plugins also need an opt-in way to validate schema changes against their table configuration.

Changes

  • Add optional partialUpsertMergerConfigs to UpsertConfig.
  • Add a backward-compatible PartialUpsertMerger.prepare(GenericRow) hook.
  • Invoke prepare() for every partial-upsert record before previous-row lookup and update, including the first record for a primary key.
  • Reject merger configs without a custom merger class and continue rejecting combinations of custom mergers with built-in per-column strategies.
  • Treat a custom merger class and its configs as immutable on existing tables, preventing replicas from using mixed semantics while handlers remain initialized with old configuration.
  • Add an opt-in schema-validation phase to TableConfigValidator and invoke registered validators during schema writes.

Compatibility

Existing custom mergers remain compatible because prepare() defaults to a no-op, and existing table configs need not define merger configs.

This intentionally tightens table-update validation: adding, removing, or changing a custom merger class/config on an existing table is rejected. Such handlers are initialized at table startup, so accepting live mutations could give replicas different merge semantics.

Implementations must not change primary-key or comparison-column values from prepare() because RecordInfo has already been constructed.

Tests

  • 107 focused tests passed across pinot-spi and pinot-segment-local, including UpsertConfigTest, TableConfigUtilsTest, PartialUpsertHandlerTest, BasePartitionUpsertMetadataManagerTest, TableConfigValidatorRegistryTest, and SchemaUtilsTableConfigValidatorTest.
  • Affected-module formatting, Checkstyle, license checks, and git diff --check passed.

@codecov-commenter

codecov-commenter commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.17647% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.64%. Comparing base (d08ac91) to head (7ed3c9d).

Files with missing lines Patch % Lines
...he/pinot/segment/local/utils/TableConfigUtils.java 85.71% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #19422      +/-   ##
============================================
- Coverage     67.67%   67.64%   -0.03%     
  Complexity     1430     1430              
============================================
  Files          3488     3490       +2     
  Lines        224395   224430      +35     
  Branches      35422    35428       +6     
============================================
- Hits         151867   151826      -41     
- Misses        60488    60577      +89     
+ Partials      12040    12027      -13     
Flag Coverage Δ
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-25 67.64% <91.17%> (-0.03%) ⬇️
lane-a 100.00% <ø> (ø)
lane-b 0.00% <ø> (ø)
temurin 67.64% <91.17%> (-0.03%) ⬇️
unittests 67.64% <91.17%> (-0.03%) ⬇️
unittests1 57.78% <26.47%> (-0.01%) ⬇️
unittests2 39.36% <91.17%> (-0.02%) ⬇️

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.

@xiangfu0
xiangfu0 force-pushed the codex/structured-partial-upsert-plumbing branch 3 times, most recently from 9f8e968 to 294a20f Compare September 2, 2026 09:23
@xiangfu0
xiangfu0 force-pushed the codex/structured-partial-upsert-plumbing branch from 294a20f to 7ed3c9d Compare September 3, 2026 09:24
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.

2 participants