You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@asterixdb.apache.org by al...@apache.org on 2019/05/29 03:04:25 UTC
[asterixdb] branch master updated: [ASTERIXDB-2547][COMP] Disallow
passing UNION tag to get comparator
This is an automated email from the ASF dual-hosted git repository.
alsuliman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git
The following commit(s) were added to refs/heads/master by this push:
new f76b0f2 [ASTERIXDB-2547][COMP] Disallow passing UNION tag to get comparator
f76b0f2 is described below
commit f76b0f2ed6de78e222864004b638c17927c67d09
Author: Ali Alsuliman <al...@gmail.com>
AuthorDate: Mon May 27 20:04:53 2019 -0700
[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 <je...@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
Reviewed-by: Dmitry Lychagin <dm...@couchbase.com>
---
.../translator/LangExpressionToPlanTranslator.java | 2 +-
.../asterix/lang/common/util/RangeMapBuilder.java | 23 +++++++++++++++-------
.../nontagged/BinaryComparatorFactoryProvider.java | 10 ++++++----
3 files changed, 23 insertions(+), 12 deletions(-)
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 @@ class LangExpressionToPlanTranslator
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 java.util.List;
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.IBinaryComparator;
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 class RangeMapBuilder {
}
}
- 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 @@ public class RangeMapBuilder {
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 @@ package org.apache.asterix.formats.nontagged;
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 @@ public class BinaryComparatorFactoryProvider implements IBinaryComparatorFactory
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();