This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 499a109ed0 Fixes partial upsert not reflecting multiple comparison
column values (#10693)
499a109ed0 is described below
commit 499a109ed022497603df279fd73c6ad7017d73c3
Author: Evan Galpin <[email protected]>
AuthorDate: Thu Apr 27 10:53:03 2023 -0700
Fixes partial upsert not reflecting multiple comparison column values
(#10693)
---
.../apache/pinot/segment/local/upsert/PartialUpsertHandler.java | 8 ++++++--
.../local/indexsegment/mutable/MutableSegmentImplUpsertTest.java | 7 +++++++
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/PartialUpsertHandler.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/PartialUpsertHandler.java
index f18a95ca00..86c268e509 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/PartialUpsertHandler.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/PartialUpsertHandler.java
@@ -66,12 +66,16 @@ public class PartialUpsertHandler {
*/
public GenericRow merge(GenericRow previousRecord, GenericRow newRecord) {
for (String column : previousRecord.getFieldToValueMap().keySet()) {
- if (!_primaryKeyColumns.contains(column) &&
!_comparisonColumns.contains(column)) {
+ if (!_primaryKeyColumns.contains(column)) {
if (!previousRecord.isNullValue(column)) {
if (newRecord.isNullValue(column)) {
+ // Note that we intentionally want to overwrite any previous
_comparisonColumn value in the case of using
+ // multiple comparison columns. We never apply a merge function to
it, rather we just take any/all non-null
+ // comparison column values from the previous record, and the sole
non-null comparison column value from
+ // the new record.
newRecord.putValue(column, previousRecord.getValue(column));
newRecord.removeNullValueField(column);
- } else {
+ } else if (!_comparisonColumns.contains(column)) {
PartialUpsertMerger merger = _column2Mergers.getOrDefault(column,
_defaultPartialUpsertMerger);
newRecord.putValue(column,
merger.merge(previousRecord.getValue(column),
newRecord.getValue(column)));
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/indexsegment/mutable/MutableSegmentImplUpsertTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/indexsegment/mutable/MutableSegmentImplUpsertTest.java
index 4f5ad8bc6c..c8b38db61e 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/indexsegment/mutable/MutableSegmentImplUpsertTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/indexsegment/mutable/MutableSegmentImplUpsertTest.java
@@ -135,10 +135,17 @@ public class MutableSegmentImplUpsertTest {
Assert.assertFalse(bitmap.contains(1));
Assert.assertTrue(bitmap.contains(2));
Assert.assertFalse(bitmap.contains(3));
+ // Confirm that both comparison column values have made it into the
persisted upserted doc
+ Assert.assertEquals(1567205397L, _mutableSegmentImpl.getValue(2,
"secondsSinceEpoch"));
+ Assert.assertEquals(1567205395L, _mutableSegmentImpl.getValue(2,
"otherComparisonColumn"));
+
// bb
Assert.assertFalse(bitmap.contains(4));
Assert.assertTrue(bitmap.contains(5));
Assert.assertFalse(bitmap.contains(6));
+ // Confirm that comparison column values have made it into the persisted
upserted doc
+ Assert.assertEquals(1567205396L, _mutableSegmentImpl.getValue(5,
"secondsSinceEpoch"));
+ Assert.assertEquals(Long.MIN_VALUE, _mutableSegmentImpl.getValue(5,
"otherComparisonColumn"));
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
