>From Ali Alsuliman <[email protected]>: Attention is currently required from: Calvin Thomas Dani.
Ali Alsuliman has posted comments on this change by Calvin Thomas Dani. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126?usp=email ) Change subject: [ASTERIXDB-3676] Vector Distance Functions ...................................................................... Patch Set 30: (12 comments) File asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/CommonFunctionMapUtil.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/793d844a_3bc52db9?usp=email : PS30, Line 170: addFunctionMapping("l2_distance", "euclidean-distance"); Just checking, is the below going to also work? ``` `l2-distance()` ``` File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/BuiltinFunctions.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/238e9593_a515bb78?usp=email : PS30, Line 1200: public static final FunctionIdentifier EUCLIDEAN_DISTANCE_ARRAY = Change `EUCLIDEAN_DISTANCE_ARRAY` -> `EUCLIDEAN_DISTANCE`. Do the same for the others. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/728b45a0_e7074b32?usp=email : PS30, Line 1202: // public static final FunctionIdentifier L2_DISTANCE_ARRAY = FunctionConstants.newAsterix("l2-distance", 2); Remove commented out code. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/293d21b3_5b6312cb?usp=email : PS30, Line 1930: addFunction(EUCLIDEAN_DISTANCE_ARRAY, ADoubleTypeComputer.INSTANCE_NULLABLE, true); You should use `ADoubleTypeComputer.INSTANCE`. File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/vector/CosineDistanceArrDescriptor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/eba2e723_03aa5229?usp=email : PS30, Line 31: public class CosineDistanceArrDescriptor extends AbstractScalarFunctionDynamicDescriptor { Annotate them all with `@MissingNullInOutFunction` since you return MISSING/NULL if any arg is MISSING/NULL. Also rename from `CosineDistanceArrDescriptor` to `CosineDistanceDescriptor`. Do the same for the others. File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/vector/VectorDistanceArrScalarEvaluator.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/34acde37_1e9f2a18?usp=email : PS30, Line 48: public class VectorDistanceArrScalarEvaluator implements IScalarEvaluator { Rename `VectorDistanceArrScalarEvaluator` -> `VectorDistanceEvaluator` https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/2d9ddd5a_fa8878fb?usp=email : PS30, Line 236: if (PointableHelper.checkAndSetMissingOrNull(result, pointableLeft, pointableRight)) { We want the check for the MissingOrNull to happen the first thing on the two args to maintain the semantics of functions that return MISSING or NULL if either argument is MISSING or NULL. So, you will have to call at the top before anything (if not constant): ``` evaluatorLeft.evaluate(tuple, pointableLeft); evaluatorRight.evaluate(tuple, pointableRight); if (PointableHelper.checkAndSetMissingOrNull(result, pointableLeft, pointableRight)) { return; } ``` https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/bde07a7f_1c22c63e?usp=email : PS30, Line 237: warnEvalFailed("unable to derive element type for one or both list arguments"); Why not use `ExceptionUtil.warnFunctionEvalFailed()` and remove `warnEvalFailed`? https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/ad9dbc97_69ee6f32?usp=email : PS30, Line 259: if (leftArgType != rightArgType) { Lots of checks like this should go away if we change the code so that it always converts to double[] File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/vector/VectorListDecoder.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/bedf3bc1_a151f3bc?usp=email : PS30, Line 123: if (typeTag != listAccessor.getItemTypeAt(i) || typeTag != ATypeTag.DOUBLE) { This can be simplified now to just check if the element is NUMERIC and then read the element according to its numeric type and save in the double[]. File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/utils/VectorDistanceArrCalculation.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/2c4144c0_13017649?usp=email : PS30, Line 22: public class VectorDistanceArrCalculation { Rename `VectorDistanceArrCalculation` -> `VectorDistanceCalculation`. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/30af5e8a_3724947b?usp=email : PS30, Line 28: double sum = euclideanSquared(a, b); I believe we already check they are of the same length inside the function. We can remove these checks to make things simpler and make it the responsibility of the caller to check for the same length. -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126?usp=email To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: I45176e7da13ed222f48bb36bd1f0ab4074a0bcb9 Gerrit-Change-Number: 20126 Gerrit-PatchSet: 30 Gerrit-Owner: Calvin Thomas Dani <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Attention: Calvin Thomas Dani <[email protected]> Gerrit-Comment-Date: Wed, 24 Jun 2026 22:33:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No
