This is an automated email from the ASF dual-hosted git repository. mblow pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/asterixdb.git
commit 50f6830d965a7bd9d0850d108236ae68905892bc Author: Ali Alsuliman <[email protected]> AuthorDate: Tue Jan 26 21:22:05 2021 -0800 [ASTERIXDB-2689][FUN] Make field-access-nested return MISSING on wrong args - user model changes: no - storage format changes: no - interface changes: no Details: Make field-access-nested return MISSING on wrong args Change-Id: I59ba234e352408e1017e879443a22d00d3c2262a Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/9747 Integration-Tests: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> Reviewed-by: Ali Alsuliman <[email protected]> Reviewed-by: Dmitry Lychagin <[email protected]> --- .../impl/FieldAccessNestedResultType.java | 38 +++---------------- .../records/FieldAccessNestedDescriptor.java | 2 +- .../records/FieldAccessNestedEvalFactory.java | 43 +++++++++++++--------- .../records/GetRecordFieldValueEvalFactory.java | 5 ++- .../runtime/functions/FunctionTypeInferers.java | 10 ++--- 5 files changed, 39 insertions(+), 59 deletions(-) diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/FieldAccessNestedResultType.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/FieldAccessNestedResultType.java index 2f3965d..04f23d4 100644 --- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/FieldAccessNestedResultType.java +++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/FieldAccessNestedResultType.java @@ -25,7 +25,6 @@ import org.apache.asterix.om.base.AOrderedList; import org.apache.asterix.om.base.AString; import org.apache.asterix.om.base.IAObject; import org.apache.asterix.om.constants.AsterixConstantValue; -import org.apache.asterix.om.exceptions.TypeMismatchException; import org.apache.asterix.om.typecomputer.base.AbstractResultTypeComputer; import org.apache.asterix.om.types.AOrderedListType; import org.apache.asterix.om.types.ARecordType; @@ -37,8 +36,6 @@ import org.apache.hyracks.algebricks.core.algebra.base.ILogicalExpression; import org.apache.hyracks.algebricks.core.algebra.base.LogicalExpressionTag; import org.apache.hyracks.algebricks.core.algebra.expressions.AbstractFunctionCallExpression; import org.apache.hyracks.algebricks.core.algebra.expressions.ConstantExpression; -import org.apache.hyracks.algebricks.core.algebra.functions.FunctionIdentifier; -import org.apache.hyracks.api.exceptions.SourceLocation; public class FieldAccessNestedResultType extends AbstractResultTypeComputer { public static final FieldAccessNestedResultType INSTANCE = new FieldAccessNestedResultType(); @@ -47,34 +44,6 @@ public class FieldAccessNestedResultType extends AbstractResultTypeComputer { } @Override - protected void checkArgType(FunctionIdentifier funcId, int argIndex, IAType type, SourceLocation sourceLoc) - throws AlgebricksException { - ATypeTag actualTypeTag = type.getTypeTag(); - if (argIndex == 0 && actualTypeTag != ATypeTag.OBJECT) { - throw new TypeMismatchException(sourceLoc, actualTypeTag, ATypeTag.OBJECT); - } - if (argIndex == 1) { - switch (actualTypeTag) { - case STRING: - break; - case ARRAY: - checkOrderedList(type, sourceLoc); - break; - default: - throw new TypeMismatchException(sourceLoc, actualTypeTag, ATypeTag.STRING, ATypeTag.ARRAY); - } - } - } - - private void checkOrderedList(IAType type, SourceLocation sourceLoc) throws AlgebricksException { - AOrderedListType listType = (AOrderedListType) type; - ATypeTag itemTypeTag = listType.getItemType().getTypeTag(); - if (itemTypeTag != ATypeTag.STRING && itemTypeTag != ATypeTag.ANY) { - throw new TypeMismatchException(sourceLoc, itemTypeTag, ATypeTag.STRING, ATypeTag.ANY); - } - } - - @Override protected IAType getResultType(ILogicalExpression expr, IAType... strippedInputTypes) throws AlgebricksException { IAType firstArgType = strippedInputTypes[0]; if (firstArgType.getTypeTag() != ATypeTag.OBJECT) { @@ -88,12 +57,15 @@ public class FieldAccessNestedResultType extends AbstractResultTypeComputer { ConstantExpression ce = (ConstantExpression) arg1; IAObject v = ((AsterixConstantValue) ce.getValue()).getObject(); List<String> fieldPath = new ArrayList<>(); - if (v.getType().getTypeTag() == ATypeTag.ARRAY) { + ATypeTag tag = v.getType().getTypeTag(); + if (tag == ATypeTag.ARRAY && ((AOrderedListType) v.getType()).getItemType().getTypeTag() == ATypeTag.STRING) { for (int i = 0; i < ((AOrderedList) v).size(); i++) { fieldPath.add(((AString) ((AOrderedList) v).getItem(i)).getStringValue()); } - } else { + } else if (tag == ATypeTag.STRING) { fieldPath.add(((AString) v).getStringValue()); + } else { + return BuiltinType.ANY; } ARecordType recType = (ARecordType) firstArgType; IAType fieldType = recType.getSubFieldType(fieldPath); diff --git a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/FieldAccessNestedDescriptor.java b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/FieldAccessNestedDescriptor.java index 3871fa2..27be285 100644 --- a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/FieldAccessNestedDescriptor.java +++ b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/FieldAccessNestedDescriptor.java @@ -63,6 +63,6 @@ public class FieldAccessNestedDescriptor extends AbstractScalarFunctionDynamicDe @Override public IScalarEvaluatorFactory createEvaluatorFactory(IScalarEvaluatorFactory[] args) { - return new FieldAccessNestedEvalFactory(args[0], recType, fldName, sourceLoc); + return new FieldAccessNestedEvalFactory(args[0], recType, fldName, sourceLoc, getIdentifier()); } } diff --git a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/FieldAccessNestedEvalFactory.java b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/FieldAccessNestedEvalFactory.java index ef958f1..30f3fde 100644 --- a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/FieldAccessNestedEvalFactory.java +++ b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/FieldAccessNestedEvalFactory.java @@ -29,6 +29,7 @@ import org.apache.asterix.formats.nontagged.SerializerDeserializerProvider; import org.apache.asterix.om.base.AMissing; import org.apache.asterix.om.base.ANull; import org.apache.asterix.om.base.AString; +import org.apache.asterix.om.exceptions.ExceptionUtil; import org.apache.asterix.om.types.ARecordType; import org.apache.asterix.om.types.ATypeTag; import org.apache.asterix.om.types.AUnionType; @@ -39,7 +40,7 @@ import org.apache.asterix.om.types.runtime.RuntimeRecordTypeInfo; import org.apache.asterix.om.utils.NonTaggedFormatUtil; import org.apache.asterix.om.utils.RecordUtil; import org.apache.asterix.runtime.evaluators.functions.PointableHelper; -import org.apache.asterix.runtime.exceptions.TypeMismatchException; +import org.apache.hyracks.algebricks.core.algebra.functions.FunctionIdentifier; import org.apache.hyracks.algebricks.runtime.base.IEvaluatorContext; import org.apache.hyracks.algebricks.runtime.base.IScalarEvaluator; import org.apache.hyracks.algebricks.runtime.base.IScalarEvaluatorFactory; @@ -62,13 +63,15 @@ public class FieldAccessNestedEvalFactory implements IScalarEvaluatorFactory { private final ARecordType recordType; private final List<String> fieldPath; private final SourceLocation sourceLoc; + private final FunctionIdentifier funID; public FieldAccessNestedEvalFactory(IScalarEvaluatorFactory recordEvalFactory, ARecordType recordType, - List<String> fldName, SourceLocation sourceLoc) { + List<String> fldName, SourceLocation sourceLoc, FunctionIdentifier funID) { this.recordEvalFactory = recordEvalFactory; this.recordType = recordType; this.fieldPath = fldName; this.sourceLoc = sourceLoc; + this.funID = funID; } @Override @@ -79,7 +82,7 @@ public class FieldAccessNestedEvalFactory implements IScalarEvaluatorFactory { BinaryHashFunctionFactoryProvider.UTF8STRING_POINTABLE_INSTANCE.createBinaryHashFunction(); private final IBinaryComparator fieldNameComparator = BinaryComparatorFactoryProvider.UTF8STRING_POINTABLE_INSTANCE.createBinaryComparator(); - private ArrayBackedValueStorage resultStorage = new ArrayBackedValueStorage(); + private final ArrayBackedValueStorage resultStorage = new ArrayBackedValueStorage(); private final DataOutput out = resultStorage.getDataOutput(); private final ByteArrayAccessibleOutputStream subRecordTmpStream = new ByteArrayAccessibleOutputStream(); @@ -125,13 +128,14 @@ public class FieldAccessNestedEvalFactory implements IScalarEvaluatorFactory { } byte[] serRecord = inputArg0.getByteArray(); - int offset = inputArg0.getStartOffset(); - int start = offset; + int start = inputArg0.getStartOffset(); int len = inputArg0.getLength(); if (serRecord[start] != ATypeTag.SERIALIZED_RECORD_TYPE_TAG) { - throw new TypeMismatchException(sourceLoc, serRecord[start], - ATypeTag.SERIALIZED_RECORD_TYPE_TAG); + ExceptionUtil.warnTypeMismatch(ctx, sourceLoc, funID, serRecord[start], 0, ATypeTag.OBJECT); + missingSerde.serialize(AMissing.MISSING, out); + result.set(resultStorage); + return; } int subFieldIndex = -1; @@ -143,7 +147,6 @@ public class FieldAccessNestedEvalFactory implements IScalarEvaluatorFactory { recTypeInfos[0].reset(recordType); ATypeTag subTypeTag = ATypeTag.MISSING; - boolean openField = false; int pathIndex = 0; // Moving through closed fields first. @@ -153,12 +156,13 @@ public class FieldAccessNestedEvalFactory implements IScalarEvaluatorFactory { subType = ((AUnionType) subType).getActualType(); byte serializedTypeTag = subType.getTypeTag().serialize(); if (serializedTypeTag != ATypeTag.SERIALIZED_RECORD_TYPE_TAG) { - throw new TypeMismatchException(sourceLoc, serializedTypeTag, - ATypeTag.SERIALIZED_RECORD_TYPE_TAG); - } - if (subType.getTypeTag() == ATypeTag.OBJECT) { - recTypeInfos[pathIndex].reset((ARecordType) subType); + ExceptionUtil.warnTypeMismatch(ctx, sourceLoc, funID, serializedTypeTag, 0, + ATypeTag.OBJECT); + missingSerde.serialize(AMissing.MISSING, out); + result.set(resultStorage); + return; } + recTypeInfos[pathIndex].reset((ARecordType) subType); } subFieldIndex = recTypeInfos[pathIndex].getFieldIndex(fieldPointables[pathIndex].getByteArray(), fieldPointables[pathIndex].getStartOffset() + 1, @@ -209,12 +213,15 @@ public class FieldAccessNestedEvalFactory implements IScalarEvaluatorFactory { // type check if (pathIndex < fieldPointables.length - 1 && serRecord[start] != ATypeTag.SERIALIZED_RECORD_TYPE_TAG) { - throw new TypeMismatchException(sourceLoc, serRecord[start], - ATypeTag.SERIALIZED_RECORD_TYPE_TAG); + ExceptionUtil.warnTypeMismatch(ctx, sourceLoc, funID, serRecord[start], 0, ATypeTag.OBJECT); + missingSerde.serialize(AMissing.MISSING, out); + result.set(resultStorage); + return; } } // Moving through open fields after we hit the first open field. + boolean openField = false; for (; pathIndex < fieldPointables.length; pathIndex++) { openField = true; subFieldOffset = ARecordSerializerDeserializer.getFieldOffsetByName(serRecord, start, len, @@ -245,8 +252,10 @@ public class FieldAccessNestedEvalFactory implements IScalarEvaluatorFactory { return; } if (serRecord[start] != ATypeTag.SERIALIZED_RECORD_TYPE_TAG) { - throw new TypeMismatchException(sourceLoc, serRecord[start], - ATypeTag.SERIALIZED_RECORD_TYPE_TAG); + ExceptionUtil.warnTypeMismatch(ctx, sourceLoc, funID, serRecord[start], 0, ATypeTag.OBJECT); + missingSerde.serialize(AMissing.MISSING, out); + result.set(resultStorage); + return; } } // emit the final result. diff --git a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/GetRecordFieldValueEvalFactory.java b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/GetRecordFieldValueEvalFactory.java index d6c0e21..f8dacfa 100644 --- a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/GetRecordFieldValueEvalFactory.java +++ b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/GetRecordFieldValueEvalFactory.java @@ -98,11 +98,14 @@ public class GetRecordFieldValueEvalFactory implements IScalarEvaluatorFactory { byte[] serFldName = inputArg1.getByteArray(); int serFldNameOffset = inputArg1.getStartOffset(); int serFldNameLen = inputArg1.getLength(); + if (serFldName[serFldNameOffset] != ATypeTag.SERIALIZED_STRING_TYPE_TAG) { + throw new TypeMismatchException(sourceLoc, BuiltinFunctions.GET_RECORD_FIELD_VALUE, 0, + serFldName[serFldNameOffset], ATypeTag.SERIALIZED_STRING_TYPE_TAG); + } byte[] serRecord = inputArg0.getByteArray(); int serRecordOffset = inputArg0.getStartOffset(); int serRecordLen = inputArg0.getLength(); - if (serRecord[serRecordOffset] != ATypeTag.SERIALIZED_RECORD_TYPE_TAG) { throw new TypeMismatchException(sourceLoc, BuiltinFunctions.GET_RECORD_FIELD_VALUE, 0, serRecord[serRecordOffset], ATypeTag.SERIALIZED_RECORD_TYPE_TAG); diff --git a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/functions/FunctionTypeInferers.java b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/functions/FunctionTypeInferers.java index ebb0717..2af4de5 100644 --- a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/functions/FunctionTypeInferers.java +++ b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/functions/FunctionTypeInferers.java @@ -163,7 +163,7 @@ public final class FunctionTypeInferers { public void infer(ILogicalExpression expr, IFunctionDescriptor fd, IVariableTypeEnvironment context, CompilerProperties compilerProps) throws AlgebricksException { AbstractFunctionCallExpression fce = (AbstractFunctionCallExpression) expr; - IAType t = (IAType) context.getType(fce.getArguments().get(0).getValue()); + // arg 1 should always be a constant array of strings AOrderedList fieldPath = (AOrderedList) (((AsterixConstantValue) ((ConstantExpression) fce.getArguments().get(1).getValue()) .getValue()).getObject()); @@ -171,17 +171,13 @@ public final class FunctionTypeInferers { for (int i = 0; i < fieldPath.size(); i++) { listFieldPath.add(((AString) fieldPath.getItem(i)).getStringValue()); } - - // TODO(ali): I guess this may not work as well if t happens to be UNION(record), not sure if it ever does + IAType t = TypeComputeUtils.getActualType((IAType) context.getType(fce.getArguments().get(0).getValue())); switch (t.getTypeTag()) { case OBJECT: fd.setImmutableStates(t, listFieldPath); break; - case ANY: - fd.setImmutableStates(RecordUtil.FULLY_OPEN_RECORD_TYPE, listFieldPath); - break; default: - fd.setImmutableStates(null, listFieldPath); + fd.setImmutableStates(RecordUtil.FULLY_OPEN_RECORD_TYPE, listFieldPath); break; } }
