Re: [PR] [Scalar Function][Array] Add ArrayJoinString Scalar function [pinot]
xiangfu0 commented on PR #14537: URL: https://github.com/apache/pinot/pull/14537#issuecomment-2501975636 Thanks for supporting this ! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
Re: [PR] [Scalar Function][Array] Add ArrayJoinString Scalar function [pinot]
xiangfu0 merged PR #14537: URL: https://github.com/apache/pinot/pull/14537 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
Re: [PR] [Scalar Function][Array] Add ArrayJoinString Scalar function [pinot]
nikhilsu commented on code in PR #14537:
URL: https://github.com/apache/pinot/pull/14537#discussion_r1857491579
##
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArrayFunctions.java:
##
@@ -347,4 +347,9 @@ public static double[] generateDoubleArray(double start,
double end, double inc)
}
return arr;
}
+
+ @ScalarFunction
+ public static String arrayJoinString(String[] values, String delimiter) {
Review Comment:
@xiangfu0 done. PTAL - need your eyes on the null checker.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
Re: [PR] [Scalar Function][Array] Add ArrayJoinString Scalar function [pinot]
xiangfu0 commented on code in PR #14537:
URL: https://github.com/apache/pinot/pull/14537#discussion_r1857491186
##
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArrayFunctions.java:
##
@@ -347,4 +347,9 @@ public static double[] generateDoubleArray(double start,
double end, double inc)
}
return arr;
}
+
+ @ScalarFunction
+ public static String arrayJoinString(String[] values, String delimiter) {
Review Comment:
Yeah, let's call it `arrayToString`. Pinot internally handles naming
transfer for `array_to_string` to `arrayToString`.
You could also verify this in the test as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
Re: [PR] [Scalar Function][Array] Add ArrayJoinString Scalar function [pinot]
nikhilsu commented on code in PR #14537:
URL: https://github.com/apache/pinot/pull/14537#discussion_r1857470658
##
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArrayFunctions.java:
##
@@ -347,4 +347,9 @@ public static double[] generateDoubleArray(double start,
double end, double inc)
}
return arr;
}
+
+ @ScalarFunction
+ public static String arrayJoinString(String[] values, String delimiter) {
Review Comment:
Postgres calls it
[array_to_string](https://www.postgresql.org/docs/15/functions-array.html#FUNCTION-ARRAY-TO-STRING).
It also offers NULL replacements.
I named it `arrayJoinString` to be consistent with the other array functions
above. I can call it `array_to_string`, if you'd like.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
Re: [PR] [Scalar Function][Array] Add ArrayJoinString Scalar function [pinot]
xiangfu0 commented on code in PR #14537:
URL: https://github.com/apache/pinot/pull/14537#discussion_r1857469558
##
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArrayFunctions.java:
##
@@ -347,4 +347,9 @@ public static double[] generateDoubleArray(double start,
double end, double inc)
}
return arr;
}
+
+ @ScalarFunction
+ public static String arrayJoinString(String[] values, String delimiter) {
Review Comment:
For null handling you need to create a new function:
```
public static String arrayToString(String[] values, String delimiter, String
nullString) {
...
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
Re: [PR] [Scalar Function][Array] Add ArrayJoinString Scalar function [pinot]
xiangfu0 commented on code in PR #14537:
URL: https://github.com/apache/pinot/pull/14537#discussion_r1857468862
##
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArrayFunctions.java:
##
@@ -347,4 +347,9 @@ public static double[] generateDoubleArray(double start,
double end, double inc)
}
return arr;
}
+
+ @ScalarFunction
+ public static String arrayJoinString(String[] values, String delimiter) {
Review Comment:
Can you change the function name to `arrayToString`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
Re: [PR] [Scalar Function][Array] Add ArrayJoinString Scalar function [pinot]
xiangfu0 commented on code in PR #14537:
URL: https://github.com/apache/pinot/pull/14537#discussion_r1857468556
##
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArrayFunctions.java:
##
@@ -347,4 +347,9 @@ public static double[] generateDoubleArray(double start,
double end, double inc)
}
return arr;
}
+
+ @ScalarFunction
+ public static String arrayJoinString(String[] values, String delimiter) {
Review Comment:
Postgres has a function called: `ARRAY_TO_STRING` :
https://www.postgresql.org/docs/9.1/functions-array.html
I think we can implement this signature.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
Re: [PR] [Scalar Function][Array] Add ArrayJoinString Scalar function [pinot]
codecov-commenter commented on PR #14537: URL: https://github.com/apache/pinot/pull/14537#issuecomment-2499233286 ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/14537?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report All modified and coverable lines are covered by tests :white_check_mark: > Project coverage is 0.00%. Comparing base [(`59551e4`)](https://app.codecov.io/gh/apache/pinot/commit/59551e45224f1535c4863fd577622b37366ccc97?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`f369c02`)](https://app.codecov.io/gh/apache/pinot/commit/f369c0287ca13ed055db9acdd230c9f02a2c7872?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). > Report is 1390 commits behind head on master. > :exclamation: There is a different number of reports uploaded between BASE (59551e4) and HEAD (f369c02). Click for more details. > > HEAD has 53 uploads less than BASE > >| Flag | BASE (59551e4) | HEAD (f369c02) | >|--|--|--| >|integration|7|1| >|integration2|3|1| >|temurin|12|1| >|java-21|7|1| >|skip-bytebuffers-true|3|1| >|skip-bytebuffers-false|7|0| >|unittests|5|0| >|unittests1|2|0| >|java-11|5|0| >|unittests2|3|0| >|integration1|2|0| >|custom-integration1|2|0| > Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #14537 +/- ## = - Coverage 61.75%0.00% -61.76% = Files 24363 -2433 Lines1332336 -133227 Branches 206360-20636 = - Hits 822740-82274 + Misses449116-44905 + Partials 60480 -6048 ``` | [Flag](https://app.codecov.io/gh/apache/pinot/pull/14537/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | |---|---|---| | [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/14537/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | | [integration](https://app.codecov.io/gh/apache/pinot/pull/14537/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-0.01%)` | :arrow_down: | | [integration1](https://app.codecov.io/gh/apache/pinot/pull/14537/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | | [integration2](https://app.codecov.io/gh/apache/pinot/pull/14537/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (ø)` | | | [java-11](https://app.codecov.io/gh/apache/pinot/pull/14537/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | | [java-21](https://app.codecov.io/gh/apache/pinot/pull/14537/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-61.63%)` | :arrow_down: | | [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/14537/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | | [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/14537/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-27.73%)` | :arrow_down: | | [temurin](https://app.codecov.io/gh/apache/pinot/pull/14537/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-61.76%)` | :arrow_down: | | [unittests](https://app.codecov.io/gh/apache/pinot/pull/14537/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | | [unittests1](https://app.codecov.io/gh/apache/pinot/pull/14537/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | | [unittests2](https://app.codecov.io/gh/apache/pinot/pull/14537/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | Flags with carried for
Re: [PR] [Scalar Function][Array] Add ArrayJoinString Scalar function [pinot]
mayankshriv commented on code in PR #14537:
URL: https://github.com/apache/pinot/pull/14537#discussion_r1857436539
##
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArrayFunctions.java:
##
@@ -347,4 +347,9 @@ public static double[] generateDoubleArray(double start,
double end, double inc)
}
return arr;
}
+
+ @ScalarFunction
+ public static String arrayJoinString(String[] values, String delimiter) {
Review Comment:
Is there a Postgres equivalent? Does it make sense to use the same/similar
name?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
