Ali Alsuliman has submitted this change and it was merged. Change subject: [ASTERIXDB-2516][COMP] Avoid writing into buffer when comparing numbers ......................................................................
[ASTERIXDB-2516][COMP] Avoid writing into buffer when comparing numbers - user model changes: no - storage format changes: no - interface changes: no Details: Avoid writing into buffer when comparing and promoting between numbers. - made seed the initial hash for arrays and records. - renamed & refactored LogicalComparatorUtil to share code between logical and physical comparators - minor code clean-ups Change-Id: Ie089d386a9ab8271f2833c05ffdfb0d484937b51 Reviewed-on: https://asterix-gerrit.ics.uci.edu/3272 Integration-Tests: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Tested-by: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Contrib: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Reviewed-by: Dmitry Lychagin <dmitry.lycha...@couchbase.com> Reviewed-by: Till Westmann <ti...@apache.org> --- M asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/AbstractAGenericBinaryComparator.java R asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/ComparatorUtil.java M asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/LogicalComplexBinaryComparator.java M asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/LogicalScalarBinaryComparator.java M asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/hash/AMurmurHash3BinaryHashFunctionFamily.java M asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/ATypeTag.java M asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/AbstractComparisonEvaluator.java 7 files changed, 106 insertions(+), 249 deletions(-) Approvals: Anon. E. Moose #1000171: Till Westmann: Looks good to me, approved Jenkins: Verified; ; Verified Dmitry Lychagin: Looks good to me, but someone else must approve Objections: Jenkins: Violations found 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 da50c56..098c81f 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 @@ -27,6 +27,11 @@ import org.apache.asterix.dataflow.data.common.ListAccessorUtil; import org.apache.asterix.dataflow.data.nontagged.CompareHashUtil; +import org.apache.asterix.dataflow.data.nontagged.serde.ADateSerializerDeserializer; +import org.apache.asterix.dataflow.data.nontagged.serde.ADateTimeSerializerDeserializer; +import org.apache.asterix.dataflow.data.nontagged.serde.ADayTimeDurationSerializerDeserializer; +import org.apache.asterix.dataflow.data.nontagged.serde.ATimeSerializerDeserializer; +import org.apache.asterix.dataflow.data.nontagged.serde.AYearMonthDurationSerializerDeserializer; import org.apache.asterix.om.pointables.ARecordVisitablePointable; import org.apache.asterix.om.pointables.PointableAllocator; import org.apache.asterix.om.pointables.base.IVisitablePointable; @@ -37,7 +42,7 @@ import org.apache.asterix.om.types.EnumDeserializer; import org.apache.asterix.om.types.IAType; import org.apache.asterix.om.types.hierachy.ATypeHierarchy; -import org.apache.asterix.om.types.hierachy.ITypeConvertComputer; +import org.apache.asterix.om.types.hierachy.ATypeHierarchy.Domain; import org.apache.asterix.om.util.container.IObjectPool; import org.apache.asterix.om.util.container.ListObjectPool; import org.apache.asterix.om.util.container.ObjectFactories; @@ -47,41 +52,23 @@ import org.apache.hyracks.data.std.api.IMutableValueStorage; import org.apache.hyracks.data.std.api.IPointable; import org.apache.hyracks.data.std.primitive.ByteArrayPointable; -import org.apache.hyracks.data.std.primitive.BytePointable; -import org.apache.hyracks.data.std.primitive.DoublePointable; -import org.apache.hyracks.data.std.primitive.FloatPointable; -import org.apache.hyracks.data.std.primitive.IntegerPointable; -import org.apache.hyracks.data.std.primitive.ShortPointable; import org.apache.hyracks.data.std.primitive.UTF8StringPointable; import org.apache.hyracks.data.std.util.ArrayBackedValueStorage; +/** + * This comparator is an ordering comparator. It deals with MISSING, NULL, and incompatible types different than the + * logical comparison. + */ abstract class AbstractAGenericBinaryComparator implements IBinaryComparator { // BOOLEAN private final IBinaryComparator ascBoolComp = BooleanBinaryComparatorFactory.INSTANCE.createBinaryComparator(); - // TINYINT - private final IBinaryComparator ascByteComp = - new PointableBinaryComparatorFactory(BytePointable.FACTORY).createBinaryComparator(); - // SMALLINT - private final IBinaryComparator ascShortComp = - new PointableBinaryComparatorFactory(ShortPointable.FACTORY).createBinaryComparator(); - // INTEGER - private final IBinaryComparator ascIntComp = - new PointableBinaryComparatorFactory(IntegerPointable.FACTORY).createBinaryComparator(); - // BIGINT - private final IBinaryComparator ascLongComp = LongBinaryComparatorFactory.INSTANCE.createBinaryComparator(); // STRING private final IBinaryComparator ascStrComp = new PointableBinaryComparatorFactory(UTF8StringPointable.FACTORY).createBinaryComparator(); // BINARY private final IBinaryComparator ascByteArrayComp = new PointableBinaryComparatorFactory(ByteArrayPointable.FACTORY).createBinaryComparator(); - // FLOAT - private final IBinaryComparator ascFloatComp = - new PointableBinaryComparatorFactory(FloatPointable.FACTORY).createBinaryComparator(); - // DOUBLE - private final IBinaryComparator ascDoubleComp = - new PointableBinaryComparatorFactory(DoublePointable.FACTORY).createBinaryComparator(); // RECTANGLE private final IBinaryComparator ascRectangleComp = ARectanglePartialBinaryComparatorFactory.INSTANCE.createBinaryComparator(); @@ -113,8 +100,6 @@ // these fields can be null protected final IAType leftType; protected final IAType rightType; - // a storage to promote a value - private final ArrayBackedValueStorage castBuffer; private final IObjectPool<IMutableValueStorage, Void> storageAllocator; private final IObjectPool<IPointable, Void> voidPointableAllocator; // used for record comparison, sorting field names @@ -126,7 +111,6 @@ // factory should have already made sure to get the actual type this.leftType = leftType; this.rightType = rightType; - this.castBuffer = new ArrayBackedValueStorage(); this.storageAllocator = new ListObjectPool<>(ObjectFactories.STORAGE_FACTORY); this.voidPointableAllocator = new ListObjectPool<>(ObjectFactories.VOID_FACTORY); this.recordAllocator = new PointableAllocator(); @@ -136,181 +120,54 @@ protected int compare(IAType leftType, byte[] b1, int s1, int l1, IAType rightType, byte[] b2, int s2, int l2) throws HyracksDataException { - // normally, comparing between MISSING and non-MISSING values should return MISSING as the result. - // however, this comparator is used by order-by/group-by/distinct-by. - // therefore, inside this method, we return an order between two values even if one value is MISSING. 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; - } + } else if (b2[s2] == ATypeTag.SERIALIZED_MISSING_TYPE_TAG) { + return 1; } - - // normally, comparing between NULL and non-NULL/MISSING values should return NULL as the result. - // however, this comparator is used by order-by/group-by/distinct-by. - // therefore, inside this method, we return an order between two values even if one value is NULL. 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; - } + } 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]); - // if one of tag is null, that means we are dealing with an empty byte array in one side. // and, we don't need to continue. We just compare raw byte by byte. if (tag1 == null || tag2 == null) { return rawComp.compare(b1, s1, l1, b2, s2, l2); } - - // if two type does not match, we identify the source and the target and - // promote the source to the target type if they are compatible. - ATypeTag sourceTypeTag = null; - ATypeTag targetTypeTag = null; - boolean areTwoTagsEqual = false; - boolean typePromotionApplied = false; - boolean leftValueChanged = false; - - if (tag1 != tag2) { - // tag1 can be promoted to tag2 (e.g. tag1: SMALLINT, tag2: INTEGER) - if (ATypeHierarchy.canPromote(tag1, tag2)) { - sourceTypeTag = tag1; - targetTypeTag = tag2; - typePromotionApplied = true; - leftValueChanged = true; - // or tag2 can be promoted to tag1 (e.g. tag2: INTEGER, tag1: DOUBLE) - } else if (ATypeHierarchy.canPromote(tag2, tag1)) { - sourceTypeTag = tag2; - targetTypeTag = tag1; - typePromotionApplied = true; - } - - // we promote the source to the target by using a promoteComputer - if (typePromotionApplied) { - castBuffer.reset(); - ITypeConvertComputer promoter = ATypeHierarchy.getTypePromoteComputer(sourceTypeTag, targetTypeTag); - if (promoter != null) { - try { - if (leftValueChanged) { - // left side is the source - promoter.convertType(b1, s1 + 1, l1 - 1, castBuffer.getDataOutput()); - } else { - // right side is the source - promoter.convertType(b2, s2 + 1, l2 - 1, castBuffer.getDataOutput()); - } - } catch (IOException e) { - throw new HyracksDataException("ComparatorFactory - failed to promote the type:" + sourceTypeTag - + " to the type:" + targetTypeTag); - } - } else { - // No appropriate typePromoteComputer. - throw new HyracksDataException("No appropriate typePromoteComputer exists for " + sourceTypeTag - + " to the " + targetTypeTag + " type. Please check the code."); - } - } - } else { - // tag1 == tag2. - sourceTypeTag = tag1; - targetTypeTag = tag1; - areTwoTagsEqual = true; + if (ATypeHierarchy.isCompatible(tag1, tag2) && ATypeHierarchy.getTypeDomain(tag1) == Domain.NUMERIC) { + return ComparatorUtil.compareNumbers(tag1, b1, s1 + 1, tag2, b2, s2 + 1); } - - // if two tags are not compatible, then we compare raw byte by byte, including the type tag. + // currently only numbers are compatible. if two tags are not compatible, we compare the tags. // this is especially useful when we need to generate some order between any two types. - if ((!areTwoTagsEqual && !typePromotionApplied)) { - return rawComp.compare(b1, s1, l1, b2, s2, l2); + if (tag1 != tag2) { + return Byte.compare(b1[s1], b2[s2]); } - // conduct actual compare() - switch (targetTypeTag) { + switch (tag1) { + case STRING: + return ascStrComp.compare(b1, s1 + 1, l1 - 1, b2, s2 + 1, l2 - 1); case UUID: return ascUUIDComp.compare(b1, s1 + 1, l1 - 1, b2, s2 + 1, l2 - 1); case BOOLEAN: return ascBoolComp.compare(b1, s1 + 1, l1 - 1, b2, s2 + 1, l2 - 1); - case TINYINT: - // No type promotion from another type to the TINYINT can happen - return ascByteComp.compare(b1, s1 + 1, l1 - 1, b2, s2 + 1, l2 - 1); - case SMALLINT: { - if (!typePromotionApplied) { - // No type promotion case - return ascShortComp.compare(b1, s1 + 1, l1 - 1, b2, s2 + 1, l2 - 1); - } else if (leftValueChanged) { - // Type promotion happened. Left side was the source - return ascShortComp.compare(castBuffer.getByteArray(), castBuffer.getStartOffset() + 1, - castBuffer.getLength() - 1, b2, s2 + 1, l2 - 1); - } else { - // Type promotion happened. Right side was the source - return ascShortComp.compare(b1, s1 + 1, l1 - 1, castBuffer.getByteArray(), - castBuffer.getStartOffset() + 1, castBuffer.getLength() - 1); - } - } case TIME: + return Integer.compare(ATimeSerializerDeserializer.getChronon(b1, s1 + 1), + ATimeSerializerDeserializer.getChronon(b2, s2 + 1)); case DATE: + return Integer.compare(ADateSerializerDeserializer.getChronon(b1, s1 + 1), + ADateSerializerDeserializer.getChronon(b2, s2 + 1)); case YEARMONTHDURATION: - case INTEGER: { - if (!typePromotionApplied) { - // No type promotion case - return ascIntComp.compare(b1, s1 + 1, l1 - 1, b2, s2 + 1, l2 - 1); - } else if (leftValueChanged) { - // Type promotion happened. Left side was the source - return ascIntComp.compare(castBuffer.getByteArray(), castBuffer.getStartOffset() + 1, - castBuffer.getLength() - 1, b2, s2 + 1, l2 - 1); - } else { - // Type promotion happened. Right side was the source - return ascIntComp.compare(b1, s1 + 1, l1 - 1, castBuffer.getByteArray(), - castBuffer.getStartOffset() + 1, castBuffer.getLength() - 1); - } - } + return Integer.compare(AYearMonthDurationSerializerDeserializer.getYearMonth(b1, s1 + 1), + AYearMonthDurationSerializerDeserializer.getYearMonth(b2, s2 + 1)); case DATETIME: + return Long.compare(ADateTimeSerializerDeserializer.getChronon(b1, s1 + 1), + ADateTimeSerializerDeserializer.getChronon(b2, s2 + 1)); case DAYTIMEDURATION: - case BIGINT: { - if (!typePromotionApplied) { - // No type promotion case - return ascLongComp.compare(b1, s1 + 1, l1 - 1, b2, s2 + 1, l2 - 1); - } else if (leftValueChanged) { - // Type promotion happened. Left side was the source - return ascLongComp.compare(castBuffer.getByteArray(), castBuffer.getStartOffset() + 1, - castBuffer.getLength() - 1, b2, s2 + 1, l2 - 1); - } else { - // Type promotion happened. Right side was the source - return ascLongComp.compare(b1, s1 + 1, l1 - 1, castBuffer.getByteArray(), - castBuffer.getStartOffset() + 1, castBuffer.getLength() - 1); - } - } - case FLOAT: { - if (!typePromotionApplied) { - // No type promotion case - return ascFloatComp.compare(b1, s1 + 1, l1 - 1, b2, s2 + 1, l2 - 1); - } else if (leftValueChanged) { - // Type promotion happened. Left side was the source - return ascFloatComp.compare(castBuffer.getByteArray(), castBuffer.getStartOffset() + 1, - castBuffer.getLength() - 1, b2, s2 + 1, l2 - 1); - } else { - // Type promotion happened. Right side was the source - return ascFloatComp.compare(b1, s1 + 1, l1 - 1, castBuffer.getByteArray(), - castBuffer.getStartOffset() + 1, castBuffer.getLength() - 1); - } - } - case DOUBLE: { - if (!typePromotionApplied) { - // No type promotion case - return ascDoubleComp.compare(b1, s1 + 1, l1 - 1, b2, s2 + 1, l2 - 1); - } else if (leftValueChanged) { - // Type promotion happened. Left side was the source - return ascDoubleComp.compare(castBuffer.getByteArray(), castBuffer.getStartOffset() + 1, - castBuffer.getLength() - 1, b2, s2 + 1, l2 - 1); - } else { - // Type promotion happened. Right side was the source - return ascDoubleComp.compare(b1, s1 + 1, l1 - 1, castBuffer.getByteArray(), - castBuffer.getStartOffset() + 1, castBuffer.getLength() - 1); - } - } - case STRING: - return ascStrComp.compare(b1, s1 + 1, l1 - 1, b2, s2 + 1, l2 - 1); + return Long.compare(ADayTimeDurationSerializerDeserializer.getDayTime(b1, s1 + 1), + ADayTimeDurationSerializerDeserializer.getDayTime(b2, s2 + 1)); case RECTANGLE: return ascRectangleComp.compare(b1, s1 + 1, l1 - 1, b2, s2 + 1, l2 - 1); case CIRCLE: diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/LogicalComparatorUtil.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/ComparatorUtil.java similarity index 65% rename from asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/LogicalComparatorUtil.java rename to asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/ComparatorUtil.java index d074010..9cd2723 100644 --- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/LogicalComparatorUtil.java +++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/ComparatorUtil.java @@ -18,6 +18,7 @@ */ package org.apache.asterix.dataflow.data.nontagged.comparators; +import static org.apache.asterix.dataflow.data.common.ILogicalBinaryComparator.asResult; import static org.apache.asterix.om.types.ATypeTag.BIGINT; import static org.apache.asterix.om.types.ATypeTag.DOUBLE; import static org.apache.asterix.om.types.ATypeTag.FLOAT; @@ -48,9 +49,10 @@ import org.apache.asterix.om.types.hierachy.ATypeHierarchy; import org.apache.hyracks.data.std.api.IPointable; -public class LogicalComparatorUtil { +// TODO(ali): refactor some functionality with ATypeHierarchy and others +public class ComparatorUtil { - private LogicalComparatorUtil() { + private ComparatorUtil() { } public static ILogicalBinaryComparator createLogicalComparator(IAType left, IAType right, boolean isEquality) { @@ -80,92 +82,95 @@ return null; } - // checking that left and right are compatible has to be done before calling this + // checking that left and right are compatible and are numbers has to be done before calling this static Result compareNumbers(ATypeTag leftTag, IPointable left, ATypeTag rightTag, IPointable right) { - int result; - if (leftTag == DOUBLE || rightTag == DOUBLE) { - result = Double.compare(getDoubleValue(leftTag, left), getDoubleValue(rightTag, right)); - } else if (leftTag == FLOAT || rightTag == FLOAT) { - result = Float.compare((float) getDoubleValue(leftTag, left), (float) getDoubleValue(rightTag, right)); - } else if (leftTag == BIGINT || rightTag == BIGINT) { - result = Long.compare(getLongValue(leftTag, left), getLongValue(rightTag, right)); - } else if (leftTag == INTEGER || leftTag == SMALLINT || leftTag == TINYINT) { - result = Integer.compare((int) getLongValue(leftTag, left), (int) getLongValue(rightTag, right)); - } else { - return null; + return asResult(compareNumbers(leftTag, left.getByteArray(), left.getStartOffset() + 1, rightTag, + right.getByteArray(), right.getStartOffset() + 1)); + } + + // start args point to the value + static int compareNumbers(ATypeTag lTag, byte[] l, int lStart, ATypeTag rTag, byte[] r, int rStart) { + if (lTag == DOUBLE || rTag == DOUBLE) { + return Double.compare(getDoubleValue(lTag, l, lStart), getDoubleValue(rTag, r, rStart)); + } else if (lTag == FLOAT || rTag == FLOAT) { + return Float.compare((float) getDoubleValue(lTag, l, lStart), (float) getDoubleValue(rTag, r, rStart)); + } else if (lTag == BIGINT || rTag == BIGINT) { + return Long.compare(getLongValue(lTag, l, lStart), getLongValue(rTag, r, rStart)); + } else if (lTag == INTEGER || lTag == SMALLINT || lTag == TINYINT) { + return Integer.compare((int) getLongValue(lTag, l, lStart), (int) getLongValue(rTag, r, rStart)); } - return ILogicalBinaryComparator.asResult(result); + // TODO(ali): use unsupported type + throw new UnsupportedOperationException(); } // checking that left and right are compatible has to be done before calling this - static Result compareNumWithConstant(ATypeTag leftTag, IPointable left, IAObject rightConstant) { - int result; - ATypeTag rightTag = rightConstant.getType().getTypeTag(); + static Result compareNumWithConstant(ATypeTag leftTag, IPointable left, IAObject right) { + ATypeTag rightTag = right.getType().getTypeTag(); + byte[] leftBytes = left.getByteArray(); + int start = left.getStartOffset() + 1; if (leftTag == DOUBLE || rightTag == DOUBLE) { - result = Double.compare(getDoubleValue(leftTag, left), getConstantDouble(rightConstant)); + return asResult(Double.compare(getDoubleValue(leftTag, leftBytes, start), getConstantDouble(right))); } else if (leftTag == FLOAT || rightTag == FLOAT) { - result = Float.compare((float) getDoubleValue(leftTag, left), (float) getConstantDouble(rightConstant)); + return asResult( + Float.compare((float) getDoubleValue(leftTag, leftBytes, start), (float) getConstantDouble(right))); } else if (leftTag == BIGINT || rightTag == BIGINT) { - result = Long.compare(getLongValue(leftTag, left), getConstantLong(rightConstant)); + return asResult(Long.compare(getLongValue(leftTag, leftBytes, start), getConstantLong(right))); } else if (leftTag == INTEGER || leftTag == SMALLINT || leftTag == TINYINT) { - result = Integer.compare((int) getLongValue(leftTag, left), (int) getConstantLong(rightConstant)); - } else { - return null; + return asResult( + Integer.compare((int) getLongValue(leftTag, leftBytes, start), (int) getConstantLong(right))); } - return ILogicalBinaryComparator.asResult(result); + // TODO(ali): use unsupported type + throw new UnsupportedOperationException(); } // checking that left and right are compatible has to be done before calling this static Result compareConstants(IAObject leftConstant, IAObject rightConstant) { - int result; ATypeTag leftTag = leftConstant.getType().getTypeTag(); ATypeTag rightTag = rightConstant.getType().getTypeTag(); if (leftTag == DOUBLE || rightTag == DOUBLE) { - result = Double.compare(getConstantDouble(leftConstant), getConstantDouble(rightConstant)); + return asResult(Double.compare(getConstantDouble(leftConstant), getConstantDouble(rightConstant))); } else if (leftTag == FLOAT || rightTag == FLOAT) { - result = Float.compare((float) getConstantDouble(leftConstant), (float) getConstantDouble(rightConstant)); + return asResult( + Float.compare((float) getConstantDouble(leftConstant), (float) getConstantDouble(rightConstant))); } else if (leftTag == BIGINT || rightTag == BIGINT) { - result = Long.compare(getConstantLong(leftConstant), getConstantLong(rightConstant)); + return asResult(Long.compare(getConstantLong(leftConstant), getConstantLong(rightConstant))); } else if (leftTag == INTEGER || leftTag == SMALLINT || leftTag == TINYINT) { - result = Integer.compare((int) getConstantLong(leftConstant), (int) getConstantLong(rightConstant)); - } else { - return null; + return asResult(Integer.compare((int) getConstantLong(leftConstant), (int) getConstantLong(rightConstant))); } - return ILogicalBinaryComparator.asResult(result); + // TODO(ali): use unsupported type + throw new UnsupportedOperationException(); } - private static double getDoubleValue(ATypeTag numericTag, IPointable value) { - int start = value.getStartOffset() + 1; + private static double getDoubleValue(ATypeTag numericTag, byte[] bytes, int start) { switch (numericTag) { case TINYINT: - return AInt8SerializerDeserializer.getByte(value.getByteArray(), start); + return AInt8SerializerDeserializer.getByte(bytes, start); case SMALLINT: - return AInt16SerializerDeserializer.getShort(value.getByteArray(), start); + return AInt16SerializerDeserializer.getShort(bytes, start); case INTEGER: - return AInt32SerializerDeserializer.getInt(value.getByteArray(), start); + return AInt32SerializerDeserializer.getInt(bytes, start); case BIGINT: - return AInt64SerializerDeserializer.getLong(value.getByteArray(), start); + return AInt64SerializerDeserializer.getLong(bytes, start); case FLOAT: - return AFloatSerializerDeserializer.getFloat(value.getByteArray(), start); + return AFloatSerializerDeserializer.getFloat(bytes, start); case DOUBLE: - return ADoubleSerializerDeserializer.getDouble(value.getByteArray(), start); + return ADoubleSerializerDeserializer.getDouble(bytes, start); default: // TODO(ali): use unsupported type throw new UnsupportedOperationException(); } } - private static long getLongValue(ATypeTag numericTag, IPointable value) { - int start = value.getStartOffset() + 1; + private static long getLongValue(ATypeTag numericTag, byte[] bytes, int start) { switch (numericTag) { case TINYINT: - return AInt8SerializerDeserializer.getByte(value.getByteArray(), start); + return AInt8SerializerDeserializer.getByte(bytes, start); case SMALLINT: - return AInt16SerializerDeserializer.getShort(value.getByteArray(), start); + return AInt16SerializerDeserializer.getShort(bytes, start); case INTEGER: - return AInt32SerializerDeserializer.getInt(value.getByteArray(), start); + return AInt32SerializerDeserializer.getInt(bytes, start); case BIGINT: - return AInt64SerializerDeserializer.getLong(value.getByteArray(), start); + return AInt64SerializerDeserializer.getLong(bytes, start); default: // TODO(ali): use unsupported type throw new UnsupportedOperationException(); diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/LogicalComplexBinaryComparator.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/LogicalComplexBinaryComparator.java index e792e23..b83e248 100644 --- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/LogicalComplexBinaryComparator.java +++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/LogicalComplexBinaryComparator.java @@ -79,7 +79,7 @@ public Result compare(IPointable left, IPointable right) throws HyracksDataException { ATypeTag leftRuntimeTag = VALUE_TYPE_MAPPING[left.getByteArray()[left.getStartOffset()]]; ATypeTag rightRuntimeTag = VALUE_TYPE_MAPPING[right.getByteArray()[right.getStartOffset()]]; - Result comparisonResult = LogicalComparatorUtil.returnMissingOrNullOrMismatch(leftRuntimeTag, rightRuntimeTag); + Result comparisonResult = ComparatorUtil.returnMissingOrNullOrMismatch(leftRuntimeTag, rightRuntimeTag); if (comparisonResult != null) { return comparisonResult; } @@ -95,7 +95,7 @@ // TODO(ali): not defined currently for constant complex types ATypeTag leftTag = VALUE_TYPE_MAPPING[left.getByteArray()[left.getStartOffset()]]; ATypeTag rightTag = rightConstant.getType().getTypeTag(); - Result comparisonResult = LogicalComparatorUtil.returnMissingOrNullOrMismatch(leftTag, rightTag); + Result comparisonResult = ComparatorUtil.returnMissingOrNullOrMismatch(leftTag, rightTag); if (comparisonResult != null) { return comparisonResult; } @@ -122,7 +122,7 @@ // TODO(ali): not defined currently for constant complex types ATypeTag leftTag = leftConstant.getType().getTypeTag(); ATypeTag rightTag = rightConstant.getType().getTypeTag(); - Result comparisonResult = LogicalComparatorUtil.returnMissingOrNullOrMismatch(leftTag, rightTag); + Result comparisonResult = ComparatorUtil.returnMissingOrNullOrMismatch(leftTag, rightTag); if (comparisonResult != null) { return comparisonResult; } diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/LogicalScalarBinaryComparator.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/LogicalScalarBinaryComparator.java index f293193..61036f9 100644 --- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/LogicalScalarBinaryComparator.java +++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/comparators/LogicalScalarBinaryComparator.java @@ -39,6 +39,7 @@ import org.apache.asterix.formats.nontagged.BinaryComparatorFactoryProvider; import org.apache.asterix.om.base.IAObject; import org.apache.asterix.om.types.ATypeTag; +import org.apache.asterix.om.types.hierachy.ATypeHierarchy; import org.apache.hyracks.api.dataflow.value.IBinaryComparator; import org.apache.hyracks.api.exceptions.HyracksDataException; import org.apache.hyracks.data.std.accessors.PointableBinaryComparatorFactory; @@ -79,22 +80,20 @@ this.isEquality = isEquality; } - @SuppressWarnings("squid:S1226") // asking for introducing a new variable for incremented local variables @Override public Result compare(IPointable left, IPointable right) throws HyracksDataException { ATypeTag leftTag = VALUE_TYPE_MAPPING[left.getByteArray()[left.getStartOffset()]]; ATypeTag rightTag = VALUE_TYPE_MAPPING[right.getByteArray()[right.getStartOffset()]]; - Result comparisonResult = LogicalComparatorUtil.returnMissingOrNullOrMismatch(leftTag, rightTag); + Result comparisonResult = ComparatorUtil.returnMissingOrNullOrMismatch(leftTag, rightTag); if (comparisonResult != null) { return comparisonResult; } if (comparisonUndefined(leftTag, rightTag, isEquality)) { return Result.INCOMPARABLE; } - // compare number if one of args is number - comparisonResult = LogicalComparatorUtil.compareNumbers(leftTag, left, rightTag, right); - if (comparisonResult != null) { - return comparisonResult; + // compare number if one of args is number since compatibility has already been checked above + if (ATypeHierarchy.getTypeDomain(leftTag) == ATypeHierarchy.Domain.NUMERIC) { + return ComparatorUtil.compareNumbers(leftTag, left, rightTag, right); } // comparing non-numeric @@ -182,16 +181,15 @@ // TODO(ali): currently defined for numbers only ATypeTag leftTag = VALUE_TYPE_MAPPING[left.getByteArray()[left.getStartOffset()]]; ATypeTag rightTag = rightConstant.getType().getTypeTag(); - Result comparisonResult = LogicalComparatorUtil.returnMissingOrNullOrMismatch(leftTag, rightTag); + Result comparisonResult = ComparatorUtil.returnMissingOrNullOrMismatch(leftTag, rightTag); if (comparisonResult != null) { return comparisonResult; } if (comparisonUndefined(leftTag, rightTag, isEquality)) { return Result.NULL; } - comparisonResult = LogicalComparatorUtil.compareNumWithConstant(leftTag, left, rightConstant); - if (comparisonResult != null) { - return comparisonResult; + if (ATypeHierarchy.getTypeDomain(leftTag) == ATypeHierarchy.Domain.NUMERIC) { + return ComparatorUtil.compareNumWithConstant(leftTag, left, rightConstant); } return Result.NULL; } @@ -213,16 +211,15 @@ // TODO(ali): currently defined for numbers only ATypeTag leftTag = leftConstant.getType().getTypeTag(); ATypeTag rightTag = rightConstant.getType().getTypeTag(); - Result comparisonResult = LogicalComparatorUtil.returnMissingOrNullOrMismatch(leftTag, rightTag); + Result comparisonResult = ComparatorUtil.returnMissingOrNullOrMismatch(leftTag, rightTag); if (comparisonResult != null) { return comparisonResult; } if (comparisonUndefined(leftTag, rightTag, isEquality)) { return Result.NULL; } - comparisonResult = LogicalComparatorUtil.compareConstants(leftConstant, rightConstant); - if (comparisonResult != null) { - return comparisonResult; + if (ATypeHierarchy.getTypeDomain(leftTag) == ATypeHierarchy.Domain.NUMERIC) { + return ComparatorUtil.compareConstants(leftConstant, rightConstant); } return Result.NULL; } 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 cc33faa..2ef6e80 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 @@ -126,7 +126,6 @@ } return MurmurHash3BinaryHash.hash(valueBuffer.getByteArray(), valueBuffer.getStartOffset(), valueBuffer.getLength(), seed); - case FLOAT: try { FloatToDoubleTypeConvertComputer.getInstance().convertType(bytes, offset + 1, length - 1, @@ -136,9 +135,6 @@ } return MurmurHash3BinaryHash.hash(valueBuffer.getByteArray(), valueBuffer.getStartOffset(), valueBuffer.getLength(), seed); - - case DOUBLE: - return MurmurHash3BinaryHash.hash(bytes, offset, length, seed); case ARRAY: try { return hashArray(type, bytes, offset, length); @@ -147,6 +143,7 @@ } case OBJECT: return hashRecord(type, bytes, offset, length); + case DOUBLE: default: return MurmurHash3BinaryHash.hash(bytes, offset, length, seed); } @@ -160,7 +157,7 @@ IAType itemType = ((AbstractCollectionType) arrayType).getItemType(); ATypeTag itemTag = itemType.getTypeTag(); int numItems = ListAccessorUtil.numberOfItems(bytes, offset); - int hash = 0; + int hash = seed; IPointable item = voidPointableAllocator.allocate(null); ArrayBackedValueStorage storage = (ArrayBackedValueStorage) storageAllocator.allocate(null); try { @@ -192,7 +189,7 @@ IVisitablePointable fieldName, fieldValue; IAType fieldType; ATypeTag fieldTag; - int hash = 0; + int hash = seed; int fieldIdx; while (!namesHeap.isEmpty()) { fieldName = namesHeap.poll(); diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/ATypeTag.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/ATypeTag.java index f2b004f..941f59f 100644 --- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/ATypeTag.java +++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/ATypeTag.java @@ -107,8 +107,8 @@ * Serialized Tags end */ public static final int TYPE_COUNT = ATypeTag.values().length; - private byte value; public static final ATypeTag[] VALUE_TYPE_MAPPING; + private byte value; static { List<ATypeTag> typeList = new ArrayList<>(); @@ -122,7 +122,7 @@ VALUE_TYPE_MAPPING = typeList.toArray(new ATypeTag[typeList.size()]); } - private ATypeTag(int value) { + ATypeTag(int value) { this.value = (byte) value; } @@ -135,6 +135,7 @@ return this == ATypeTag.OBJECT || this == ATypeTag.ARRAY || this == ATypeTag.MULTISET || this == ATypeTag.UNION; } + // TODO(ali): remove and use ATypeHierarchy getTypeDomain() public final boolean isListType() { return this == ATypeTag.ARRAY || this == ATypeTag.MULTISET; } diff --git a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/AbstractComparisonEvaluator.java b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/AbstractComparisonEvaluator.java index 1d1b3e1..0134e6b 100644 --- a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/AbstractComparisonEvaluator.java +++ b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/AbstractComparisonEvaluator.java @@ -22,7 +22,7 @@ import org.apache.asterix.dataflow.data.common.ILogicalBinaryComparator; import org.apache.asterix.dataflow.data.common.ILogicalBinaryComparator.Result; -import org.apache.asterix.dataflow.data.nontagged.comparators.LogicalComparatorUtil; +import org.apache.asterix.dataflow.data.nontagged.comparators.ComparatorUtil; import org.apache.asterix.dataflow.data.nontagged.serde.ADoubleSerializerDeserializer; import org.apache.asterix.dataflow.data.nontagged.serde.AFloatSerializerDeserializer; import org.apache.asterix.dataflow.data.nontagged.serde.AInt16SerializerDeserializer; @@ -80,7 +80,7 @@ this.evalLeft = evalLeftFactory.createScalarEvaluator(ctx); this.evalRight = evalRightFactory.createScalarEvaluator(ctx); this.sourceLoc = sourceLoc; - logicalComparator = LogicalComparatorUtil.createLogicalComparator(leftType, rightType, isEquality); + logicalComparator = ComparatorUtil.createLogicalComparator(leftType, rightType, isEquality); leftConstant = getValueOfConstantEval(evalLeftFactory); rightConstant = getValueOfConstantEval(evalRightFactory); } -- To view, visit https://asterix-gerrit.ics.uci.edu/3272 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ie089d386a9ab8271f2833c05ffdfb0d484937b51 Gerrit-PatchSet: 4 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Ali Alsuliman <ali.al.solai...@gmail.com> Gerrit-Reviewer: Ali Alsuliman <ali.al.solai...@gmail.com> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <dmitry.lycha...@couchbase.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Till Westmann <ti...@apache.org>