[GitHub] [pinot] amrishlal commented on a change in pull request #7820: push JSON Path evaluation down to storage layer

[GitBox <gi...@apache.org>]

amrishlal commented on a change in pull request #7820:
URL: https://github.com/apache/pinot/pull/7820#discussion_r759667776



##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/JsonPathQueriesTest.java
##########
@@ -177,40 +206,103 @@ private void checkresult(String query, Object[][] expecteds) {
       Assert.assertEquals(actual, expected);
     }
   }
+  @DataProvider
+  public static Object[][] allJsonColumns() {
+    return new Object[][]{
+        {JSON_COLUMN},
+        {RAW_JSON_COLUMN},
+        {JSON_COLUMN_WITHOUT_INDEX},
+        {RAW_BYTES_COLUMN},
+        {DICTIONARY_BYTES_COLUMN},
+        {RAW_STRING_COLUMN},
+        {DICTIONARY_STRING_COLUMN},
+    };
+  }
+
+  @DataProvider
+  public static Object[][] nativeJsonColumns() {
+    return new Object[][]{
+        {JSON_COLUMN},
+        {RAW_JSON_COLUMN},
+        {JSON_COLUMN_WITHOUT_INDEX},
+    };
+  }
+
+  @DataProvider
+  public static Object[][] nonNativeJsonColumns() {
+    // columns where we should be able to extract JSON with a function, but can't use all the literal features
+    return new Object[][]{
+        {RAW_BYTES_COLUMN},
+        {DICTIONARY_BYTES_COLUMN},
+        {RAW_STRING_COLUMN},
+        {DICTIONARY_STRING_COLUMN},
+    };
+  }
+
+  @Test(dataProvider = "nonNativeJsonColumns")
+  public void testExtractJsonField(String column) {
+    Object[][] expecteds1 = {{"duck"}, {"mouse"}, {"duck"}};
+    checkresult("SELECT jsonextractscalar(" + column + ", '$.name.last', 'STRING') FROM testTable LIMIT 3", expecteds1);
+  }
+
+  @Test(dataProvider = "allJsonColumns")
+  public void testNestedExtractJsonField(String column) {
+    Object[][] expecteds1 = {{"duck"}, {"mouse"}, {"duck"}};
+    checkresult("SELECT jsonextractscalar(jsonextractscalar(" + column
+        + ", '$.name', 'STRING'), '$.last', 'STRING') FROM testTable LIMIT 3", expecteds1);
+  }
 
   /** Test that a json path expression in SELECT list is properly converted to a JSON_EXTRACT_SCALAR function within
    * an AS function. */
-  @Test
-  public void testJsonSelect() {
+  @Test(dataProvider = "nativeJsonColumns")
+  public void testJsonSelect(String column) {
     // SELECT using a simple json path expression.
     Object[][] expecteds1 = {{"duck"}, {"mouse"}, {"duck"}};
-    checkresult("SELECT jsonColumn.name.last FROM testTable LIMIT 3", expecteds1);
+    checkresult("SELECT " + column + ".name.last FROM testTable LIMIT 3", expecteds1);
 
     Object[][] expecteds2 =
         {{"null"}, {"null"}, {"null"}, {"null"}, {"null"}, {"null"}, {"null"}, {"null"}, {"null"}, {"1"}};
-    checkresult("SELECT jsonColumn.data[0].e[2].z[0].i1 FROM testTable", expecteds2);
+    checkresult("SELECT " + column + ".data[0].e[2].z[0].i1 FROM testTable", expecteds2);
   }
 
   /** Test that a predicate comparing a json path expression with literal is properly converted into a JSON_MATCH
    * function. */
   @Test
   public void testJsonFilter() {
     // Comparing json path expression with a string value.
+    // note that only int, long, and string columns are projected so more kinds of json fields can be added without
+    // disrupting this test
     Object[][] expecteds1 =

Review comment:
       Please make sure that the existing test cases continue to work as is without modifying the expected results. To ensure compatiblity, I would suggest leaving the original test cases as is and adding new test cases if needed.

##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/JsonPathQueriesTest.java
##########
@@ -55,17 +60,37 @@
   private static final String LONG_COLUMN = "longColumn";
   private static final String STRING_COLUMN = "stringColumn";
   private static final String JSON_COLUMN = "jsonColumn";
+  private static final String RAW_JSON_COLUMN = "rawJsonColumn";
+  private static final String RAW_BYTES_COLUMN = "rawBytesColumn";
+  private static final String DICTIONARY_BYTES_COLUMN = "dictionaryBytesColumn";
+  private static final String RAW_STRING_COLUMN = "rawStringColumn";
+  private static final String DICTIONARY_STRING_COLUMN = "dictionaryStringColumn";
   private static final String JSON_COLUMN_WITHOUT_INDEX = "jsonColumnWithoutIndex";
 
   private static final Schema SCHEMA = new Schema.SchemaBuilder().setSchemaName(RAW_TABLE_NAME)
       .addSingleValueDimension(INT_COLUMN, FieldSpec.DataType.INT)
       .addSingleValueDimension(LONG_COLUMN, FieldSpec.DataType.LONG)
       .addSingleValueDimension(STRING_COLUMN, FieldSpec.DataType.STRING)
       .addSingleValueDimension(JSON_COLUMN, FieldSpec.DataType.JSON)
+      .addSingleValueDimension(RAW_JSON_COLUMN, FieldSpec.DataType.JSON)
+      .addSingleValueDimension(RAW_BYTES_COLUMN, FieldSpec.DataType.BYTES)

Review comment:
       JSON queries should work only on columns of JSON datatype, not on BYTES or STRING (I thought this was explicitly blocked already?). For backward compatibility, we still have JSON_EXTRACT_SCALAR and JSON_MATCH functions working on STRING column.




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