You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "tibrewalpratik17 (via GitHub)" <gi...@apache.org> on 2024/03/08 09:01:33 UTC

[PR] Add isJson UDF [pinot]

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

   label:
   `feature`
   
   WHY ARE WE ADDING THIS? 
   
   - This will be helpful in validating String-columns with json-indexes during ingestion time. We can use this UDF as a filter-transform config.
   - If we do not use `skipInvalidJson` in json-index config introduced in #12238 then the ingestion stops in case of malformed / truncated json.
   - If we use `skipInvalidJson` in json-index, then ingestion in not stopped but the record becomes un-queryable with json_match queries anyways. We also end up persisting malformed data in the table which can potentially cause that particular primary-key to not come in query response at all (if using upsert). This is again an issue. 
   
   Adding a json-validator as a UDF so that we can use it in filterTransform config, add alerts on `NUMBER_ROWS_FILTERED` metric and using this #12602 immediately track the events which are malformed.
   
   UDFS IN OTHER DBS
   
   Saw few DBs have similar relatable functionalities too.
   - Amazon Redshift ([IS_VALID_JSON](https://docs.aws.amazon.com/redshift/latest/dg/IS_VALID_JSON.html))
   - Snowflake ([CHECK_JSON](https://docs.snowflake.com/en/sql-reference/functions/check_json))
   
   Did not find anything similar in Postgresql though.


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


Re: [PR] Add isJson UDF [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #12603:
URL: https://github.com/apache/pinot/pull/12603#discussion_r1520467627


##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java:
##########
@@ -831,4 +832,24 @@ public static boolean like(String inputStr, String likePatternStr) {
     String regexPatternStr = RegexpPatternConverterUtils.likeToRegexpLike(likePatternStr);
     return regexpLike(inputStr, regexPatternStr);
   }
+
+  /**
+   * Checks whether the input string can be parsed into a json node or not. Useful for scenarios where we want
+   * to filter out malformed json. In case of nulls, it is treated as valid json as in partial-upsert we might
+   * want to treat that column value as valid.
+   *
+   * @param inputStr Input string to test for valid json
+   */
+  @ScalarFunction
+  public static boolean isJson(String inputStr) {
+    try {
+      if (inputStr == null) {

Review Comment:
   If there is no `nullableParameters = true` in the annotation, it will return `null` whenever the argument is `null`. No need to explicitly return `null`



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


Re: [PR] Add isJson UDF [pinot]

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


##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java:
##########
@@ -831,4 +832,24 @@ public static boolean like(String inputStr, String likePatternStr) {
     String regexPatternStr = RegexpPatternConverterUtils.likeToRegexpLike(likePatternStr);
     return regexpLike(inputStr, regexPatternStr);
   }
+
+  /**
+   * Checks whether the input string can be parsed into a json node or not. Useful for scenarios where we want
+   * to filter out malformed json. In case of nulls we return null itself, as null can be treated as valid json

Review Comment:
   Ack.



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


Re: [PR] Add isJson UDF [pinot]

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


##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java:
##########
@@ -831,4 +832,24 @@ public static boolean like(String inputStr, String likePatternStr) {
     String regexPatternStr = RegexpPatternConverterUtils.likeToRegexpLike(likePatternStr);
     return regexpLike(inputStr, regexPatternStr);
   }
+
+  /**
+   * Checks whether the input string can be parsed into a json node or not. Useful for scenarios where we want
+   * to filter out malformed json. In case of nulls, it is treated as valid json as in partial-upsert we might
+   * want to treat that column value as valid.
+   *
+   * @param inputStr Input string to test for valid json
+   */
+  @ScalarFunction
+  public static boolean isJson(String inputStr) {
+    try {
+      if (inputStr == null) {

Review Comment:
   Something like this?
   ```
   @ScalarFunction(nullableParameters = true, names = {"isJson", "is_json"})
     public static Object isJson(@Nullable String inputStr) {
       try {
         if (inputStr == null) {
           return null;
         }
         JsonUtils.stringToJsonNode(inputStr);
         return true;
       } catch (Exception e) {
         return false;
       }
     }
   ```



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


Re: [PR] Add isJson UDF [pinot]

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


##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java:
##########
@@ -831,4 +832,24 @@ public static boolean like(String inputStr, String likePatternStr) {
     String regexPatternStr = RegexpPatternConverterUtils.likeToRegexpLike(likePatternStr);
     return regexpLike(inputStr, regexPatternStr);
   }
+
+  /**
+   * Checks whether the input string can be parsed into a json node or not. Useful for scenarios where we want
+   * to filter out malformed json. In case of nulls, it is treated as valid json as in partial-upsert we might
+   * want to treat that column value as valid.
+   *
+   * @param inputStr Input string to test for valid json
+   */
+  @ScalarFunction
+  public static boolean isJson(String inputStr) {
+    try {
+      if (inputStr == null) {

Review Comment:
   updated! 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


Re: [PR] Add isJson UDF [pinot]

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


##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java:
##########
@@ -831,4 +832,24 @@ public static boolean like(String inputStr, String likePatternStr) {
     String regexPatternStr = RegexpPatternConverterUtils.likeToRegexpLike(likePatternStr);
     return regexpLike(inputStr, regexPatternStr);
   }
+
+  /**
+   * Checks whether the input string can be parsed into a json node or not. Useful for scenarios where we want
+   * to filter out malformed json. In case of nulls, it is treated as valid json as in partial-upsert we might
+   * want to treat that column value as valid.
+   *
+   * @param inputStr Input string to test for valid json
+   */
+  @ScalarFunction
+  public static boolean isJson(String inputStr) {
+    try {
+      if (inputStr == null) {

Review Comment:
   nit: Semantically speaking, `null` in databases means "unknown" or "missing", and hence one might expect `isJson` to return false for null.
   
   To address this, we could consider adding a two argument variant of this
   
   ```
   public static boolean isJson(String inputStr, boolean acceptNull) {
     try {
       if (inputStr == null) {
         return acceptNull;
       }
      ...
     }
   }
   ```



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


Re: [PR] Add isJson UDF [pinot]

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

   ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/12603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   Attention: Patch coverage is `0%` with `6 lines` in your changes are missing coverage. Please review.
   > Project coverage is 0.00%. Comparing base [(`59551e4`)](https://app.codecov.io/gh/apache/pinot/commit/59551e45224f1535c4863fd577622b37366ccc97?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`32d2865`)](https://app.codecov.io/gh/apache/pinot/pull/12603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   > Report is 92 commits behind head on master.
   
   | [Files](https://app.codecov.io/gh/apache/pinot/pull/12603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [.../pinot/common/function/scalar/StringFunctions.java](https://app.codecov.io/gh/apache/pinot/pull/12603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL1N0cmluZ0Z1bmN0aW9ucy5qYXZh) | 0.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   ```diff
   @@              Coverage Diff              @@
   ##             master   #12603       +/-   ##
   =============================================
   - Coverage     61.75%    0.00%   -61.76%     
   =============================================
     Files          2436     2375       -61     
     Lines        133233   130107     -3126     
     Branches      20636    20194      -442     
   =============================================
   - Hits          82274        0    -82274     
   - Misses        44911   130107    +85196     
   + Partials       6048        0     -6048     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/pinot/pull/12603/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/12603/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/12603/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-0.01%)` | :arrow_down: |
   | [integration1](https://app.codecov.io/gh/apache/pinot/pull/12603/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/12603/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (ø)` | |
   | [java-11](https://app.codecov.io/gh/apache/pinot/pull/12603/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/12603/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-61.63%)` | :arrow_down: |
   | [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/12603/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/12603/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-27.73%)` | :arrow_down: |
   | [temurin](https://app.codecov.io/gh/apache/pinot/pull/12603/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-61.76%)` | :arrow_down: |
   | [unittests](https://app.codecov.io/gh/apache/pinot/pull/12603/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/12603/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/12603/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   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.
   
   
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/12603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?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


Re: [PR] Add isJson UDF [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #12603:
URL: https://github.com/apache/pinot/pull/12603#discussion_r1520391643


##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java:
##########
@@ -831,4 +832,24 @@ public static boolean like(String inputStr, String likePatternStr) {
     String regexPatternStr = RegexpPatternConverterUtils.likeToRegexpLike(likePatternStr);
     return regexpLike(inputStr, regexPatternStr);
   }
+
+  /**
+   * Checks whether the input string can be parsed into a json node or not. Useful for scenarios where we want
+   * to filter out malformed json. In case of nulls, it is treated as valid json as in partial-upsert we might
+   * want to treat that column value as valid.
+   *
+   * @param inputStr Input string to test for valid json
+   */
+  @ScalarFunction
+  public static boolean isJson(String inputStr) {
+    try {
+      if (inputStr == null) {

Review Comment:
   The current semantic is quite confusing. I feel we should just not take `null` and the framework will simply return `null` when the input is `null`
   ```suggestion
     @ScalarFunction(names = {"isJson", "is_json"})
     public static boolean isJson(String inputStr) {
       try {
         JsonUtils.stringToJsonNode(inputStr);
         return true;
       } catch (Exception e) {
         return false;
       }
     }
   ```
   
   cc @ankitsultana 



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


Re: [PR] Add isJson UDF [pinot]

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


##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java:
##########
@@ -831,4 +832,24 @@ public static boolean like(String inputStr, String likePatternStr) {
     String regexPatternStr = RegexpPatternConverterUtils.likeToRegexpLike(likePatternStr);
     return regexpLike(inputStr, regexPatternStr);
   }
+
+  /**
+   * Checks whether the input string can be parsed into a json node or not. Useful for scenarios where we want
+   * to filter out malformed json. In case of nulls, it is treated as valid json as in partial-upsert we might
+   * want to treat that column value as valid.
+   *
+   * @param inputStr Input string to test for valid json
+   */
+  @ScalarFunction
+  public static boolean isJson(String inputStr) {
+    try {
+      if (inputStr == null) {

Review Comment:
   Ah that makes it much simpler then! We can just use `isJson(str) == false` to find invalid jsons. Let me update 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


Re: [PR] Add isJson UDF [pinot]

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


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


Re: [PR] Add isJson UDF [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #12603:
URL: https://github.com/apache/pinot/pull/12603#discussion_r1518149953


##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java:
##########
@@ -831,4 +832,24 @@ public static boolean like(String inputStr, String likePatternStr) {
     String regexPatternStr = RegexpPatternConverterUtils.likeToRegexpLike(likePatternStr);
     return regexpLike(inputStr, regexPatternStr);
   }
+
+  /**
+   * Checks whether the input string can be parsed into a json node or not. Useful for scenarios where we want
+   * to filter out malformed json. In case of nulls, it is treated as valid json as in partial-upsert we might
+   * want to treat that column value as valid.
+   *
+   * @param inputStr Input string to test for valid json
+   */
+  @ScalarFunction
+  public static boolean isJson(String inputStr) {
+    try {
+      if (inputStr == null) {

Review Comment:
   In order to take `null`, you need to annotate the function with `nullableParameters`. Please also annotate the parameter as `@Nullable`. You might also want to add alias `is_json`



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


Re: [PR] Add isJson UDF [pinot]

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


##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java:
##########
@@ -831,4 +832,24 @@ public static boolean like(String inputStr, String likePatternStr) {
     String regexPatternStr = RegexpPatternConverterUtils.likeToRegexpLike(likePatternStr);
     return regexpLike(inputStr, regexPatternStr);
   }
+
+  /**
+   * Checks whether the input string can be parsed into a json node or not. Useful for scenarios where we want
+   * to filter out malformed json. In case of nulls we return null itself, as null can be treated as valid json

Review Comment:
   This statement does not seem to be aligned with what is implemented and merged here. If the argument is null, it returns false as `JsonUtils.stringToJsonNode(inputStr);` throws an Exception.
   
   The discussion in the comment [thread](https://github.com/apache/pinot/pull/12603#discussion_r1518149953) is also confusing to what is finally added here. It is good to add the conclusion in that thread for readers if it is decided based on offline discussion. 
    



##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java:
##########
@@ -831,4 +832,24 @@ public static boolean like(String inputStr, String likePatternStr) {
     String regexPatternStr = RegexpPatternConverterUtils.likeToRegexpLike(likePatternStr);
     return regexpLike(inputStr, regexPatternStr);
   }
+
+  /**
+   * Checks whether the input string can be parsed into a json node or not. Useful for scenarios where we want
+   * to filter out malformed json. In case of nulls we return null itself, as null can be treated as valid json
+   * in partial-upsert scenarios. Upto the user to use null response accordingly.
+   *
+   * @param inputStr Input string to test for valid json
+   * @return in case of null value, it returns null. In case of non-null, it returns true in case of valid json

Review Comment:
   Same comment as earlier for a null argument.



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


Re: [PR] Add isJson UDF [pinot]

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


##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java:
##########
@@ -831,4 +832,24 @@ public static boolean like(String inputStr, String likePatternStr) {
     String regexPatternStr = RegexpPatternConverterUtils.likeToRegexpLike(likePatternStr);
     return regexpLike(inputStr, regexPatternStr);
   }
+
+  /**
+   * Checks whether the input string can be parsed into a json node or not. Useful for scenarios where we want
+   * to filter out malformed json. In case of nulls, it is treated as valid json as in partial-upsert we might
+   * want to treat that column value as valid.
+   *
+   * @param inputStr Input string to test for valid json
+   */
+  @ScalarFunction
+  public static boolean isJson(String inputStr) {
+    try {
+      if (inputStr == null) {

Review Comment:
   Yeah sounds good to me! Updated 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


Re: [PR] Add isJson UDF [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #12603:
URL: https://github.com/apache/pinot/pull/12603#discussion_r1521814981


##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java:
##########
@@ -831,4 +832,24 @@ public static boolean like(String inputStr, String likePatternStr) {
     String regexPatternStr = RegexpPatternConverterUtils.likeToRegexpLike(likePatternStr);
     return regexpLike(inputStr, regexPatternStr);
   }
+
+  /**
+   * Checks whether the input string can be parsed into a json node or not. Useful for scenarios where we want
+   * to filter out malformed json. In case of nulls we return null itself, as null can be treated as valid json

Review Comment:
   @tibrewalpratik17 We may update the javadoc. `null` values are handled by the caller (function invoker), instead of within this function



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


Re: [PR] Add isJson UDF [pinot]

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


##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java:
##########
@@ -831,4 +832,24 @@ public static boolean like(String inputStr, String likePatternStr) {
     String regexPatternStr = RegexpPatternConverterUtils.likeToRegexpLike(likePatternStr);
     return regexpLike(inputStr, regexPatternStr);
   }
+
+  /**
+   * Checks whether the input string can be parsed into a json node or not. Useful for scenarios where we want
+   * to filter out malformed json. In case of nulls, it is treated as valid json as in partial-upsert we might
+   * want to treat that column value as valid.
+   *
+   * @param inputStr Input string to test for valid json
+   */
+  @ScalarFunction
+  public static boolean isJson(String inputStr) {
+    try {
+      if (inputStr == null) {

Review Comment:
   @Jackie-Jiang the suggestion will return `false` for `null` right as it would be an exception? Do you mean to return Object instead? 



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