>From Calvin Thomas Dani <[email protected]>: Attention is currently required from: Ali Alsuliman, Ian Maxon.
Calvin Thomas Dani 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 36: (15 comments) File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/BuiltinFunctions.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/6bc2f767_d9681e25?usp=email : PS30, Line 1200: public static final FunctionIdentifier EUCLIDEAN_DISTANCE_ARRAY = > Change `EUCLIDEAN_DISTANCE_ARRAY` -> `EUCLIDEAN_DISTANCE`. Do the same for > the others. Acknowledged https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/61836dcd_a42ecccc?usp=email : PS30, Line 1202: // public static final FunctionIdentifier L2_DISTANCE_ARRAY = FunctionConstants.newAsterix("l2-distance", 2); > Remove commented out code. Acknowledged https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/143cb91e_6e330763?usp=email : PS30, Line 1206: public static final FunctionIdentifier DOT_DISTANCE_ARRAY = FunctionConstants.newAsterix("dot-distance", 2); > dot isn't really a distance, is it? it's a similarity metric but not a > distance, right? Acknowledged https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/8db34dd7_31ac3fef?usp=email : PS30, Line 1930: addFunction(EUCLIDEAN_DISTANCE_ARRAY, ADoubleTypeComputer.INSTANCE_NULLABLE, true); > You should use `ADoubleTypeComputer.INSTANCE`. Acknowledged 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/cd2e1422_2967db3d?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 se […] Acknowledged https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/b11cab2b_ab67c0d1?usp=email : PS30, Line 237: warnEvalFailed("unable to derive element type for one or both list arguments"); > Why not use `ExceptionUtil. […] Acknowledged https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/a88e9d47_59badf65?usp=email : PS30, Line 254: ExceptionUtil.warnUnsupportedType(ctx, sourceLoc, funcId.getName() + "()", leftArgType); : ExceptionUtil.warnUnsupportedType(ctx, sourceLoc, funcId.getName() + "()", rightArgType); > Acknowledged Acknowledged https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/cf52f7e1_53583455?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[] Acknowledged 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/87835867_81eba9c4?usp=email : PS34, Line 74: if (derivedTypeTag == ATypeTag.ANY) { > You don't have to do this check: `if (derivedTypeTag == ATypeTag.ANY) {`. […] Acknowledged https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/bba31c75_d06ab926?usp=email : PS34, Line 80: throw new TypeMismatchException(sourceLoc, derivedTypeTag.serialize(), ATypeTag.DOUBLE.serialize()); > We shouldn't throw an exception. […] Acknowledged https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/ff9d0842_1539a2e6?usp=email : PS34, Line 106: listAccessor.getOrWriteItem(0, this.tempArrVal, this.storage); > We shouldn't rely on the first element. The rest could be non-numbers. […] Acknowledged https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/da1229c5_956b6a3e?usp=email : PS34, Line 120: public ATypeTag findFirstMismatchedAnyItemType(ListAccessor listAccessor, ATypeTag expectedItemTag) > I don't see `findFirstMismatchedAnyItemType` being used. Remove it. Acknowledged File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/utils/VectorDistanceArrCalculation.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/24016f95_d5aa79e2?usp=email : PS30, Line 28: double sum = euclideanSquared(a, b); > @[email protected] looping you in, […] Acknowledged https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/ee2cd316_20304725?usp=email : PS30, Line 171: return dot / (Math.sqrt(normA) * Math.sqrt(normB)); > Actually with the next patch adds 1 - cosine. I'll move it in here and mark > it as done. Acknowledged File hyracks-fullstack/pom.xml: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20126/comment/82d3328d_6ea4b59f?usp=email : PS34, Line 748: <exclude>**/target/**</exclude> > What's the purpose of this? Remove it Acknowledged -- 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: 36 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: Ian Maxon <[email protected]> Gerrit-Attention: Ali Alsuliman <[email protected]> Gerrit-Comment-Date: Tue, 30 Jun 2026 06:48:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Calvin Thomas Dani <[email protected]> Comment-In-Reply-To: Ian Maxon <[email protected]> Comment-In-Reply-To: Ali Alsuliman <[email protected]>
