Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23729 )
Change subject: IMPALA-14566: Add euclidean_distance and cosine_similarity functions for ARRAY<FLOAT> ...................................................................... Patch Set 3: (31 comments) gerrit-auto-critic failed. You can reproduce it locally using command: python3 bin/jenkins/critique-gerrit-review.py --dryrun To run it, you might need a virtual env with Python3's venv installed. http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc File be/src/exprs/vector-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@17 PS3, Line 17: // Each tuple has a null indicator 1 byte which are Followed by the float value which has 4 bytes line too long (97 > 90) http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@21 PS3, Line 21: constexpr int SLOT_OFFSET = NULL_INDICATOR_SIZE; // Float will come after the null indicator. line too long (94 > 90) http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@22 PS3, Line 22: constexpr int NULL_INDICATOR_OFFSET = 0; // Null indicator is always present at the start of tuple line too long (99 > 90) http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@27 PS3, Line 27: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@30 PS3, Line 30: return 0.0f; // Return 0.0 float value for NULL elements line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@32 PS3, Line 32: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@35 PS3, Line 35: // The below logic should work as for ex if we want to extract the float value at index 1 line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@37 PS3, Line 37: // Index* tuple size will be 1 * 5 (tuple has null and float bytes) . Tuple ptr will be start array ptr + index*tuple size. line too long (125 > 90) http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@38 PS3, Line 38: // Float will be after the above tuple ptr address where float value is present. To read float of 4 bytes , we will use memcpy function line too long (137 > 90) http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@58 PS3, Line 58: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@64 PS3, Line 64: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@70 PS3, Line 70: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@74 PS3, Line 74: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@78 PS3, Line 78: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@84 PS3, Line 84: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@95 PS3, Line 95: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@101 PS3, Line 101: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@107 PS3, Line 107: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@109 PS3, Line 109: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@114 PS3, Line 114: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@118 PS3, Line 118: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@123 PS3, Line 123: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@127 PS3, Line 127: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@133 PS3, Line 133: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@136 PS3, Line 136: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions-ir.cc@139 PS3, Line 139: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions.h File be/src/exprs/vector-functions.h: http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions.h@14 PS3, Line 14: /// The Return Type for the below distance functions DOUBLE and I did not use FLOAT because of better precision. line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions.h@14 PS3, Line 14: /// The Return Type for the below distance functions DOUBLE and I did not use FLOAT because of better precision. line too long (115 > 90) http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions.h@15 PS3, Line 15: // Distance calculations usually involve square roots which will benefit from 15-17 digit precision in Double vs 7 digits in FLOAT. line too long (133 > 90) http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions.h@16 PS3, Line 16: // Value returned from this Euclidean distance function is either a DOUBLE, or NULL if inputs are invalid line too long (107 > 90) http://gerrit.cloudera.org:8080/#/c/23729/3/be/src/exprs/vector-functions.h@35 PS3, Line 35: /// Declaring the Helper functions under private to get a float value from an array element. line too long (94 > 90) -- To view, visit http://gerrit.cloudera.org:8080/23729 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic1f17fa85819994703510b425c50db46af573836 Gerrit-Change-Number: 23729 Gerrit-PatchSet: 3 Gerrit-Owner: Raghav Jindal <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Tue, 02 Dec 2025 02:07:44 +0000 Gerrit-HasComments: Yes
