amrishlal commented on a change in pull request #6651:
URL: https://github.com/apache/incubator-pinot/pull/6651#discussion_r589764720



##########
File path: 
pinot-core/src/test/java/org/apache/pinot/queries/JsonMatchPredicateTest.java
##########
@@ -282,6 +282,36 @@ public void testJsonMatchArrayWithIndex() {
     Assert.assertEquals(iterator.next()[0], "goofy");
   }
 
+  /** Evaluate json_extract_scalar over string column that does not contain 
valid json data. */
+  @Test
+  public void testJsonExtractScalarAgainstInvalidJson() {
+    // json_extract_scalar throws exception since we are trying to parse a 
non-JSON string.
+    Operator operator1 = getOperatorForSqlQuery(
+        "select count(*) FROM testTable WHERE 
json_extract_scalar(stringColumn, '$.name.first', 'INT') = 0");
+    try {
+      IntermediateResultsBlock block1 = (IntermediateResultsBlock) 
operator1.nextBlock();
+      Assert.assertTrue(false);
+    } catch (RuntimeException re) {
+      Assert.assertEquals(re.toString(),
+          "java.lang.RuntimeException: Illegal Json Path: [$.name.first], when 
reading [daffy duck]");
+    }
+
+    // JSON data is stored in columns of type STRING, so there is nothing 
preventing the column from storing bad json
+    // string. Bad JSON string in columns will cause json_extract_scalar to 
throw an exception which would terminate
+    // query processing. However, when json_extract_scalar is used within the 
WHERE clause, we should return the
+    // default value instead of throwing exception. This will allow the 
predicate to be evaluated to either true or
+    // false and hence allow the query to complete successfully. Returning 
default value from json_extract_scalar is
+    // an undocumented feature. Ideally, json_extract_scalar should return 
NULL in when it encounters bad JSON. However,
+    // NULL support is currently pending, so this is the best we can do.
+    Operator operator2 = getOperatorForSqlQuery(
+        "select count(*) FROM testTable WHERE 
json_extract_scalar(stringColumn, '$.name.first', 'INT', 0) = 0");
+
+    IntermediateResultsBlock block2 = (IntermediateResultsBlock) 
operator2.nextBlock();
+    Collection<Object[]> rows = block2.getSelectionResult();
+
+    Assert.assertEquals(block2.getAggregationResult().get(0), 9l);

Review comment:
       Done.




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to