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 11:00:21 UTC

[GitHub] [pinot] abhioncbr opened a new pull request, #10790: [multistage]: 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_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] 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] 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