Vamsi-klu opened a new pull request, #19399: URL: https://github.com/apache/pinot/pull/19399
Fixes #10848 Please tag `bug`. No `release-notes` (no new config, no public API change). ## What's the problem Consume-time MV length check compared `values.length` to `getMaxChunkCapacity()` (the incremental buffer, often millions). `updateHeader` only `assert`ed, and asserts are off in production. Rows with 1001+ values were written, then seal failed because value counts did not match. Partial-upsert APPEND/UNION can grow MV arrays after the inbound check. `addNewRow` swallows per-index exceptions and still advances `_numDocsIndexed`. ## What's the solution Compare inbound MV length to `getMaxNumberOfMultiValuesPerRow()` (1000). `Preconditions.checkArgument` in `updateHeader` before the header is mutated. After `updateRecord`, re-validate the merged upsert row before `addRecord` / `updateDictionary` / `addNewRow`. ## Why this way The 1000 cap is already the consume contract (`ForwardIndexType.MAX_MULTI_VALUES_PER_ROW`). This PR does not raise that cap. It does not change `addNewRow`'s catch (that is #16316 / #19088). CLP columns are left alone (`instanceof FixedByteMVMutableForwardIndex` skip). ## How it is implemented - `FixedByteMVMutableForwardIndex`: reject oversized rows in `updateHeader` with `Preconditions.checkArgument`. - `MutableSegmentImpl.validateLengthOfMVColumns`: use the per-row max, not chunk capacity. Call it on the inbound row, then again on the post-merge upsert row. ## Impact Rows with 1001+ MV values that used to ingest are now dropped at consume (`UnsupportedOperationException`). The consume loop logs the row and advances the offset. Mixed-version is intended. No new cluster keys. ## Test plan - [x] `FixedByteMVMutableForwardIndexTest`: reject 1001, accept 1000, write after reject - [x] `MutableSegmentImplMVLengthValidationTest`: inbound 1001 and APPEND merge 1000+1 - [x] 8/8 tests passed - [x] `./mvnw spotless:apply checkstyle:check license:format license:check -pl pinot-segment-local` ``` ./mvnw -pl pinot-segment-local -Dtest=FixedByteMVMutableForwardIndexTest,MutableSegmentImplMVLengthValidationTest -Dsurefire.failIfNoSpecifiedTests=false test ``` cc @kirkrodrigues @Jackie-Jiang @xiangfu0 ##### Was generative AI tooling used to co-author this PR? - [x] Yes Made with [Cursor](https://cursor.com) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
