You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "klsince (via GitHub)" <gi...@apache.org> on 2023/06/06 05:24:35 UTC

[GitHub] [pinot] klsince opened a new pull request, #10855: jsonPathString should return null instead of string literal "null"

klsince opened a new pull request, #10855:
URL: https://github.com/apache/pinot/pull/10855

   `bugfix`
   
   jsonPathString() should return null instead of string literal "null" when json has no value for the requested field


-- 
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 #10855: jsonPathString should return null instead of string literal "null"

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10855:
URL: https://github.com/apache/pinot/pull/10855#issuecomment-1577972487

   ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/10855?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   > Merging [#10855](https://app.codecov.io/gh/apache/pinot/pull/10855?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (5aeaa78) into [master](https://app.codecov.io/gh/apache/pinot/commit/5e442959a4bbae87766029529970c6edb469752b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (5e44295) will **decrease** coverage by `20.75%`.
   > The diff coverage is `0.00%`.
   
   ```diff
   @@              Coverage Diff              @@
   ##             master   #10855       +/-   ##
   =============================================
   - Coverage     34.44%   13.69%   -20.75%     
   + Complexity      462      439       -23     
   =============================================
     Files          2174     2120       -54     
     Lines        116833   114341     -2492     
     Branches      17688    17391      -297     
   =============================================
   - Hits          40238    15663    -24575     
   - Misses        73096    97393    +24297     
   + Partials       3499     1285     -2214     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `?` | |
   | integration2 | `?` | |
   | unittests2 | `13.69% <0.00%> (+0.01%)` | :arrow_up: |
   
   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/10855?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [...he/pinot/common/function/scalar/JsonFunctions.java](https://app.codecov.io/gh/apache/pinot/pull/10855?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL0pzb25GdW5jdGlvbnMuamF2YQ==) | `0.00% <0.00%> (-27.28%)` | :arrow_down: |
   
   ... and [774 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/10855/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] klsince commented on pull request #10855: jsonPathString should return null instead of string literal "null"

Posted by "klsince (via GitHub)" <gi...@apache.org>.
klsince commented on PR #10855:
URL: https://github.com/apache/pinot/pull/10855#issuecomment-1577930262

   related to https://github.com/apache/pinot/pull/8382/files#diff-41bc4e8942849a09f684b4ebea4ba5cf8b1c8d87dc20989c95429ac1059a1f96L137


-- 
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] npawar commented on a diff in pull request #10855: jsonPathString should return null instead of string literal "null"

Posted by "npawar (via GitHub)" <gi...@apache.org>.
npawar commented on code in PR #10855:
URL: https://github.com/apache/pinot/pull/10855#discussion_r1219984101


##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java:
##########
@@ -135,7 +135,7 @@ public static String jsonPathString(Object object, String jsonPath)
     if (jsonValue instanceof String) {
       return (String) jsonValue;
     }
-    return JsonUtils.objectToString(jsonValue);

Review Comment:
   during the null work, we introduced the convention to annotate these functions Nullable, as well as the params (check out examples in ObjectFunctions). Perhaps good to add here too



-- 
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 #10855: jsonPathString should return null instead of string literal "null"

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #10855:
URL: https://github.com/apache/pinot/pull/10855


-- 
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