You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "abhioncbr (via GitHub)" <gi...@apache.org> on 2023/05/21 03:09:15 UTC
[GitHub] [pinot] abhioncbr opened a new pull request, #10790: 10767: Changes for supporting Json scalar functions.
abhioncbr opened a new pull request, #10790:
URL: https://github.com/apache/pinot/pull/10790
- Added support for `json` scalar function(`JSONEXTRACTKEY` , `jsonextractscalar`)
- Changes handle the String `split` function as well.
**Solution approach**
- [toSql](https://github.com/apache/pinot/blob/master/pinot-query-planner/src/main/java/org/apache/calcite/prepare/PinotCalciteCatalogReader.java#L420) function in class `PinotCalciteCatalogReader` sets columns data type as `SqlTypeName.ANY` if any scalar function return Object class.
- [convertToColumnDataType](https://github.com/apache/pinot/blob/master/pinot-query-planner/src/main/java/org/apache/pinot/query/planner/logical/RelToPlanNodeConverter.java#L299) function in class `RelToPlanNodeConverter` sets the ColumnDataType as `DataSchema.ColumnDataType.BYTES` because we aren't handling BYTES there.
- Eventually, all such queries based on`DataSchema.ColumnDataType.BYTES` fails with a ClassCastException During the serialization step. However, we get the correct Data Type for the column from SelectionResultsBlock. We can use this right type to propagate the result. Added `handleBytesDataType` function tries to do the same.
cc: @walterddr
--
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] abhioncbr commented on a diff in pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on code in PR #10790:
URL: https://github.com/apache/pinot/pull/10790#discussion_r1200711702
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -205,5 +205,15 @@ public static boolean textMatch(String text, String pattern) {
public static boolean jsonMatch(String text, String pattern) {
throw new UnsupportedOperationException("Placeholder scalar function, should not reach here");
}
+
+ @ScalarFunction(names = {"split", "split"}, isPlaceholder = true)
Review Comment:
Gotcha, let me revert the `split` changes.
--
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] abhioncbr commented on pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on PR #10790:
URL: https://github.com/apache/pinot/pull/10790#issuecomment-1625536220
> Another try here: #11048
Thanks for mentioning it, I think I can close this PR. Thanks
--
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] walterddr commented on a diff in pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10790:
URL: https://github.com/apache/pinot/pull/10790#discussion_r1207272732
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/LeafStageTransferableBlockOperator.java:
##########
@@ -308,13 +309,16 @@ private static boolean inOrder(int[] columnIndices) {
* @param dataSchema data schema desired for the row.
* @return canonicalize row.
*/
- private static Object[] canonicalizeRow(Object[] row, DataSchema dataSchema) {
+ private static Object[] canonicalizeRow(Object[] row, DataSchema dataSchema, DataSchema resultSchema) {
Object[] resultRow = new Object[row.length];
for (int colId = 0; colId < row.length; colId++) {
Object value = row[colId];
if (value != null) {
if (dataSchema.getColumnDataType(colId) == DataSchema.ColumnDataType.OBJECT) {
resultRow[colId] = value;
+ } else if (dataSchema.getColumnDataType(colId) == DataSchema.ColumnDataType.BYTES) {
+ dataSchema.getColumnDataTypes()[colId] = resultSchema.getColumnDataType(colId);
Review Comment:
this seems wrong. we can't simply modify the "desiredDataSchema" which is given during logical planning. modifying it causes downstream issues which might've been hidden due to the auto-casting nature of v2 operator.
--
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] abhioncbr commented on pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on PR #10790:
URL: https://github.com/apache/pinot/pull/10790#issuecomment-1627551548
Closing as per https://github.com/apache/pinot/pull/11048
--
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] walterddr commented on a diff in pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10790:
URL: https://github.com/apache/pinot/pull/10790#discussion_r1201461100
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/LeafStageTransferableBlockOperator.java:
##########
@@ -239,6 +239,7 @@ private static TransferableBlock composeSelectTransferableBlock(InstanceResponse
+ " Column Ordering: " + Arrays.toString(columnIndices));
return composeColumnIndexedTransferableBlock(responseBlock, adjustedResultSchema, columnIndices);
} else {
+ handleBytesDataType(desiredDataSchema.getColumnDataTypes(), resultSchema.getColumnDataTypes());
Review Comment:
handling this here is wrong IMO. b/c both column index reordering and direct transferrable had to handle bytes type. you can add the logic in `canonicalizeRow` method IMO
--
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 #10790: 10767: Changes for supporting Json scalar functions.
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10790:
URL: https://github.com/apache/pinot/pull/10790#issuecomment-1556074903
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/10790?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#10790](https://app.codecov.io/gh/apache/pinot/pull/10790?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (3235d92) into [master](https://app.codecov.io/gh/apache/pinot/commit/e302318d33d41facdaff0c03cfbc96e423f41d9d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (e302318) will **decrease** coverage by `20.76%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #10790 +/- ##
=============================================
- Coverage 34.43% 13.68% -20.76%
+ Complexity 462 439 -23
=============================================
Files 2159 2105 -54
Lines 116044 113559 -2485
Branches 17569 17272 -297
=============================================
- Hits 39961 15535 -24426
- Misses 72604 96756 +24152
+ Partials 3479 1268 -2211
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests2 | `13.68% <0.00%> (-0.01%)` | :arrow_down: |
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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://app.codecov.io/gh/apache/pinot/pull/10790?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...apache/pinot/common/function/FunctionRegistry.java](https://app.codecov.io/gh/apache/pinot/pull/10790?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRnVuY3Rpb25SZWdpc3RyeS5qYXZh) | `0.00% <0.00%> (-76.48%)` | :arrow_down: |
| [...e/operator/LeafStageTransferableBlockOperator.java](https://app.codecov.io/gh/apache/pinot/pull/10790?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9MZWFmU3RhZ2VUcmFuc2ZlcmFibGVCbG9ja09wZXJhdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | |
... and [768 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/10790/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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] abhioncbr commented on a diff in pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on code in PR #10790:
URL: https://github.com/apache/pinot/pull/10790#discussion_r1207383670
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/LeafStageTransferableBlockOperator.java:
##########
@@ -308,13 +309,16 @@ private static boolean inOrder(int[] columnIndices) {
* @param dataSchema data schema desired for the row.
* @return canonicalize row.
*/
- private static Object[] canonicalizeRow(Object[] row, DataSchema dataSchema) {
+ private static Object[] canonicalizeRow(Object[] row, DataSchema dataSchema, DataSchema resultSchema) {
Object[] resultRow = new Object[row.length];
for (int colId = 0; colId < row.length; colId++) {
Object value = row[colId];
if (value != null) {
if (dataSchema.getColumnDataType(colId) == DataSchema.ColumnDataType.OBJECT) {
resultRow[colId] = value;
+ } else if (dataSchema.getColumnDataType(colId) == DataSchema.ColumnDataType.BYTES) {
+ dataSchema.getColumnDataTypes()[colId] = resultSchema.getColumnDataType(colId);
Review Comment:
What should be the approach to handle such cases, where the scalar function return type is `Object` and getting translated to `DataSchema.ColumnDataType.BYTES`?
--
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] walterddr commented on a diff in pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10790:
URL: https://github.com/apache/pinot/pull/10790#discussion_r1213791510
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/LeafStageTransferableBlockOperator.java:
##########
@@ -308,13 +309,16 @@ private static boolean inOrder(int[] columnIndices) {
* @param dataSchema data schema desired for the row.
* @return canonicalize row.
*/
- private static Object[] canonicalizeRow(Object[] row, DataSchema dataSchema) {
+ private static Object[] canonicalizeRow(Object[] row, DataSchema dataSchema, DataSchema resultSchema) {
Object[] resultRow = new Object[row.length];
for (int colId = 0; colId < row.length; colId++) {
Object value = row[colId];
if (value != null) {
if (dataSchema.getColumnDataType(colId) == DataSchema.ColumnDataType.OBJECT) {
resultRow[colId] = value;
+ } else if (dataSchema.getColumnDataType(colId) == DataSchema.ColumnDataType.BYTES) {
+ dataSchema.getColumnDataTypes()[colId] = resultSchema.getColumnDataType(colId);
Review Comment:
i don't rightfully know. we should add tests against Postgres behavior and see what's expected and modify our behavior accordingly
--
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] abhioncbr closed pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr closed pull request #10790: [multistage]: Changes for supporting Json scalar functions.
URL: https://github.com/apache/pinot/pull/10790
--
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] abhioncbr commented on a diff in pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on code in PR #10790:
URL: https://github.com/apache/pinot/pull/10790#discussion_r1204907532
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/LeafStageTransferableBlockOperator.java:
##########
@@ -239,6 +239,7 @@ private static TransferableBlock composeSelectTransferableBlock(InstanceResponse
+ " Column Ordering: " + Arrays.toString(columnIndices));
return composeColumnIndexedTransferableBlock(responseBlock, adjustedResultSchema, columnIndices);
} else {
+ handleBytesDataType(desiredDataSchema.getColumnDataTypes(), resultSchema.getColumnDataTypes());
Review Comment:
It is updated as per the comments. Please have another look. Thanks
--
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] abhioncbr closed pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr closed pull request #10790: [multistage]: Changes for supporting Json scalar functions.
URL: https://github.com/apache/pinot/pull/10790
--
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] abhioncbr closed pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr closed pull request #10790: [multistage]: Changes for supporting Json scalar functions.
URL: https://github.com/apache/pinot/pull/10790
--
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] walterddr commented on a diff in pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10790:
URL: https://github.com/apache/pinot/pull/10790#discussion_r1201460145
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -187,7 +187,7 @@ public static Object jsonExtractScalar(String jsonFieldName, String jsonPath, St
}
@ScalarFunction(names = {"jsonExtractKey", "json_extract_key"}, isPlaceholder = true)
- public static String jsonExtractKey(String jsonFieldName, String jsonPath) {
Review Comment:
good catch!
--
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] abhioncbr commented on pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on PR #10790:
URL: https://github.com/apache/pinot/pull/10790#issuecomment-1622847277
> Let me take a look at this one @abhioncbr
>
> It is not as straightforward as agg function and some of the tricks doesn't apply
No need to review rn; I haven't made changes. Was testing after merging the master branch changes. I'll ask you for the review.
--
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] xiangfu0 commented on pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on PR #10790:
URL: https://github.com/apache/pinot/pull/10790#issuecomment-1624759393
Another try here:
https://github.com/apache/pinot/pull/11048
--
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] walterddr commented on a diff in pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10790:
URL: https://github.com/apache/pinot/pull/10790#discussion_r1200643418
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -205,5 +205,15 @@ public static boolean textMatch(String text, String pattern) {
public static boolean jsonMatch(String text, String pattern) {
throw new UnsupportedOperationException("Placeholder scalar function, should not reach here");
}
+
+ @ScalarFunction(names = {"split", "split"}, isPlaceholder = true)
Review Comment:
this probably can probably be address separately. essentially we need to support ARRAY and V1 currently support MV column, see https://github.com/apache/pinot/issues/10658
--
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] abhioncbr commented on a diff in pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on code in PR #10790:
URL: https://github.com/apache/pinot/pull/10790#discussion_r1199774793
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -205,5 +205,15 @@ public static boolean textMatch(String text, String pattern) {
public static boolean jsonMatch(String text, String pattern) {
throw new UnsupportedOperationException("Placeholder scalar function, should not reach here");
}
+
+ @ScalarFunction(names = {"split", "split"}, isPlaceholder = true)
Review Comment:
With this change, we can handle the `split` function in multistage.
The other option available is to leave it as it is i.e. returning `string[]`. In that case, it will translate to `SqlTypeName.OTHER` and eventually become `DataSchema.ColumnDataType.OBJECT`. To get it working we need to add support in `ObjectSerDeUtils` for `string[]`.
Currently, it's following the above-mentioned option and failing with an exception getting generated from `ObjectSerDeUtils`
```
throw new IllegalArgumentException("Unsupported type of value: " + value.getClass().getSimpleName());
```
--
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] abhioncbr commented on a diff in pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on code in PR #10790:
URL: https://github.com/apache/pinot/pull/10790#discussion_r1199773576
##########
pinot-query-runtime/src/test/resources/queries/StringFunctions.json:
##########
@@ -62,6 +57,12 @@
}
},
"queries": [
+ {
+ "sql": "SELECT split(strCol, ' ') FROM {stringTbl} where strCol like '%wiTH%'",
+ "outputs": [
Review Comment:
H2 doesn't support `string split` [function](http://www.h2database.com/html/functions.html).
--
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] abhioncbr closed pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr closed pull request #10790: [multistage]: Changes for supporting Json scalar functions.
URL: https://github.com/apache/pinot/pull/10790
--
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] abhioncbr closed pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr closed pull request #10790: [multistage]: Changes for supporting Json scalar functions.
URL: https://github.com/apache/pinot/pull/10790
--
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] walterddr commented on pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on PR #10790:
URL: https://github.com/apache/pinot/pull/10790#issuecomment-1622738872
Let me take a look at this one @abhioncbr
It is not as straightforward as agg function and some of the tricks doesn't apply
--
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] abhioncbr commented on pull request #10790: [multistage]: Changes for supporting Json scalar functions.
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on PR #10790:
URL: https://github.com/apache/pinot/pull/10790#issuecomment-1615311406
I'll continue on this once these two PR's(https://github.com/apache/pinot/pull/10845, https://github.com/apache/pinot/pull/10846) get merged. Following the same approach will be helpful here.
--
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