This is an automated email from the ASF dual-hosted git repository. htowaileb 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 e5c0d8d [ASTERIXDB-2625][OTH] Proper error message for invalid uuid e5c0d8d is described below commit e5c0d8d65ff7504f304c8ed0fa5ff11ad84fd27a Author: Hussain Towaileb <hussain.towai...@couchbase.com> AuthorDate: Wed Sep 4 18:13:15 2019 +0300 [ASTERIXDB-2625][OTH] Proper error message for invalid uuid - user model changes: no - storage format changes: no - interface changes: no Details: - Returns a proper message for invalid uuid. (incorrect uuid length) - Add a setter to allow setting the SourceLocation to an already created exception. Change-Id: I2cd836afcb35955e23e6cba7da5e17917d0705b0 Reviewed-on: https://asterix-gerrit.ics.uci.edu/3537 Contrib: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Tested-by: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Integration-Tests: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Reviewed-by: Murtadha Hubail <mhub...@apache.org> --- .../constructor/{ => uuid}/uuid_01/uuid_01.1.ddl.sqlpp | 0 .../{ => uuid}/uuid_01/uuid_01.2.update.sqlpp | 0 .../{ => uuid}/uuid_01/uuid_01.3.query.sqlpp | 0 .../uuid_02/uuid_02.1.query.sqlpp} | 4 +--- .../uuid_03/uuid_03.1.query.sqlpp} | 1 + .../constructor/{ => uuid}/uuid_01/uuid_01.1.adm | 0 .../src/test/resources/runtimets/testsuite_sqlpp.xml | 14 +++++++++++++- .../java/org/apache/asterix/om/base/AMutableUUID.java | 17 ++++++++++++----- .../AUUIDFromStringConstructorDescriptor.java | 18 ++++-------------- .../hyracks/api/exceptions/HyracksException.java | 6 +++++- 10 files changed, 36 insertions(+), 24 deletions(-) diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid_01/uuid_01.1.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid/uuid_01/uuid_01.1.ddl.sqlpp similarity index 100% copy from asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid_01/uuid_01.1.ddl.sqlpp copy to asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid/uuid_01/uuid_01.1.ddl.sqlpp diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid_01/uuid_01.2.update.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid/uuid_01/uuid_01.2.update.sqlpp similarity index 100% copy from asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid_01/uuid_01.2.update.sqlpp copy to asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid/uuid_01/uuid_01.2.update.sqlpp diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid_01/uuid_01.3.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid/uuid_01/uuid_01.3.query.sqlpp similarity index 100% rename from asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid_01/uuid_01.3.query.sqlpp rename to asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid/uuid_01/uuid_01.3.query.sqlpp diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid_01/uuid_01.1.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid/uuid_02/uuid_02.1.query.sqlpp similarity index 92% rename from asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid_01/uuid_01.1.ddl.sqlpp rename to asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid/uuid_02/uuid_02.1.query.sqlpp index 21479a2..a11fd8b 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid_01/uuid_01.1.ddl.sqlpp +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid/uuid_02/uuid_02.1.query.sqlpp @@ -17,6 +17,4 @@ * under the License. */ -drop dataverse test if exists; -create dataverse test; - +uuid('02a199ca-bf58-412e-bd9f-60a0c975a8a-'); // Invalid format diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid_01/uuid_01.2.update.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid/uuid_03/uuid_03.1.query.sqlpp similarity index 96% rename from asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid_01/uuid_01.2.update.sqlpp rename to asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid/uuid_03/uuid_03.1.query.sqlpp index bd244d0..a9fb88e 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid_01/uuid_01.2.update.sqlpp +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/constructor/uuid/uuid_03/uuid_03.1.query.sqlpp @@ -17,3 +17,4 @@ * under the License. */ +uuid('12345'); // Invalid length diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/constructor/uuid_01/uuid_01.1.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/constructor/uuid/uuid_01/uuid_01.1.adm similarity index 100% rename from asterixdb/asterix-app/src/test/resources/runtimets/results/constructor/uuid_01/uuid_01.1.adm rename to asterixdb/asterix-app/src/test/resources/runtimets/results/constructor/uuid/uuid_01/uuid_01.1.adm diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml index 91c86f2..a3c9508 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml +++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml @@ -3344,11 +3344,23 @@ <output-dir compare="Text">time_01</output-dir> </compilation-unit> </test-case> - <test-case FilePath="constructor"> + <test-case FilePath="constructor/uuid"> <compilation-unit name="uuid_01"> <output-dir compare="Text">uuid_01</output-dir> </compilation-unit> </test-case> + <test-case FilePath="constructor/uuid"> + <compilation-unit name="uuid_02"> + <output-dir compare="Text">uuid_02</output-dir> + <expected-error>Invalid format for uuid in 02a199ca-bf58-412e-bd9f-60a0c975a8a-</expected-error> + </compilation-unit> + </test-case> + <test-case FilePath="constructor/uuid"> + <compilation-unit name="uuid_03"> + <output-dir compare="Text">uuid_03</output-dir> + <expected-error>Invalid format for uuid in 12345</expected-error> + </compilation-unit> + </test-case> </test-group> <test-group name="custord"> <!-- diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/base/AMutableUUID.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/base/AMutableUUID.java index 9a097dc..e5e810a 100644 --- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/base/AMutableUUID.java +++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/base/AMutableUUID.java @@ -19,7 +19,10 @@ package org.apache.asterix.om.base; +import org.apache.asterix.common.exceptions.ErrorCode; +import org.apache.asterix.common.exceptions.RuntimeDataException; import org.apache.hyracks.api.exceptions.HyracksDataException; +import org.apache.hyracks.util.string.UTF8StringUtil; public class AMutableUUID extends AUUID { @@ -71,9 +74,14 @@ public class AMutableUUID extends AUUID { // Calculate a long value from a hex string. private static void decodeBytesFromHex(byte[] hexArray, int hexArrayOffset, byte[] outputArray, int outputOffset, int length) throws HyracksDataException { - for (int i = hexArrayOffset; i < hexArrayOffset + length;) { - int hi = transformHexCharToInt(hexArray[i++]); - outputArray[outputOffset++] = (byte) (hi << 4 | transformHexCharToInt(hexArray[i++])); + try { + for (int i = hexArrayOffset; i < hexArrayOffset + length;) { + int hi = transformHexCharToInt(hexArray[i++]); + outputArray[outputOffset++] = (byte) (hi << 4 | transformHexCharToInt(hexArray[i++])); + } + } catch (Exception ex) { + // Can also happen in case of invalid length, out of bound exception + throw new RuntimeDataException(ErrorCode.INVALID_FORMAT, "uuid", UTF8StringUtil.toString(hexArray, 1)); } } @@ -119,8 +127,7 @@ public class AMutableUUID extends AUUID { case 'F': return 15; default: - throw new HyracksDataException("This is not a correct UUID value."); + throw new RuntimeDataException(ErrorCode.INVALID_FORMAT); } } - } \ No newline at end of file diff --git a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/constructors/AUUIDFromStringConstructorDescriptor.java b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/constructors/AUUIDFromStringConstructorDescriptor.java index bfc7177..6693877 100644 --- a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/constructors/AUUIDFromStringConstructorDescriptor.java +++ b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/constructors/AUUIDFromStringConstructorDescriptor.java @@ -19,20 +19,17 @@ package org.apache.asterix.runtime.evaluators.constructors; import java.io.DataOutput; -import java.io.IOException; import org.apache.asterix.common.annotations.MissingNullInOutFunction; import org.apache.asterix.formats.nontagged.SerializerDeserializerProvider; import org.apache.asterix.om.base.AMutableUUID; import org.apache.asterix.om.base.AUUID; import org.apache.asterix.om.functions.BuiltinFunctions; -import org.apache.asterix.om.functions.IFunctionDescriptor; import org.apache.asterix.om.functions.IFunctionDescriptorFactory; import org.apache.asterix.om.types.ATypeTag; import org.apache.asterix.om.types.BuiltinType; import org.apache.asterix.runtime.evaluators.base.AbstractScalarFunctionDynamicDescriptor; import org.apache.asterix.runtime.evaluators.functions.PointableHelper; -import org.apache.asterix.runtime.exceptions.InvalidDataFormatException; import org.apache.asterix.runtime.exceptions.TypeMismatchException; import org.apache.hyracks.algebricks.core.algebra.functions.FunctionIdentifier; import org.apache.hyracks.algebricks.runtime.base.IEvaluatorContext; @@ -55,12 +52,7 @@ import org.apache.hyracks.dataflow.common.data.accessors.IFrameTupleReference; @MissingNullInOutFunction public class AUUIDFromStringConstructorDescriptor extends AbstractScalarFunctionDynamicDescriptor { private static final long serialVersionUID = 1L; - public static final IFunctionDescriptorFactory FACTORY = new IFunctionDescriptorFactory() { - @Override - public IFunctionDescriptor createFunctionDescriptor() { - return new AUUIDFromStringConstructorDescriptor(); - } - }; + public static final IFunctionDescriptorFactory FACTORY = AUUIDFromStringConstructorDescriptor::new; @Override public IScalarEvaluatorFactory createEvaluatorFactory(final IScalarEvaluatorFactory[] args) { @@ -111,15 +103,13 @@ public class AUUIDFromStringConstructorDescriptor extends AbstractScalarFunction throw new TypeMismatchException(sourceLoc, getIdentifier(), 0, tt, ATypeTag.SERIALIZED_STRING_TYPE_TAG); } - } catch (IOException e) { - throw new InvalidDataFormatException(sourceLoc, getIdentifier(), e, - ATypeTag.SERIALIZED_UUID_TYPE_TAG); + } catch (HyracksDataException e) { + e.setSourceLocation(sourceLoc); + throw e; } } - }; } - }; } diff --git a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/HyracksException.java b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/HyracksException.java index 69601cf..89e46de 100644 --- a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/HyracksException.java +++ b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/HyracksException.java @@ -32,7 +32,7 @@ public class HyracksException extends IOException implements IFormattedException private final int errorCode; private final Serializable[] params; private final String nodeId; - private final SourceLocation sourceLoc; + private SourceLocation sourceLoc; private transient volatile String msgCache; public static HyracksException create(Throwable cause) { @@ -142,6 +142,10 @@ public class HyracksException extends IOException implements IFormattedException return sourceLoc; } + public void setSourceLocation(SourceLocation sourceLocation) { + this.sourceLoc = sourceLocation; + } + @Override public String getMessage() { if (msgCache == null) {