abdullah alamoudi has uploaded a new change for review.

  https://asterix-gerrit.ics.uci.edu/1735

Change subject: Rename type check methods into meaningful names
......................................................................

Rename type check methods into meaningful names

This change renames methods used for checking operation in upsert
operators into meaningful names. In addition, it removes the
unnecessary search in case of delete operations with only a primary
index in the pipeline.

Change-Id: I35e5ed919aff2c374be1fbbb00ad7a752916a3dc
---
M 
asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/TypeTagUtil.java
M 
asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMPrimaryUpsertOperatorNodePushable.java
M 
asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMSecondaryUpsertOperatorNodePushable.java
3 files changed, 46 insertions(+), 35 deletions(-)


  git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb 
refs/changes/35/1735/1

diff --git 
a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/TypeTagUtil.java
 
b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/TypeTagUtil.java
index 1af3578..f3fe477 100644
--- 
a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/TypeTagUtil.java
+++ 
b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/TypeTagUtil.java
@@ -20,6 +20,7 @@
 
 import org.apache.asterix.common.exceptions.AsterixException;
 import org.apache.asterix.om.utils.RecordUtil;
+import org.apache.hyracks.dataflow.common.data.accessors.ITupleReference;
 
 public class TypeTagUtil {
 
@@ -91,4 +92,8 @@
                 throw new AsterixException("Typetag " + typeTag + " is not a 
built-in type");
         }
     }
+    
+    public static boolean isType(ITupleReference tuple, int fieldIdx, byte 
tag){
+        return tuple.getFieldData(fieldIdx)[tuple.getFieldStart(fieldIdx)] == 
tag;
+    }
 }
diff --git 
a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMPrimaryUpsertOperatorNodePushable.java
 
b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMPrimaryUpsertOperatorNodePushable.java
index 03ef88f..c70baa0 100644
--- 
a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMPrimaryUpsertOperatorNodePushable.java
+++ 
b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMPrimaryUpsertOperatorNodePushable.java
@@ -31,6 +31,7 @@
 import org.apache.asterix.om.pointables.nonvisitor.ARecordPointable;
 import org.apache.asterix.om.types.ARecordType;
 import org.apache.asterix.om.types.ATypeTag;
+import org.apache.asterix.om.types.TypeTagUtil;
 import 
org.apache.asterix.transaction.management.opcallbacks.AbstractIndexModificationOperationCallback;
 import 
org.apache.asterix.transaction.management.opcallbacks.AbstractIndexModificationOperationCallback.Operation;
 import 
org.apache.asterix.transaction.management.opcallbacks.LockThenSearchOperationCallback;
@@ -193,11 +194,11 @@
         }
     }
 
-    public static boolean isNull(ITupleReference t1, int field) {
-        return t1.getFieldData(field)[t1.getFieldStart(field)] == 
ATypeTag.SERIALIZED_MISSING_TYPE_TAG;
+    private static boolean isDeleteOperation(ITupleReference t1, int field) {
+        return TypeTagUtil.isType(t1, field, 
ATypeTag.SERIALIZED_MISSING_TYPE_TAG);
     }
 
-    private void addNullField() throws IOException {
+    private void writeMissingField() throws IOException {
         dos.write(missingTupleBuilder.getByteArray());
         tb.addFieldEndOffset();
     }
@@ -213,9 +214,10 @@
             while (i < tupleCount) {
                 tb.reset();
                 boolean recordWasInserted = false;
+                boolean recordWasDeleted = false;
                 tuple.reset(accessor, i);
                 resetSearchPredicate(i);
-                if (hasSecondaries || isNull(tuple, numOfPrimaryKeys)) {
+                if (hasSecondaries) {
                     lsmAccessor.search(cursor, searchPred);
                     if (cursor.hasNext()) {
                         cursor.next();
@@ -240,24 +242,24 @@
                                     
prevTuple.getFieldLength(filterFieldIndex));
                             tb.addFieldEndOffset();
                         }
-                        if (isNull(tuple, numOfPrimaryKeys)) {
-                            // Only delete if it is a delete and not upsert
-                            abstractModCallback.setOp(Operation.DELETE);
-                            if (firstModification) {
-                                lsmAccessor.delete(prevTuple);
-                                firstModification = false;
-                            } else {
-                                lsmAccessor.forceDelete(prevTuple);
-                            }
-                        }
                     } else {
-                        appendNullPreviousTuple();
+                        appendPreviousTupleAsMissing();
                     }
                 } else {
                     searchCallback.before(key);
-                    appendNullPreviousTuple();
+                    appendPreviousTupleAsMissing();
                 }
-                if (!isNull(tuple, numOfPrimaryKeys)) {
+                if (isDeleteOperation(tuple, numOfPrimaryKeys)) {
+                    // Only delete if it is a delete and not upsert
+                    abstractModCallback.setOp(Operation.DELETE);
+                    if (firstModification) {
+                        lsmAccessor.delete(tuple);
+                        firstModification = false;
+                    } else {
+                        lsmAccessor.forceDelete(tuple);
+                    }
+                    recordWasDeleted = true;
+                } else {
                     abstractModCallback.setOp(Operation.UPSERT);
                     if (firstModification) {
                         lsmAccessor.upsert(tuple);
@@ -267,7 +269,7 @@
                     }
                     recordWasInserted = true;
                 }
-                writeOutput(i, recordWasInserted, prevTuple != null);
+                writeOutput(i, recordWasInserted, recordWasDeleted);
                 i++;
             }
             // callback here before calling nextFrame on the next operator
@@ -278,15 +280,15 @@
         }
     }
 
-    private void appendNullPreviousTuple() throws IOException {
+    private void appendPreviousTupleAsMissing() throws IOException {
         prevTuple = null;
-        addNullField();
+        writeMissingField();
         if (hasMeta) {
-            addNullField();
+            writeMissingField();
         }
         // if with filters, append null
         if (isFiltered) {
-            addNullField();
+            writeMissingField();
         }
         cursor.reset();
     }
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 0db5ff0..a3292f0 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
@@ -20,6 +20,8 @@
 
 import java.nio.ByteBuffer;
 
+import org.apache.asterix.om.types.ATypeTag;
+import org.apache.asterix.om.types.TypeTagUtil;
 import 
org.apache.asterix.transaction.management.opcallbacks.AbstractIndexModificationOperationCallback;
 import 
org.apache.asterix.transaction.management.opcallbacks.AbstractIndexModificationOperationCallback.Operation;
 import org.apache.hyracks.api.context.IHyracksTaskContext;
@@ -52,8 +54,8 @@
 
     private final PermutingFrameTupleReference prevValueTuple = new 
PermutingFrameTupleReference();
     private int numberOfFields;
-    private boolean isNewNull = false;
-    private boolean isPrevValueNull = false;
+    private boolean isNewValueMissing = false;
+    private boolean isOldValueMissing = false;
     private AbstractIndexModificationOperationCallback abstractModCallback;
 
     public LSMSecondaryUpsertOperatorNodePushable(IIndexOperatorDescriptor 
opDesc, IHyracksTaskContext ctx,
@@ -105,9 +107,10 @@
                 // if both previous value and new value are null, then we skip
                 tuple.reset(accessor, i);
                 prevValueTuple.reset(accessor, i);
-                isNewNull = LSMPrimaryUpsertOperatorNodePushable.isNull(tuple, 
0);
-                isPrevValueNull = 
LSMPrimaryUpsertOperatorNodePushable.isNull(prevValueTuple, 0);
-                if (isNewNull && isPrevValueNull) {
+                isNewValueMissing = isMissing(tuple, 0);
+                isOldValueMissing = isMissing(prevValueTuple, 0);
+                if (isNewValueMissing && isOldValueMissing) {
+                    // No op
                     continue;
                 }
                 // At least, one is not null
@@ -115,21 +118,18 @@
                 if (equalTuples(tuple, prevValueTuple, numberOfFields)) {
                     continue;
                 }
-                if (!isPrevValueNull) {
-                    // previous is not null, we need to delete previous
+                if (!isOldValueMissing) {
+                    // We need to delete previous
                     abstractModCallback.setOp(Operation.DELETE);
                     lsmAccessor.forceDelete(prevValueTuple);
                 }
-                if (!isNewNull) {
-                    // new is not null, we need to insert the new value
+                if (!isNewValueMissing) {
+                    // we need to insert the new value
                     abstractModCallback.setOp(Operation.INSERT);
                     lsmAccessor.forceInsert(tuple);
                 }
-
-            } catch (HyracksDataException e) {
-                throw e;
             } catch (Exception e) {
-                throw new HyracksDataException(e);
+                throw HyracksDataException.create(e);
             }
         }
         // No partial flushing was necessary. Forward entire frame.
@@ -137,4 +137,8 @@
         FrameUtils.copyAndFlip(buffer, writeBuffer.getBuffer());
         FrameUtils.flushFrame(writeBuffer.getBuffer(), writer);
     }
+
+    private boolean isMissing(PermutingFrameTupleReference tuple, int 
fieldIdx) {
+        return TypeTagUtil.isType(tuple, fieldIdx, 
ATypeTag.SERIALIZED_MISSING_TYPE_TAG);
+    }
 }

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1735
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I35e5ed919aff2c374be1fbbb00ad7a752916a3dc
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <bamou...@gmail.com>

Reply via email to