You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/03/21 16:55:19 UTC
[GitHub] [pinot] jasperjiaguo opened a new pull request #8378: Scalar function for url encoding and decoding
jasperjiaguo opened a new pull request #8378:
URL: https://github.com/apache/pinot/pull/8378
## Description
Add scalar functions to perform url encoding and decoding in queries.
## Upgrade Notes
Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
No
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang merged pull request #8378: Scalar function for url encoding and decoding
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #8378:
URL: https://github.com/apache/pinot/pull/8378
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] jasperjiaguo commented on a change in pull request #8378: Scalar function for url encoding and decoding
Posted by GitBox <gi...@apache.org>.
jasperjiaguo commented on a change in pull request #8378:
URL: https://github.com/apache/pinot/pull/8378#discussion_r832506905
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java
##########
@@ -443,4 +446,28 @@ public static boolean contains(String input, String substring) {
public static int strcmp(String input1, String input2) {
return input1.compareTo(input2);
}
+
+ /**
+ *
+ * @param input plaintext string
+ * @return url encoded string
+ * @throws UnsupportedEncodingException
+ */
+ @ScalarFunction
+ public static String encodeUrl(String input)
+ throws UnsupportedEncodingException {
+ return URLEncoder.encode(input, StandardCharsets.UTF_8.toString());
Review comment:
The java8 compilation will fail if we don't add `.toString()`
https://github.com/apache/pinot/runs/5632328340?check_suite_focus=true
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #8378: Scalar function for url encoding and decoding
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #8378:
URL: https://github.com/apache/pinot/pull/8378#discussion_r832494999
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java
##########
@@ -443,4 +446,28 @@ public static boolean contains(String input, String substring) {
public static int strcmp(String input1, String input2) {
return input1.compareTo(input2);
}
+
+ /**
+ *
+ * @param input plaintext string
+ * @return url encoded string
+ * @throws UnsupportedEncodingException
+ */
+ @ScalarFunction
+ public static String encodeUrl(String input)
+ throws UnsupportedEncodingException {
+ return URLEncoder.encode(input, StandardCharsets.UTF_8.toString());
Review comment:
```suggestion
return URLEncoder.encode(input, StandardCharsets.UTF_8);
```
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java
##########
@@ -443,4 +446,28 @@ public static boolean contains(String input, String substring) {
public static int strcmp(String input1, String input2) {
return input1.compareTo(input2);
}
+
+ /**
+ *
+ * @param input plaintext string
+ * @return url encoded string
+ * @throws UnsupportedEncodingException
+ */
+ @ScalarFunction
+ public static String encodeUrl(String input)
+ throws UnsupportedEncodingException {
+ return URLEncoder.encode(input, StandardCharsets.UTF_8.toString());
+ }
+
+ /**
+ *
+ * @param input url encoded string
+ * @return plaintext string
+ * @throws UnsupportedEncodingException
+ */
+ @ScalarFunction
+ public static String decodeUrl(String input)
+ throws UnsupportedEncodingException {
+ return URLDecoder.decode(input, StandardCharsets.UTF_8.toString());
Review comment:
```suggestion
return URLDecoder.decode(input, StandardCharsets.UTF_8);
```
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #8378: [WIP] Scalar function for url encoding and decoding
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #8378:
URL: https://github.com/apache/pinot/pull/8378#issuecomment-1074531174
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8378?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8378](https://codecov.io/gh/apache/pinot/pull/8378?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (00c545e) into [master](https://codecov.io/gh/apache/pinot/commit/24d4fd268d28473ffd3ce1ce262322391810f356?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (24d4fd2) will **decrease** coverage by `9.72%`.
> The diff coverage is `69.09%`.
> :exclamation: Current head 00c545e differs from pull request most recent head 6dcee76. Consider uploading reports for the commit 6dcee76 to get more accurate results
```diff
@@ Coverage Diff @@
## master #8378 +/- ##
============================================
- Coverage 70.79% 61.07% -9.73%
+ Complexity 4264 4192 -72
============================================
Files 1640 1640
Lines 85931 86228 +297
Branches 12922 13035 +113
============================================
- Hits 60837 52664 -8173
- Misses 20899 29679 +8780
+ Partials 4195 3885 -310
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `27.26% <24.06%> (-0.27%)` | :arrow_down: |
| unittests1 | `67.03% <62.45%> (+0.08%)` | :arrow_up: |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8378?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...roker/requesthandler/GrpcBrokerRequestHandler.java](https://codecov.io/gh/apache/pinot/pull/8378/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcmVxdWVzdGhhbmRsZXIvR3JwY0Jyb2tlclJlcXVlc3RIYW5kbGVyLmphdmE=) | `75.67% <ø> (ø)` | |
| [...inot/controller/helix/ControllerRequestClient.java](https://codecov.io/gh/apache/pinot/pull/8378/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9Db250cm9sbGVyUmVxdWVzdENsaWVudC5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...manager/realtime/LLRealtimeSegmentDataManager.java](https://codecov.io/gh/apache/pinot/pull/8378/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvTExSZWFsdGltZVNlZ21lbnREYXRhTWFuYWdlci5qYXZh) | `69.70% <ø> (-1.99%)` | :arrow_down: |
| [...ion/tasks/BaseSingleSegmentConversionExecutor.java](https://codecov.io/gh/apache/pinot/pull/8378/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvQmFzZVNpbmdsZVNlZ21lbnRDb252ZXJzaW9uRXhlY3V0b3IuamF2YQ==) | `0.00% <0.00%> (-74.61%)` | :arrow_down: |
| [...ot/plugin/minion/tasks/SegmentConversionUtils.java](https://codecov.io/gh/apache/pinot/pull/8378/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvU2VnbWVudENvbnZlcnNpb25VdGlscy5qYXZh) | `38.33% <ø> (-38.34%)` | :arrow_down: |
| [...he/pinot/segment/local/utils/SegmentPushUtils.java](https://codecov.io/gh/apache/pinot/pull/8378/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9TZWdtZW50UHVzaFV0aWxzLmphdmE=) | `13.48% <0.00%> (ø)` | |
| [...e/pinot/core/query/reduce/RowBasedBlockValSet.java](https://codecov.io/gh/apache/pinot/pull/8378/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9yZWR1Y2UvUm93QmFzZWRCbG9ja1ZhbFNldC5qYXZh) | `16.12% <16.12%> (ø)` | |
| [...pache/pinot/common/utils/request/RequestUtils.java](https://codecov.io/gh/apache/pinot/pull/8378/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvcmVxdWVzdC9SZXF1ZXN0VXRpbHMuamF2YQ==) | `86.39% <33.33%> (-1.11%)` | :arrow_down: |
| [...org/apache/pinot/common/utils/http/HttpClient.java](https://codecov.io/gh/apache/pinot/pull/8378/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvaHR0cC9IdHRwQ2xpZW50LmphdmE=) | `44.65% <44.65%> (ø)` | |
| [...e/pinot/common/utils/FileUploadDownloadClient.java](https://codecov.io/gh/apache/pinot/pull/8378/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvRmlsZVVwbG9hZERvd25sb2FkQ2xpZW50LmphdmE=) | `54.80% <61.42%> (-8.21%)` | :arrow_down: |
| ... and [329 more](https://codecov.io/gh/apache/pinot/pull/8378/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8378?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8378?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [24d4fd2...6dcee76](https://codecov.io/gh/apache/pinot/pull/8378?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #8378: Scalar function for url encoding and decoding
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #8378:
URL: https://github.com/apache/pinot/pull/8378#discussion_r832551684
##########
File path: pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/LiteralOnlyBrokerRequestTest.java
##########
@@ -94,6 +94,15 @@ public void testLiteralOnlyTransformBrokerRequestFromSQL() {
.compileToPinotQuery("SELECT ago('PT1H'), fromDateTime('2020-01-01 UTC', 'yyyy-MM-dd z') FROM myTable")));
Assert.assertFalse(BaseBrokerRequestHandler
.isLiteralOnlyQuery(CalciteSqlParser.compileToPinotQuery("SELECT count(*) from foo where bar > ago('PT1H')")));
+ Assert.assertTrue(BaseBrokerRequestHandler.isLiteralOnlyQuery(CalciteSqlParser
Review comment:
Sorry, I merged the PR too soon. @jasperjiaguo Can you please address Sidd's comments in a separate PR?
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #8378: Scalar function for url encoding and decoding
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #8378:
URL: https://github.com/apache/pinot/pull/8378#discussion_r832550760
##########
File path: pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/LiteralOnlyBrokerRequestTest.java
##########
@@ -94,6 +94,15 @@ public void testLiteralOnlyTransformBrokerRequestFromSQL() {
.compileToPinotQuery("SELECT ago('PT1H'), fromDateTime('2020-01-01 UTC', 'yyyy-MM-dd z') FROM myTable")));
Assert.assertFalse(BaseBrokerRequestHandler
.isLiteralOnlyQuery(CalciteSqlParser.compileToPinotQuery("SELECT count(*) from foo where bar > ago('PT1H')")));
+ Assert.assertTrue(BaseBrokerRequestHandler.isLiteralOnlyQuery(CalciteSqlParser
Review comment:
@jasperjiaguo - please add test cases with real URLs 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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #8378: [WIP] Scalar function for url encoding and decoding
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #8378:
URL: https://github.com/apache/pinot/pull/8378#discussion_r831632421
##########
File path: pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/LiteralOnlyBrokerRequestTest.java
##########
@@ -94,6 +94,15 @@ public void testLiteralOnlyTransformBrokerRequestFromSQL() {
.compileToPinotQuery("SELECT ago('PT1H'), fromDateTime('2020-01-01 UTC', 'yyyy-MM-dd z') FROM myTable")));
Assert.assertFalse(BaseBrokerRequestHandler
.isLiteralOnlyQuery(CalciteSqlParser.compileToPinotQuery("SELECT count(*) from foo where bar > ago('PT1H')")));
+ Assert.assertTrue(BaseBrokerRequestHandler.isLiteralOnlyQuery(CalciteSqlParser
Review comment:
Should we not use full URLs as inputs to encodeUrl / decodeUrl functions in tests ? From user perspective, they will use the full URL right ?
`SELECT encodeUrl(<myURL>) FROM FOO .....`
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] jasperjiaguo commented on a change in pull request #8378: Scalar function for url encoding and decoding
Posted by GitBox <gi...@apache.org>.
jasperjiaguo commented on a change in pull request #8378:
URL: https://github.com/apache/pinot/pull/8378#discussion_r832526715
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java
##########
@@ -443,4 +446,28 @@ public static boolean contains(String input, String substring) {
public static int strcmp(String input1, String input2) {
return input1.compareTo(input2);
}
+
+ /**
+ *
+ * @param input plaintext string
+ * @return url encoded string
+ * @throws UnsupportedEncodingException
+ */
+ @ScalarFunction
+ public static String encodeUrl(String input)
+ throws UnsupportedEncodingException {
+ return URLEncoder.encode(input, StandardCharsets.UTF_8.toString());
+ }
+
+ /**
+ *
+ * @param input url encoded string
+ * @return plaintext string
+ * @throws UnsupportedEncodingException
+ */
+ @ScalarFunction
+ public static String decodeUrl(String input)
+ throws UnsupportedEncodingException {
+ return URLDecoder.decode(input, StandardCharsets.UTF_8.toString());
Review comment:
same reason as above
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org