>From Ian Maxon <[email protected]>: Ian Maxon has submitted this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21957?usp=email )
Change subject: [ASTERIXDB-3820][FUN] Make dimension a non-optional argument of isvector ...................................................................... [ASTERIXDB-3820][FUN] Make dimension a non-optional argument of isvector - user model changes: yes - storage format changes: no - interface changes: no Details: isvector(v) without the optional dimension is no longer a user-facing function. The one-argument form stays registered privately for internal use. A dimension that is not a positive integer gives null and a warning, as isbitset does. Generated-by: Claude Opus 5.5 <[email protected]> Change-Id: I9c430dceeda94efc9ec57883f6ac91739a4f4993 Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21957 Reviewed-by: Ian Maxon <[email protected]> Integration-Tests: Ian Maxon <[email protected]> Reviewed-by: Hongyu Shi <[email protected]> Tested-by: Ian Maxon <[email protected]> --- M asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/types/isvector/isvector.1.query.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/types/isvector/isvector.2.query.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/types/isvector/isvector.3.query.sqlpp M asterixdb/asterix-app/src/test/resources/runtimets/results/types/isvector/isvector.1.adm A asterixdb/asterix-app/src/test/resources/runtimets/results/types/isvector/isvector.3.adm M asterixdb/asterix-app/src/test/resources/runtimets/sqlpp_queries.xml M asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/BuiltinFunctions.java M asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/IsVectorDescriptor.java 8 files changed, 89 insertions(+), 16 deletions(-) Approvals: Anon. E. Moose #1000171: Hongyu Shi: Looks good to me, but someone else must approve Ian Maxon: Looks good to me, approved; Verified; Verified diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/types/isvector/isvector.1.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/types/isvector/isvector.1.query.sqlpp index 6450c2c..6f20b55 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/types/isvector/isvector.1.query.sqlpp +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/types/isvector/isvector.1.query.sqlpp @@ -17,10 +17,9 @@ * under the License. */ -// isvector(v) and isvector(v, dimension). Null and missing propagate as for the rest of the -// is_* family, so "k" is null and "l" is absent rather than both being false. +// isvector(v, dimension). Null and missing propagate as for the rest of the is_* family, so "k" is +// null and "l" is absent rather than both being false. { - "a_ok": isvector([0.2, 0.1, 0.4, 0.3]), "b_ok_dim": isvector([0.2, 0.1, 0.4, 0.3], 4), "c_mixed": isvector([0.2, 0.1, "s", 0.3], 4), "d_all_string": isvector(["a", "b", "c", "d"], 4), @@ -28,10 +27,9 @@ "f_nested": isvector([0.2, 0.1, [1], 0.3], 4), "g_short": isvector([0.2, 0.1, 0.3], 4), "h_empty_dim": isvector([], 4), - "i_empty_nodim": isvector([]), - "j_not_list": isvector("x"), - "k_null": isvector(null), - "l_missing": isvector(missing), + "j_not_list": isvector("x", 4), + "k_null": isvector(null, 4), + "l_missing": isvector(missing, 4), "m_integers": isvector([1, 2, 3], 3), "n_underscore": is_vector([1, 2, 3], 3) }; diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/types/isvector/isvector.2.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/types/isvector/isvector.2.query.sqlpp new file mode 100644 index 0000000..1df8262 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/types/isvector/isvector.2.query.sqlpp @@ -0,0 +1,21 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// The dimension is required: the one-argument form is not a user-facing function. +isvector([0.2, 0.1, 0.4, 0.3]); diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/types/isvector/isvector.3.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/types/isvector/isvector.3.query.sqlpp new file mode 100644 index 0000000..25a06e3 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/types/isvector/isvector.3.query.sqlpp @@ -0,0 +1,29 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// A dimension that is not a positive integer gives null and a warning rather than an answer. +// requesttype=application/json +// param max-warnings:json=10 + +{ + "a_zero_dim": isvector([1, 2, 3], 0), + "b_negative_dim": isvector([1, 2, 3], -1), + "c_fraction_dim": isvector([1, 2, 3], 2.5), + "d_string_dim": isvector([1, 2, 3], "3") +}; diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/types/isvector/isvector.1.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/types/isvector/isvector.1.adm index c9edfb0..5d617d8 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/results/types/isvector/isvector.1.adm +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/types/isvector/isvector.1.adm @@ -1 +1 @@ -{ "a_ok": true, "b_ok_dim": true, "c_mixed": false, "d_all_string": false, "e_null_elem": false, "f_nested": false, "g_short": false, "h_empty_dim": false, "i_empty_nodim": false, "j_not_list": false, "k_null": null, "m_integers": true, "n_underscore": true } +{ "b_ok_dim": true, "c_mixed": false, "d_all_string": false, "e_null_elem": false, "f_nested": false, "g_short": false, "h_empty_dim": false, "j_not_list": false, "k_null": null, "m_integers": true, "n_underscore": true } diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/types/isvector/isvector.3.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/types/isvector/isvector.3.adm new file mode 100644 index 0000000..235739a --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/types/isvector/isvector.3.adm @@ -0,0 +1 @@ +{ "a_zero_dim": null, "b_negative_dim": null, "c_fraction_dim": null, "d_string_dim": null } diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/sqlpp_queries.xml b/asterixdb/asterix-app/src/test/resources/runtimets/sqlpp_queries.xml index 55db4c0..666be3a 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/sqlpp_queries.xml +++ b/asterixdb/asterix-app/src/test/resources/runtimets/sqlpp_queries.xml @@ -15024,9 +15024,14 @@ <output-dir compare="Text">isarray</output-dir> </compilation-unit> </test-case> - <test-case FilePath="types"> + <test-case FilePath="types" check-warnings="true"> <compilation-unit name="isvector"> <output-dir compare="Text">isvector</output-dir> + <expected-error>ASX1081: Cannot find function with signature isvector(1)</expected-error> + <expected-warn>ASX0043: Value out of range, function asterix:is-vector#2 expects its 2nd input parameter value to be between 1 and 2147483647, received 0 (in line 25, at column 21)</expected-warn> + <expected-warn>ASX0043: Value out of range, function asterix:is-vector#2 expects its 2nd input parameter value to be between 1 and 2147483647, received -1 (in line 26, at column 21)</expected-warn> + <expected-warn>ASX0002: Type mismatch: function is-vector expects its 2nd input parameter to be of type bigint, but the actual input type is double (in line 27, at column 21)</expected-warn> + <expected-warn>ASX0002: Type mismatch: function is-vector expects its 2nd input parameter to be of type bigint, but the actual input type is string (in line 28, at column 21)</expected-warn> </compilation-unit> </test-case> <test-case FilePath="types"> diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/BuiltinFunctions.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/BuiltinFunctions.java index 5ebf2a2c..9102c41 100644 --- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/BuiltinFunctions.java +++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/BuiltinFunctions.java @@ -1431,7 +1431,7 @@ addFunction(IS_NUMBER, ABooleanTypeComputer.INSTANCE, true); addFunction(IS_STRING, ABooleanTypeComputer.INSTANCE, true); addFunction(IS_ARRAY, ABooleanTypeComputer.INSTANCE, true); - addFunction(IS_VECTOR, ABooleanTypeComputer.INSTANCE, true); + addPrivateFunction(IS_VECTOR, ABooleanTypeComputer.INSTANCE, true); addFunction(IS_VECTOR_WITH_DIMENSION, ABooleanTypeComputer.INSTANCE, true); addFunction(IS_OBJECT, ABooleanTypeComputer.INSTANCE, true); addFunction(IS_MULTISET, ABooleanTypeComputer.INSTANCE, true); diff --git a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/IsVectorDescriptor.java b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/IsVectorDescriptor.java index 61b7a8b..992dc8f 100644 --- a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/IsVectorDescriptor.java +++ b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/IsVectorDescriptor.java @@ -23,8 +23,10 @@ import org.apache.asterix.common.annotations.MissingNullInOutFunction; import org.apache.asterix.dataflow.data.nontagged.serde.AObjectSerializerDeserializer; import org.apache.asterix.om.base.ABoolean; +import org.apache.asterix.om.exceptions.ExceptionUtil; import org.apache.asterix.om.functions.BuiltinFunctions; import org.apache.asterix.om.functions.IFunctionDescriptorFactory; +import org.apache.asterix.om.types.ATypeTag; import org.apache.asterix.om.types.hierachy.ATypeHierarchy; import org.apache.asterix.runtime.evaluators.base.AbstractScalarFunctionDynamicDescriptor; import org.apache.asterix.runtime.evaluators.common.VectorValidator; @@ -50,7 +52,7 @@ * </pre> * <p> * Null and missing propagate as for {@code is_array} and the rest of the {@code is_*} family, so - * {@code isvector(null)} is {@code null} rather than {@code false}. Negating the call alone would + * {@code isvector(null, 4)} is {@code null} rather than {@code false}. Negating the call alone would * therefore leave out the rows whose field is null or absent, which the index skips too. * <p> * The function is <b>total</b> since {@link VectorValidator} never throws, which CLUSTER BY's usable-vector @@ -94,7 +96,7 @@ } } - private static final class IsVectorEvaluator implements IScalarEvaluator { + private final class IsVectorEvaluator implements IScalarEvaluator { private final ArrayBackedValueStorage resultStorage = new ArrayBackedValueStorage(); private final DataOutput out = resultStorage.getDataOutput(); @@ -103,8 +105,10 @@ private final IScalarEvaluator valueEval; private final IScalarEvaluator dimensionEval; private final VectorValidator validator = new VectorValidator(); + private final IEvaluatorContext ctx; private IsVectorEvaluator(IScalarEvaluatorFactory[] args, IEvaluatorContext ctx) throws HyracksDataException { + this.ctx = ctx; valueEval = args[0].createScalarEvaluator(ctx); dimensionEval = args.length > 1 ? args[1].createScalarEvaluator(ctx) : null; } @@ -122,10 +126,25 @@ return; } } - // Any dimension when none was given; getIntegerValue rejects a non-numeric second argument. - int dimension = dimensionEval == null ? -1 - : ATypeHierarchy.getIntegerValue(BuiltinFunctions.IS_VECTOR.getName(), 1, - dimensionPtr.getByteArray(), dimensionPtr.getStartOffset()); + // -1 means any length; only the internal one-argument form uses it. + int dimension = -1; + if (dimensionEval != null) { + byte[] bytes = dimensionPtr.getByteArray(); + int offset = dimensionPtr.getStartOffset(); + if (!PointableHelper.isValidLongValue(bytes, offset, true)) { + PointableHelper.setNull(result); + ExceptionUtil.warnTypeMismatch(ctx, sourceLoc, getIdentifier(), bytes[offset], 1, ATypeTag.BIGINT); + return; + } + dimension = ATypeHierarchy.getIntegerValue(getIdentifier().getName(), 1, bytes, offset); + // The validator would read 0 or a negative dimension as "any length". + if (dimension < 1) { + PointableHelper.setNull(result); + ExceptionUtil.warnValueOutOfRange(ctx, sourceLoc, getIdentifier(), 1, 1, Integer.MAX_VALUE, + dimension); + return; + } + } boolean isVector = validator.isVector(valuePtr.getByteArray(), valuePtr.getStartOffset(), valuePtr.getLength(), dimension); resultStorage.reset(); -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21957?usp=email To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: I9c430dceeda94efc9ec57883f6ac91739a4f4993 Gerrit-Change-Number: 21957 Gerrit-PatchSet: 6 Gerrit-Owner: Hongyu Shi <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Hongyu Shi <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]>
