richardstartin commented on a change in pull request #7820: URL: https://github.com/apache/pinot/pull/7820#discussion_r759974790
########## 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: I'm not sure I agree about that, I don't think that this test was stable w.r.t. extension of the test setup. In order to add columns to the test setup (e.g. to test a RAW JSON column, which was not previously tested, and is now) `expecteds1` would always need to be extended to include every column added. So there is a choice: add the values to `expected1`, duplicate the whole test _setup_ to add the new columns for a new test, or control what is projected to keep this test stable. Since this is a test for selection and not projection, I opted to do the latter. So do you want me to duplicate the test setup so I can add tests for querying a RAW JSON column, or change the query back to `select *` and add all new columns to `expected1`? -- 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-unsubscr...@pinot.apache.org 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