deepthi912 opened a new pull request, #19726:
URL: https://github.com/apache/pinot/pull/19726
## Problem
`MutableSegmentImpl` builds each record's `PrimaryKey` from the **segment's
own schema**:
```java
// MutableSegmentImpl#getRecordInfo / #getDedupRecordInfo
PrimaryKey primaryKey = row.getPrimaryKey(_schema.getPrimaryKeyColumns());
```
Every segment-level path in the metadata manager instead uses the list
captured once in `UpsertContext` / `DedupContext`:
```java
// BasePartitionUpsertMetadataManager, in doAddSegment / doPreloadSegment /
doReplaceSegment
new UpsertUtils.RecordInfoReader(segment, _primaryKeyColumns,
_comparisonColumns, _deleteRecordColumn)
```
`_primaryKeyColumns` is `final` and only ever set from the context built in
`RealtimeTableDataManager#doInit()`, i.e. at server start. A consuming segment,
on the other hand, gets a fresh `RealtimeSegmentConfig` and so re-reads the
schema every time one is created.
So changing `primaryKeyColumns` desyncs the two **without any restart**.
From the next segment rollover onward, records are keyed one way during
consumption and a different way at commit.
That difference is not benign, because `PrimaryKey.asBytes()` uses a
distinct layout for a single value versus several:
```java
public byte[] asBytes() {
if (_values.length == 1) {
return asBytesSingleVal(_values[0]); // raw bytes, no length prefix
}
// otherwise: length-prefixed multi-value layout
```
Going from one key column to two changes the encoding entirely, so the
hashes diverge. The metadata lookup misses, `addRecord` takes the "new primary
key" branch, and the new doc is marked valid **without invalidating the
previous one** — because as far as that code path is concerned, no previous
record exists.
The result is two valid docs for a single primary key. It recurs at every
segment rollover and does not self-correct until the server restarts and the
manager re-reads the schema.
`getDedupRecordInfo` has the same bug, and dedup hashes primary keys the
same way, so a primary key change silently stops deduplication in the same
manner.
## Fix
Take the columns from the metadata manager's context, which is exactly how
the comparison columns, the delete record column and the dedup time column are
**already** sourced a few lines above in the same constructor:
```java
_upsertPrimaryKeyColumns = upsertContext.getPrimaryKeyColumns();
_upsertComparisonColumns = upsertContext.getComparisonColumns();
_deleteRecordColumn = upsertContext.getDeleteRecordColumn();
```
Both `UpsertContext` and `DedupContext` already expose
`getPrimaryKeyColumns()`; the primary key was simply the one setting still read
from the schema. Cached in fields rather than calling `getContext()` per row,
since this is the per-record ingestion path.
## Testing
Adds
`MutableSegmentImplUpsertTest#testPrimaryKeyColumnsTakenFromUpsertContext`,
which builds the metadata manager from a schema with one primary key column and
the consuming segment from a schema where a second column has joined the key,
then indexes two records sharing the first column.
Verified it fails without the source change and passes with it:
```
# without the fix
testPrimaryKeyColumnsTakenFromUpsertContext FAILURE
java.lang.AssertionError: expected [1] but found [2]
# with the fix
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0
```
`expected [1] but found [2]` is the duplicate-valid-docs symptom reproduced
directly.
Existing tests in the class pass unchanged; `spotless:apply` is a no-op and
checkstyle reports 0 violations.
## Backward compatibility
No config or format change. Behaviour only differs where the segment schema
and the metadata manager context disagree on the primary key, which is the
broken case being fixed. When they agree — every case where a primary key
change has not been applied mid-process — behaviour is identical.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
