[ASTERIXDB-2547][COMP] Disallow passing UNION tag to get comparator
- user model changes: no
- storage format changes: no
- interface changes: no
Details:
UNION should not be allowed when getting a comparator by tag.
The comparator provider returns a generic comparator with types
ANY when UNION is passed as a tag. This could cause problems if
the actual type is a complex type. UNION should not be allowed.
Change-Id: Id8816a0dc5584f0a27410c512f3a44ccfc6c3151
Reviewed-on: https://asterix-gerrit.ics.uci.edu/3404
Contrib: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: Dmitry Lychagin <dmitry.lychagin@couchbase.com>
diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java
index b8bfffd..c6c8a2f 100644
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java
@@ -1252,7 +1252,7 @@
if (oc.getRangeMap() != null) {
Iterator<OrderModifier> orderModifIter = oc.getModifierList().iterator();
boolean ascending = orderModifIter.next() == OrderModifier.ASC;
- RangeMapBuilder.verifyRangeOrder(oc.getRangeMap(), ascending);
+ RangeMapBuilder.verifyRangeOrder(oc.getRangeMap(), ascending, sourceLoc);
ord.getAnnotations().put(OperatorAnnotations.USE_STATIC_RANGE, oc.getRangeMap());
}
return new Pair<>(ord, null);
diff --git a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/RangeMapBuilder.java b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/RangeMapBuilder.java
index c505c1c..a26c94d 100644
--- a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/RangeMapBuilder.java
+++ b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/RangeMapBuilder.java
@@ -23,6 +23,7 @@
import org.apache.asterix.common.exceptions.CompilationException;
import org.apache.asterix.common.exceptions.ErrorCode;
+import org.apache.asterix.common.exceptions.RuntimeDataException;
import org.apache.asterix.formats.nontagged.BinaryComparatorFactoryProvider;
import org.apache.asterix.formats.nontagged.SerializerDeserializerProvider;
import org.apache.asterix.lang.common.base.Expression;
@@ -50,6 +51,7 @@
import org.apache.hyracks.api.dataflow.value.IBinaryComparatorFactory;
import org.apache.hyracks.api.dataflow.value.ISerializerDeserializer;
import org.apache.hyracks.api.exceptions.HyracksDataException;
+import org.apache.hyracks.api.exceptions.SourceLocation;
import org.apache.hyracks.data.std.util.ArrayBackedValueStorage;
import org.apache.hyracks.dataflow.common.data.partition.range.RangeMap;
@@ -145,19 +147,25 @@
}
}
- public static void verifyRangeOrder(RangeMap rangeMap, boolean ascending) throws CompilationException {
+ public static void verifyRangeOrder(RangeMap rangeMap, boolean ascending, SourceLocation sourceLoc)
+ throws CompilationException {
// TODO Add support for composite fields.
int fieldIndex = 0;
int fieldType = rangeMap.getTag(0, 0);
BinaryComparatorFactoryProvider comparatorFactory = BinaryComparatorFactoryProvider.INSTANCE;
- IBinaryComparatorFactory bcf =
- comparatorFactory.getBinaryComparatorFactory(ATypeTag.VALUE_TYPE_MAPPING[fieldType], ascending);
+ IBinaryComparatorFactory bcf;
+ try {
+ bcf = comparatorFactory.getBinaryComparatorFactory(ATypeTag.VALUE_TYPE_MAPPING[fieldType], ascending);
+ } catch (RuntimeDataException e) {
+ throw new CompilationException(ErrorCode.COMPILATION_ERROR, sourceLoc, e.getMessage());
+ }
IBinaryComparator comparator = bcf.createBinaryComparator();
int c = 0;
for (int split = 1; split < rangeMap.getSplitCount(); ++split) {
if (fieldType != rangeMap.getTag(fieldIndex, split)) {
- throw new CompilationException("Range field contains more than a single type of items (" + fieldType
- + " and " + rangeMap.getTag(fieldIndex, split) + ").");
+ throw new CompilationException(ErrorCode.COMPILATION_ERROR, sourceLoc,
+ "Range field contains more than a single type of items (" + fieldType + " and "
+ + rangeMap.getTag(fieldIndex, split) + ").");
}
int previousSplit = split - 1;
try {
@@ -165,10 +173,11 @@
rangeMap.getLength(fieldIndex, previousSplit), rangeMap.getByteArray(),
rangeMap.getStartOffset(fieldIndex, split), rangeMap.getLength(fieldIndex, split));
} catch (HyracksDataException e) {
- throw new CompilationException(e);
+ throw new CompilationException(ErrorCode.COMPILATION_ERROR, sourceLoc, e.getMessage());
}
if (c >= 0) {
- throw new CompilationException("Range fields are not in sorted order.");
+ throw new CompilationException(ErrorCode.COMPILATION_ERROR, sourceLoc,
+ "Range fields are not in sorted order.");
}
}
}
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 396bf3b..eea9fed 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
@@ -20,6 +20,8 @@
import java.io.Serializable;
+import org.apache.asterix.common.exceptions.ErrorCode;
+import org.apache.asterix.common.exceptions.RuntimeDataException;
import org.apache.asterix.dataflow.data.nontagged.comparators.ACirclePartialBinaryComparatorFactory;
import org.apache.asterix.dataflow.data.nontagged.comparators.ADurationPartialBinaryComparatorFactory;
import org.apache.asterix.dataflow.data.nontagged.comparators.AGenericAscBinaryComparatorFactory;
@@ -112,13 +114,13 @@
return createGenericBinaryComparatorFactory((IAType) leftType, (IAType) rightType, ascending);
}
- public IBinaryComparatorFactory getBinaryComparatorFactory(ATypeTag type, boolean ascending) {
+ public IBinaryComparatorFactory getBinaryComparatorFactory(ATypeTag type, boolean ascending)
+ throws RuntimeDataException {
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(BuiltinType.ANY, BuiltinType.ANY, ascending);
+ case UNION:
+ throw new RuntimeDataException(ErrorCode.TYPE_UNSUPPORTED, "Comparator", type);
case NULL:
case MISSING:
return new AnyBinaryComparatorFactory();