[NO ISSUE][RT] Fix upserting into secondary indexes
- user model changes: no
- storage format changes: no
- interface changes: no
Details:
- do not upsert tuples that have a null/missing value
in any SK field into secondary indexes (whether
composite or non-composite ones).
Change-Id: I9cc94de34b5ef1dfd5e1e7b6b9b35cc7316759ab
Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/7443
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: Michael Blow <mblow@apache.org>
diff --git a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/storage/IndexCheckpoint.java b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/storage/IndexCheckpoint.java
index 3d0b9cb..878c94e 100644
--- a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/storage/IndexCheckpoint.java
+++ b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/storage/IndexCheckpoint.java
@@ -38,7 +38,7 @@
private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
private static final long INITIAL_CHECKPOINT_ID = 0;
// TODO(mblow): remove this marker & related logic once we no longer are able to read indexes prior to the fix
- private static final long HAS_NULL_MISSING_VALUES_FIX = -1;
+ private static final long HAS_NULL_MISSING_VALUES_FIX = -2;
private long id;
private long validComponentSequence;
private long lowWatermark;
diff --git a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMSecondaryUpsertOperatorNodePushable.java b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMSecondaryUpsertOperatorNodePushable.java
index aa5775e..35ae904 100644
--- a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMSecondaryUpsertOperatorNodePushable.java
+++ b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMSecondaryUpsertOperatorNodePushable.java
@@ -45,21 +45,20 @@
* This operator node is used for secondary indexes with upsert operations.
* It works in the following way:
* For each incoming tuple
- * -If old secondary keys == new secondary keys
+ * -If old secondary index tuple == new secondary index tuple
* --do nothing
* -else
- * --If old secondary keys are null?
+ * --If any old field is null/missing?
* ---do nothing
* --else
- * ---delete old secondary keys
- * --If new keys are null?
+ * ---delete old secondary index tuple
+ * --If any new field is null/missing?
* ---do nothing
* --else
- * ---insert new keys
+ * ---insert new secondary index tuple
*/
public class LSMSecondaryUpsertOperatorNodePushable extends LSMIndexInsertUpdateDeleteOperatorNodePushable {
- private static final int NULL_MISSING_FIELD_INDEX = 0;
private final PermutingFrameTupleReference prevValueTuple = new PermutingFrameTupleReference();
private final int upsertIndicatorFieldIndex;
private final IBinaryBooleanInspector upsertIndicatorInspector;
@@ -105,9 +104,9 @@
tuple.reset(accessor, i);
prevValueTuple.reset(accessor, i);
- boolean isNewValueNullOrMissing = isNullOrMissing(tuple);
- boolean isOldValueNullOrMissing = isNullOrMissing(prevValueTuple);
- if (isNewValueNullOrMissing && isOldValueNullOrMissing) {
+ boolean newTupleHasNullOrMissing = hasNullOrMissing(tuple);
+ boolean oldTupleHasNullOrMissing = hasNullOrMissing(prevValueTuple);
+ if (newTupleHasNullOrMissing && oldTupleHasNullOrMissing) {
// No op
continue;
}
@@ -118,13 +117,13 @@
// which are always the same
continue;
}
- if (!isOldValueNullOrMissing) {
- // We need to delete previous
+ // if all old fields are known values, then delete. skip deleting if any is null or missing
+ if (!oldTupleHasNullOrMissing) {
abstractModCallback.setOp(Operation.DELETE);
lsmAccessor.forceDelete(prevValueTuple);
}
- if (isUpsert && !isNewValueNullOrMissing) {
- // we need to insert the new value
+ // if all new fields are known values, then insert. skip inserting if any is null or missing
+ if (isUpsert && !newTupleHasNullOrMissing) {
abstractModCallback.setOp(Operation.INSERT);
lsmAccessor.forceInsert(tuple);
}
@@ -138,8 +137,18 @@
FrameUtils.flushFrame(writeBuffer.getBuffer(), writer);
}
- private static boolean isNullOrMissing(PermutingFrameTupleReference tuple) {
- return TypeTagUtil.isType(tuple, NULL_MISSING_FIELD_INDEX, ATypeTag.SERIALIZED_NULL_TYPE_TAG)
- || TypeTagUtil.isType(tuple, NULL_MISSING_FIELD_INDEX, ATypeTag.SERIALIZED_MISSING_TYPE_TAG);
+ private boolean hasNullOrMissing(PermutingFrameTupleReference tuple) {
+ int fieldCount = tuple.getFieldCount();
+ for (int i = 0; i < fieldCount; i++) {
+ if (isNullOrMissing(tuple, i)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private static boolean isNullOrMissing(PermutingFrameTupleReference tuple, int fieldIdx) {
+ return TypeTagUtil.isType(tuple, fieldIdx, ATypeTag.SERIALIZED_NULL_TYPE_TAG)
+ || TypeTagUtil.isType(tuple, fieldIdx, ATypeTag.SERIALIZED_MISSING_TYPE_TAG);
}
}