[ASTERIXDB-2516][RT] Prevent passing null type to comparator provider

- user model changes: no
- storage format changes: no
- interface changes: no

Details:
This change is to disallow passing null as IAType to the comparator
provider when asking for a comparator. It also includes few fixes
for issues appearing as a result of this change.

- changed the generic comparator to check whether the tag of data
at runtime is a valid tag and throw an exception if not.
- modified the comparator provider to return non-tagged comparator
for IAType SHORTWITHOUTINFOTYPE which is a short without tag.
SHORTWITHOUTINFOTYPE should not use the generic comparator since
the input data has no tag.
- fixed Dataset class to consider external dataset when getting
the IAType of the primary keys. The primary keys for external
datasets are different from regular datasets. They are not
part of the record type. Previously, null would be returned.
This would cause a failure when getting a comparator for the
primary keys of an external dataset since the type passed is null.
- fixed the TypeTraitProvider to give the correct lengths for
duration, day_time_duration, and year_month_duration.

Change-Id: I37767a3f3d1e3b29597d2a4998c0b60005cadb09
Reviewed-on: https://asterix-gerrit.ics.uci.edu/3336
Contrib: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: Till Westmann <tillw@apache.org>
diff --git a/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Dataset.java b/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Dataset.java
index 855cf78..d207f7f 100644
--- a/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Dataset.java
+++ b/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Dataset.java
@@ -740,8 +740,10 @@
         // Set the serde/traits for primary keys
         for (int i = 0; i < numPrimaryKeys; i++) {
             IAType keyType =
-                    (indicators == null || indicators.get(i) == 0) ? itemType.getSubFieldType(partitioningKeys.get(i))
-                            : metaType.getSubFieldType(partitioningKeys.get(i));
+                    datasetType == DatasetType.EXTERNAL ? IndexingConstants.getFieldType(i)
+                            : (indicators == null || indicators.get(i) == 0)
+                                    ? itemType.getSubFieldType(partitioningKeys.get(i))
+                                    : metaType.getSubFieldType(partitioningKeys.get(i));
             primaryRecFields[i] = serdeProvider.getSerializerDeserializer(keyType);
             primaryTypeTraits[i] = TypeTraitProvider.INSTANCE.getTypeTrait(keyType);
         }
@@ -778,8 +780,9 @@
             indicators = ((InternalDatasetDetails) getDatasetDetails()).getKeySourceIndicator();
         }
         for (int i = 0; i < numPrimaryKeys; i++) {
-            IAType keyType =
-                    (indicators == null || indicators.get(i) == 0) ? recordType.getSubFieldType(partitioningKeys.get(i))
+            IAType keyType = datasetType == DatasetType.EXTERNAL ? IndexingConstants.getFieldType(i)
+                    : (indicators == null || indicators.get(i) == 0)
+                            ? recordType.getSubFieldType(partitioningKeys.get(i))
                             : metaType.getSubFieldType(partitioningKeys.get(i));
             cmpFactories[i] = cmpFactoryProvider.getBinaryComparatorFactory(keyType, true);
         }
@@ -806,8 +809,9 @@
             indicators = ((InternalDatasetDetails) getDatasetDetails()).getKeySourceIndicator();
         }
         for (int i = 0; i < numPrimaryKeys; i++) {
-            IAType keyType =
-                    (indicators == null || indicators.get(i) == 0) ? recordType.getSubFieldType(partitioningKeys.get(i))
+            IAType keyType = datasetType == DatasetType.EXTERNAL ? IndexingConstants.getFieldType(i)
+                    : (indicators == null || indicators.get(i) == 0)
+                            ? recordType.getSubFieldType(partitioningKeys.get(i))
                             : metaType.getSubFieldType(partitioningKeys.get(i));
             hashFuncFactories[i] = BinaryHashFunctionFactoryProvider.INSTANCE.getBinaryHashFunctionFactory(keyType);
         }
diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/AbstractAGenericBinaryComparator.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/AbstractAGenericBinaryComparator.java
index 4510310..e907f75 100644
--- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/AbstractAGenericBinaryComparator.java
+++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/AbstractAGenericBinaryComparator.java
@@ -62,7 +62,7 @@
             new PointableBinaryComparatorFactory(UTF8StringPointable.FACTORY).createBinaryComparator();
     private final IBinaryComparator ascByteArrayComp =
             new PointableBinaryComparatorFactory(ByteArrayPointable.FACTORY).createBinaryComparator();
-    // the type fields can be null
+
     protected final IAType leftType;
     protected final IAType rightType;
     private final ListObjectPool<IMutableValueStorage, Void> storageAllocator = new ListObjectPool<>(STORAGE_FACTORY);
@@ -70,28 +70,27 @@
     private final ListObjectPool<SortedRecord, ARecordType> recordPool = new ListObjectPool<>(RECORD_FACTORY);
 
     AbstractAGenericBinaryComparator(IAType leftType, IAType rightType) {
-        // factory should have already made sure to get the actual type
+        // factory should have already made sure to get the actual type (and no null types)
         this.leftType = leftType;
         this.rightType = rightType;
     }
 
     protected final int compare(IAType leftType, byte[] b1, int s1, int l1, IAType rightType, byte[] b2, int s2, int l2)
             throws HyracksDataException {
-        if (b1[s1] == ATypeTag.SERIALIZED_MISSING_TYPE_TAG) {
-            return b2[s2] == ATypeTag.SERIALIZED_MISSING_TYPE_TAG ? 0 : -1;
-        } else if (b2[s2] == ATypeTag.SERIALIZED_MISSING_TYPE_TAG) {
-            return 1;
-        }
-        if (b1[s1] == ATypeTag.SERIALIZED_NULL_TYPE_TAG) {
-            return b2[s2] == ATypeTag.SERIALIZED_NULL_TYPE_TAG ? 0 : -1;
-        } else if (b2[s2] == ATypeTag.SERIALIZED_NULL_TYPE_TAG) {
-            return 1;
-        }
         ATypeTag tag1 = EnumDeserializer.ATYPETAGDESERIALIZER.deserialize(b1[s1]);
         ATypeTag tag2 = EnumDeserializer.ATYPETAGDESERIALIZER.deserialize(b2[s2]);
-        // tag being null could mean several things among of which is that the passed args are not tagged
         if (tag1 == null || tag2 == null) {
-            return RawBinaryComparatorFactory.compare(b1, s1, l1, b2, s2, l2);
+            throw new IllegalStateException("Could not recognize the type of data.");
+        }
+        if (tag1 == ATypeTag.MISSING) {
+            return tag2 == ATypeTag.MISSING ? 0 : -1;
+        } else if (tag2 == ATypeTag.MISSING) {
+            return 1;
+        }
+        if (tag1 == ATypeTag.NULL) {
+            return tag2 == ATypeTag.NULL ? 0 : -1;
+        } else if (tag2 == ATypeTag.NULL) {
+            return 1;
         }
         if (ATypeHierarchy.isCompatible(tag1, tag2) && ATypeHierarchy.getTypeDomain(tag1) == Domain.NUMERIC) {
             return ComparatorUtil.compareNumbers(tag1, b1, s1 + 1, tag2, b2, s2 + 1);
@@ -142,9 +141,9 @@
             case BINARY:
                 return ascByteArrayComp.compare(b1, s1 + 1, l1 - 1, b2, s2 + 1, l2 - 1);
             case ARRAY:
-                return compareArrays(leftType, b1, s1, l1, rightType, b2, s2, l2);
+                return compareArrays(leftType, b1, s1, rightType, b2, s2);
             case OBJECT:
-                return compareRecords(leftType, b1, s1, l1, rightType, b2, s2, l2);
+                return compareRecords(leftType, b1, s1, rightType, b2, s2);
             default:
                 return RawBinaryComparatorFactory.compare(b1, s1, l1, b2, s2, l2);
         }
@@ -154,11 +153,8 @@
         return AIntervalAscPartialBinaryComparatorFactory.compare(b1, s1 + 1, l1 - 1, b2, s2 + 1, l2 - 1);
     }
 
-    private int compareArrays(IAType leftType, byte[] b1, int s1, int l1, IAType rightType, byte[] b2, int s2, int l2)
+    private int compareArrays(IAType leftType, byte[] b1, int s1, IAType rightType, byte[] b2, int s2)
             throws HyracksDataException {
-        if (leftType == null || rightType == null) {
-            return RawBinaryComparatorFactory.compare(b1, s1, l1, b2, s2, l2);
-        }
         int leftNumItems = ListAccessorUtil.numberOfItems(b1, s1);
         int rightNumItems = ListAccessorUtil.numberOfItems(b2, s2);
         IAType leftArrayType = TypeComputeUtils.getActualTypeOrOpen(leftType, ATypeTag.ARRAY);
@@ -195,11 +191,8 @@
         }
     }
 
-    private int compareRecords(IAType leftType, byte[] b1, int s1, int l1, IAType rightType, byte[] b2, int s2, int l2)
+    private int compareRecords(IAType leftType, byte[] b1, int s1, IAType rightType, byte[] b2, int s2)
             throws HyracksDataException {
-        if (leftType == null || rightType == null) {
-            return RawBinaryComparatorFactory.compare(b1, s1, l1, b2, s2, l2);
-        }
         ARecordType leftRecordType = (ARecordType) TypeComputeUtils.getActualTypeOrOpen(leftType, ATypeTag.OBJECT);
         ARecordType rightRecordType = (ARecordType) TypeComputeUtils.getActualTypeOrOpen(rightType, ATypeTag.OBJECT);
         SortedRecord leftRecord = recordPool.allocate(leftRecordType);
diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/AbstractAGenericBinaryComparatorFactory.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/AbstractAGenericBinaryComparatorFactory.java
index 140db22..c743e6e 100644
--- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/AbstractAGenericBinaryComparatorFactory.java
+++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/AbstractAGenericBinaryComparatorFactory.java
@@ -19,6 +19,7 @@
 package org.apache.asterix.dataflow.data.nontagged.comparators;
 
 import org.apache.asterix.om.typecomputer.impl.TypeComputeUtils;
+import org.apache.asterix.om.types.BuiltinType;
 import org.apache.asterix.om.types.IAType;
 import org.apache.hyracks.api.dataflow.value.IBinaryComparatorFactory;
 import org.apache.hyracks.api.exceptions.HyracksDataException;
@@ -28,36 +29,32 @@
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 
-public abstract class AbstractAGenericBinaryComparatorFactory implements IBinaryComparatorFactory {
+abstract class AbstractAGenericBinaryComparatorFactory implements IBinaryComparatorFactory {
 
     private static final long serialVersionUID = 1L;
-    // these fields can be null
-    protected final IAType leftType;
-    protected final IAType rightType;
+    final IAType leftType;
+    final IAType rightType;
 
     AbstractAGenericBinaryComparatorFactory(IAType leftType, IAType rightType) {
-        this.leftType = leftType == null ? null : TypeComputeUtils.getActualType(leftType);
-        this.rightType = rightType == null ? null : TypeComputeUtils.getActualType(rightType);
+        this.leftType = TypeComputeUtils.getActualType(leftType);
+        this.rightType = TypeComputeUtils.getActualType(rightType);
     }
 
     JsonNode convertToJson(IPersistedResourceRegistry registry, Class<? extends IJsonSerializable> clazz, long version)
             throws HyracksDataException {
         ObjectNode jsonNode = registry.getClassIdentifier(clazz, version);
-        if (leftType != null) {
-            jsonNode.set("leftType", leftType.toJson(registry));
-        }
-        if (rightType != null) {
-            jsonNode.set("rightType", rightType.toJson(registry));
-        }
+        jsonNode.set("leftType", leftType.toJson(registry));
+        jsonNode.set("rightType", rightType.toJson(registry));
         return jsonNode;
     }
 
     static IJsonSerializable convertToObject(IPersistedResourceRegistry registry, JsonNode json, boolean asc)
             throws HyracksDataException {
-        JsonNode leftNode = json.get("leftType");
-        JsonNode rightNode = json.get("rightType");
-        IAType leftType = leftNode == null || leftNode.isNull() ? null : (IAType) registry.deserialize(leftNode);
-        IAType rightType = rightNode == null || rightNode.isNull() ? null : (IAType) registry.deserialize(rightNode);
+        JsonNode left = json.get("leftType");
+        JsonNode right = json.get("rightType");
+        // default to ANY for comparators that didn't originally have the new type fields
+        IAType leftType = left == null || left.isNull() ? BuiltinType.ANY : (IAType) registry.deserialize(left);
+        IAType rightType = right == null || right.isNull() ? BuiltinType.ANY : (IAType) registry.deserialize(right);
         return asc ? new AGenericAscBinaryComparatorFactory(leftType, rightType)
                 : new AGenericDescBinaryComparatorFactory(leftType, rightType);
     }
diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/hash/AMurmurHash3BinaryHashFunctionFamily.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/hash/AMurmurHash3BinaryHashFunctionFamily.java
index 020fbd1..f02c764 100644
--- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/hash/AMurmurHash3BinaryHashFunctionFamily.java
+++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/hash/AMurmurHash3BinaryHashFunctionFamily.java
@@ -122,22 +122,19 @@
                             valueBuffer.getLength(), seed);
                 case ARRAY:
                     try {
-                        return hashArray(type, bytes, offset, length);
+                        return hashArray(type, bytes, offset);
                     } catch (IOException e) {
                         throw HyracksDataException.create(e);
                     }
                 case OBJECT:
-                    return hashRecord(type, bytes, offset, length);
+                    return hashRecord(type, bytes, offset);
                 case DOUBLE:
                 default:
                     return MurmurHash3BinaryHash.hash(bytes, offset, length, seed);
             }
         }
 
-        private int hashArray(IAType type, byte[] bytes, int offset, int length) throws IOException {
-            if (type == null) {
-                return MurmurHash3BinaryHash.hash(bytes, offset, length, seed);
-            }
+        private int hashArray(IAType type, byte[] bytes, int offset) throws IOException {
             IAType arrayType = TypeComputeUtils.getActualTypeOrOpen(type, ATypeTag.ARRAY);
             IAType itemType = ((AbstractCollectionType) arrayType).getItemType();
             ATypeTag itemTag = itemType.getTypeTag();
@@ -158,10 +155,7 @@
             return hash;
         }
 
-        private int hashRecord(IAType type, byte[] bytes, int offset, int length) throws HyracksDataException {
-            if (type == null) {
-                return MurmurHash3BinaryHash.hash(bytes, offset, length, seed);
-            }
+        private int hashRecord(IAType type, byte[] bytes, int offset) throws HyracksDataException {
             ARecordType recordType = (ARecordType) TypeComputeUtils.getActualTypeOrOpen(type, ATypeTag.OBJECT);
             SortedRecord record = recordPool.allocate(recordType);
             IPointable fieldValue = voidPointableAllocator.allocate(null);
diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/formats/nontagged/BinaryComparatorFactoryProvider.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/formats/nontagged/BinaryComparatorFactoryProvider.java
index 7567e0c..b3a6d27 100644
--- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/formats/nontagged/BinaryComparatorFactoryProvider.java
+++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/formats/nontagged/BinaryComparatorFactoryProvider.java
@@ -33,6 +33,7 @@
 import org.apache.asterix.dataflow.data.nontagged.comparators.ARectanglePartialBinaryComparatorFactory;
 import org.apache.asterix.dataflow.data.nontagged.comparators.AUUIDPartialBinaryComparatorFactory;
 import org.apache.asterix.om.types.ATypeTag;
+import org.apache.asterix.om.types.BuiltinType;
 import org.apache.asterix.om.types.IAType;
 import org.apache.hyracks.algebricks.data.IBinaryComparatorFactoryProvider;
 import org.apache.hyracks.api.dataflow.value.IBinaryComparatorFactory;
@@ -91,19 +92,23 @@
     @Override
     public IBinaryComparatorFactory getBinaryComparatorFactory(Object leftType, Object rightType, boolean ascending,
             boolean ignoreCase) {
-        if (leftType == null || rightType == null) {
-            return createGenericBinaryComparatorFactory(null, null, ascending);
-        }
         IAType left = (IAType) leftType;
         IAType right = (IAType) rightType;
+        // TODO(ali): what if someone passed ignoreCase=true and type ANY (at runtime it could be a string)?
         if (left.getTypeTag() == ATypeTag.STRING && right.getTypeTag() == ATypeTag.STRING && ignoreCase) {
             return addOffset(UTF8STRING_LOWERCASE_POINTABLE_INSTANCE, ascending);
         }
+        if (isShortWithoutTag(left, right)) {
+            return SHORT_POINTABLE_INSTANCE;
+        }
         return createGenericBinaryComparatorFactory(left, right, ascending);
     }
 
     @Override
     public IBinaryComparatorFactory getBinaryComparatorFactory(Object leftType, Object rightType, boolean ascending) {
+        if (isShortWithoutTag((IAType) leftType, (IAType) rightType)) {
+            return SHORT_POINTABLE_INSTANCE;
+        }
         // During a comparison, since proper type promotion among several numeric types are required,
         // we will use AGenericAscBinaryComparatorFactory, instead of using a specific comparator
         return createGenericBinaryComparatorFactory((IAType) leftType, (IAType) rightType, ascending);
@@ -113,8 +118,9 @@
         switch (type) {
             case ANY:
             case UNION:
+                // i think UNION shouldn't be allowed. the actual type could be closed array or record. ANY would fail.
                 // we could do smth better for nullable fields
-                return createGenericBinaryComparatorFactory(null, null, ascending);
+                return createGenericBinaryComparatorFactory(BuiltinType.ANY, BuiltinType.ANY, ascending);
             case NULL:
             case MISSING:
                 return new AnyBinaryComparatorFactory();
@@ -186,4 +192,17 @@
             return AIntervalDescPartialBinaryComparatorFactory.INSTANCE;
         }
     }
+
+    private static boolean isShortWithoutTag(IAType left, IAType right) {
+        ATypeTag leftTag = left.getTypeTag();
+        ATypeTag rightTag = right.getTypeTag();
+        if (leftTag != ATypeTag.SHORTWITHOUTTYPEINFO && rightTag != ATypeTag.SHORTWITHOUTTYPEINFO) {
+            return false;
+        }
+        if (leftTag != rightTag) {
+            // this should not happen (i.e. comparing untagged (short without tag) vs some tagged)
+            throw new IllegalStateException();
+        }
+        return true;
+    }
 }
diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/formats/nontagged/TypeTraitProvider.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/formats/nontagged/TypeTraitProvider.java
index 8cfe78d..51f4664 100644
--- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/formats/nontagged/TypeTraitProvider.java
+++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/formats/nontagged/TypeTraitProvider.java
@@ -27,18 +27,17 @@
 public class TypeTraitProvider implements ITypeTraitProvider {
 
     // WARNING: the byte sizes depend on the serializer!
-    // currently assuming a serializer that adds a 1-byte type indicator before
-    // the data
-    private static final ITypeTraits ONEBYTETYPETRAIT = new FixedLengthTypeTrait(1 + 1);
-    private static final ITypeTraits TWOBYTETYPETRAIT = new FixedLengthTypeTrait(2 + 1);
-    private static final ITypeTraits FOURBYTETYPETRAIT = new FixedLengthTypeTrait(4 + 1);
-    private static final ITypeTraits EIGHTBYTETYPETRAIT = new FixedLengthTypeTrait(8 + 1);
-    private static final ITypeTraits SIXTEENBYTETYPETRAIT = new FixedLengthTypeTrait(16 + 1);
-    private static final ITypeTraits SEVENTEENBYTETYPETRAIT = new FixedLengthTypeTrait(17 + 1);
-    private static final ITypeTraits THIRTYTWOBYTETYPETRAIT = new FixedLengthTypeTrait(32 + 1);
-    private static final ITypeTraits TWENTYFOURBYTETYPETRAIT = new FixedLengthTypeTrait(24 + 1);
-
-    private static final ITypeTraits VARLENTYPETRAIT = VarLengthTypeTrait.INSTANCE;
+    // currently assuming a serializer that adds a 1-byte type indicator before the data
+    private static final ITypeTraits ONE_BYTE_TYPE_TRAIT = new FixedLengthTypeTrait(1 + 1);
+    private static final ITypeTraits TWO_BYTE_TYPE_TRAIT = new FixedLengthTypeTrait(2 + 1);
+    private static final ITypeTraits FOUR_BYTE_TYPE_TRAIT = new FixedLengthTypeTrait(4 + 1);
+    private static final ITypeTraits EIGHT_BYTE_TYPE_TRAIT = new FixedLengthTypeTrait(8 + 1);
+    private static final ITypeTraits TWELVE_BYTE_TYPE_TRAIT = new FixedLengthTypeTrait(12 + 1);
+    private static final ITypeTraits SIXTEEN_BYTE_TYPE_TRAIT = new FixedLengthTypeTrait(16 + 1);
+    private static final ITypeTraits SEVENTEEN_BYTE_TYPE_TRAIT = new FixedLengthTypeTrait(17 + 1);
+    private static final ITypeTraits THIRTY_TWO_BYTE_TYPE_TRAIT = new FixedLengthTypeTrait(32 + 1);
+    private static final ITypeTraits TWENTY_FOUR_BYTE_TYPE_TRAIT = new FixedLengthTypeTrait(24 + 1);
+    private static final ITypeTraits VAR_LEN_TYPE_TRAIT = VarLengthTypeTrait.INSTANCE;
 
     public static final TypeTraitProvider INSTANCE = new TypeTraitProvider();
 
@@ -51,31 +50,33 @@
         switch (type.getTypeTag()) {
             case BOOLEAN:
             case TINYINT:
-                return ONEBYTETYPETRAIT;
+                return ONE_BYTE_TYPE_TRAIT;
             case SMALLINT:
-                return TWOBYTETYPETRAIT;
+                return TWO_BYTE_TYPE_TRAIT;
             case INTEGER:
             case FLOAT:
             case DATE:
             case TIME:
-                return FOURBYTETYPETRAIT;
+            case YEARMONTHDURATION:
+                return FOUR_BYTE_TYPE_TRAIT;
             case BIGINT:
             case DOUBLE:
             case DATETIME:
+            case DAYTIMEDURATION:
+                return EIGHT_BYTE_TYPE_TRAIT;
             case DURATION:
-                return EIGHTBYTETYPETRAIT;
+                return TWELVE_BYTE_TYPE_TRAIT;
             case POINT:
             case UUID:
-                return SIXTEENBYTETYPETRAIT;
+                return SIXTEEN_BYTE_TYPE_TRAIT;
             case INTERVAL:
-                return SEVENTEENBYTETYPETRAIT;
+                return SEVENTEEN_BYTE_TYPE_TRAIT;
             case POINT3D:
-                return TWENTYFOURBYTETYPETRAIT;
+                return TWENTY_FOUR_BYTE_TYPE_TRAIT;
             case LINE:
-                return THIRTYTWOBYTETYPETRAIT;
-            default: {
-                return VARLENTYPETRAIT;
-            }
+                return THIRTY_TWO_BYTE_TYPE_TRAIT;
+            default:
+                return VAR_LEN_TYPE_TRAIT;
         }
     }
 }
\ No newline at end of file
diff --git a/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/BooleanPointable.java b/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/BooleanPointable.java
index 3b122f8..8615529 100644
--- a/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/BooleanPointable.java
+++ b/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/BooleanPointable.java
@@ -33,9 +33,9 @@
 
 public final class BooleanPointable extends AbstractPointable implements IHashable, IComparable {
 
-    private static final int LENGTH = 1;
+    private static final int SIZE = 1;
     public static final BooleanPointableFactory FACTORY = new BooleanPointableFactory();
-    public static final FixedLengthTypeTrait TYPE_TRAITS = new FixedLengthTypeTrait(LENGTH);
+    public static final FixedLengthTypeTrait TYPE_TRAITS = new FixedLengthTypeTrait(SIZE);
 
     public static class BooleanPointableFactory implements IPointableFactory {
         private static final long serialVersionUID = 1L;
@@ -99,7 +99,7 @@
     }
 
     public static int compare(byte[] b1, int s1, int l1, byte[] b2, int s2, int l2) {
-        DataUtils.ensureLengths(LENGTH, l1, l2);
+        DataUtils.ensureLengths(SIZE, l1, l2);
         return Boolean.compare(getBoolean(b1, s1), getBoolean(b2, s2));
     }
 
diff --git a/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/BytePointable.java b/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/BytePointable.java
index 41e7c82..5ac6833 100644
--- a/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/BytePointable.java
+++ b/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/BytePointable.java
@@ -34,9 +34,9 @@
 
 public final class BytePointable extends AbstractPointable implements IHashable, IComparable, INumeric {
 
-    private static final int LENGTH = 1;
+    private static final int SIZE = 1;
     public static final BytePointableFactory FACTORY = new BytePointableFactory();
-    public static final ITypeTraits TYPE_TRAITS = new FixedLengthTypeTrait(LENGTH);
+    public static final ITypeTraits TYPE_TRAITS = new FixedLengthTypeTrait(SIZE);
 
     public static final class BytePointableFactory implements IPointableFactory {
         private static final long serialVersionUID = 1L;
@@ -89,7 +89,7 @@
     }
 
     public static int compare(byte[] b1, int s1, int l1, byte[] b2, int s2, int l2) {
-        DataUtils.ensureLengths(LENGTH, l1, l2);
+        DataUtils.ensureLengths(SIZE, l1, l2);
         return Byte.compare(getByte(b1, s1), getByte(b2, s2));
     }
 
diff --git a/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/DoublePointable.java b/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/DoublePointable.java
index ee31f30..4f5d0a1 100644
--- a/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/DoublePointable.java
+++ b/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/DoublePointable.java
@@ -34,9 +34,9 @@
 
 public final class DoublePointable extends AbstractPointable implements IHashable, IComparable, INumeric {
 
-    private static final int LENGTH = 8;
+    private static final int SIZE = 8;
     public static final DoublePointableFactory FACTORY = new DoublePointableFactory();
-    public static final ITypeTraits TYPE_TRAITS = new FixedLengthTypeTrait(LENGTH) {
+    public static final ITypeTraits TYPE_TRAITS = new FixedLengthTypeTrait(SIZE) {
         private static final long serialVersionUID = 7348262203696059687L;
 
         //TODO fix RTREE logic based on class comparision in LSMRTreeUtils#proposeBestLinearizer
@@ -112,7 +112,7 @@
     }
 
     public static int compare(byte[] b1, int s1, int l1, byte[] b2, int s2, int l2) {
-        DataUtils.ensureLengths(LENGTH, l1, l2);
+        DataUtils.ensureLengths(SIZE, l1, l2);
         return Double.compare(getDouble(b1, s1), getDouble(b2, s2));
     }
 
diff --git a/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/FloatPointable.java b/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/FloatPointable.java
index 0e330e7..289e441 100644
--- a/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/FloatPointable.java
+++ b/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/FloatPointable.java
@@ -34,8 +34,8 @@
 
 public final class FloatPointable extends AbstractPointable implements IHashable, IComparable, INumeric {
 
-    private static final int LENGTH = 4;
-    public static final ITypeTraits TYPE_TRAITS = new FixedLengthTypeTrait(LENGTH);
+    private static final int SIZE = 4;
+    public static final ITypeTraits TYPE_TRAITS = new FixedLengthTypeTrait(SIZE);
     public static final FloatPointableFactory FACTORY = new FloatPointableFactory();
 
     public static final class FloatPointableFactory implements IPointableFactory {
@@ -95,7 +95,7 @@
     }
 
     public static int compare(byte[] b1, int s1, int l1, byte[] b2, int s2, int l2) {
-        DataUtils.ensureLengths(LENGTH, l1, l2);
+        DataUtils.ensureLengths(SIZE, l1, l2);
         return Float.compare(getFloat(b1, s1), getFloat(b2, s2));
     }
 
diff --git a/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/IntegerPointable.java b/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/IntegerPointable.java
index 453c568..ad80e23 100644
--- a/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/IntegerPointable.java
+++ b/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/IntegerPointable.java
@@ -34,9 +34,9 @@
 
 public final class IntegerPointable extends AbstractPointable implements IHashable, IComparable, INumeric {
 
-    private static final int LENGTH = 4;
+    private static final int SIZE = 4;
     public static final IntegerPointableFactory FACTORY = new IntegerPointableFactory();
-    public static final ITypeTraits TYPE_TRAITS = new FixedLengthTypeTrait(LENGTH) {
+    public static final ITypeTraits TYPE_TRAITS = new FixedLengthTypeTrait(SIZE) {
         private static final long serialVersionUID = -7178318032449879790L;
 
         //TODO fix RTREE logic based on class comparision in LSMRTreeUtils#proposeBestLinearizer
@@ -101,7 +101,7 @@
     }
 
     public static int compare(byte[] b1, int s1, int l1, byte[] b2, int s2, int l2) {
-        DataUtils.ensureLengths(LENGTH, l1, l2);
+        DataUtils.ensureLengths(SIZE, l1, l2);
         return Integer.compare(getInteger(b1, s1), getInteger(b2, s2));
     }
 
diff --git a/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/LongPointable.java b/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/LongPointable.java
index 5929171..2de7b82 100644
--- a/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/LongPointable.java
+++ b/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/LongPointable.java
@@ -34,9 +34,9 @@
 
 public final class LongPointable extends AbstractPointable implements IHashable, IComparable, INumeric {
 
-    private static final int LENGTH = 8;
+    private static final int SIZE = 8;
     public static final LongPointableFactory FACTORY = new LongPointableFactory();
-    public static final ITypeTraits TYPE_TRAITS = new FixedLengthTypeTrait(LENGTH);
+    public static final ITypeTraits TYPE_TRAITS = new FixedLengthTypeTrait(SIZE);
 
     public static class LongPointableFactory implements IPointableFactory {
         private static final long serialVersionUID = 1L;
@@ -113,7 +113,7 @@
     }
 
     public static int compare(byte[] b1, int s1, int l1, byte[] b2, int s2, int l2) {
-        DataUtils.ensureLengths(LENGTH, l1, l2);
+        DataUtils.ensureLengths(SIZE, l1, l2);
         return Long.compare(getLong(b1, s1), getLong(b2, s2));
     }
 
diff --git a/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/ShortPointable.java b/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/ShortPointable.java
index 48599e9..3806c0c 100644
--- a/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/ShortPointable.java
+++ b/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/ShortPointable.java
@@ -34,8 +34,8 @@
 
 public final class ShortPointable extends AbstractPointable implements IHashable, IComparable, INumeric {
 
-    private static final int LENGTH = 2;
-    public static final ITypeTraits TYPE_TRAITS = new FixedLengthTypeTrait(LENGTH);
+    private static final int SIZE = 2;
+    public static final ITypeTraits TYPE_TRAITS = new FixedLengthTypeTrait(SIZE);
     public static final IPointableFactory FACTORY = new ShortPointableFactory();
 
     public static final class ShortPointableFactory implements IPointableFactory {
@@ -90,7 +90,7 @@
     }
 
     public static int compare(byte[] b1, int s1, int l1, byte[] b2, int s2, int l2) {
-        DataUtils.ensureLengths(LENGTH, l1, l2);
+        DataUtils.ensureLengths(SIZE, l1, l2);
         return Short.compare(getShort(b1, s1), getShort(b2, s2));
     }
 
diff --git a/hyracks-fullstack/hyracks/hyracks-dataflow-common/src/main/java/org/apache/hyracks/dataflow/common/utils/SerdeUtils.java b/hyracks-fullstack/hyracks/hyracks-dataflow-common/src/main/java/org/apache/hyracks/dataflow/common/utils/SerdeUtils.java
index 2f200c0..292482d 100644
--- a/hyracks-fullstack/hyracks/hyracks-dataflow-common/src/main/java/org/apache/hyracks/dataflow/common/utils/SerdeUtils.java
+++ b/hyracks-fullstack/hyracks/hyracks-dataflow-common/src/main/java/org/apache/hyracks/dataflow/common/utils/SerdeUtils.java
@@ -23,6 +23,7 @@
 import org.apache.hyracks.api.dataflow.value.IBinaryComparatorFactory;
 import org.apache.hyracks.api.dataflow.value.ISerializerDeserializer;
 import org.apache.hyracks.api.dataflow.value.ITypeTraits;
+import org.apache.hyracks.data.std.accessors.BooleanBinaryComparatorFactory;
 import org.apache.hyracks.data.std.accessors.DoubleBinaryComparatorFactory;
 import org.apache.hyracks.data.std.accessors.FloatBinaryComparatorFactory;
 import org.apache.hyracks.data.std.accessors.IntegerBinaryComparatorFactory;
@@ -126,8 +127,7 @@
             return DoubleBinaryComparatorFactory.INSTANCE;
         }
         if (serde instanceof BooleanSerializerDeserializer) {
-            // TODO(ali): how come?
-            throw new UnsupportedOperationException("Binary comparator factory for Boolean not implemented.");
+            return BooleanBinaryComparatorFactory.INSTANCE;
         }
         if (serde instanceof UTF8StringSerializerDeserializer) {
             return PointableBinaryComparatorFactory.of(UTF8StringPointable.FACTORY);