amrishlal commented on a change in pull request #6651:
URL: https://github.com/apache/incubator-pinot/pull/6651#discussion_r589768294
##########
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(),
Review comment:
Removed validation.
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/JsonExtractScalarTransformFunction.java
##########
@@ -131,7 +131,7 @@ public TransformResultMetadata getResultMetadata() {
final String[] stringValuesSV =
_jsonFieldTransformFunction.transformToStringValuesSV(projectionBlock);
final int[] results = new int[projectionBlock.getNumDocs()];
for (int i = 0; i < results.length; i++) {
- Object read = JsonPath.read(stringValuesSV[i], _jsonPath);
+ Object read =
JsonPath.using(JSON_PARSER_CONFIG).parse(stringValuesSV[i]).read(_jsonPath);
Review comment:
I am a bit hesitant to do this since since it will allocate static
memory for Parser, but if this function is never used in query, the parser will
still take up memory space. Although, it seems like there is scope for both
performance and memory improvement here. For example, currently
`JsonExtractScalarFunction` object will be constructed fresh for each query
that uses `json_extract_scalar` and if the query has multiple instances of
`json_extract_scalar` then `JsonExtractScalarFunction` will be constructed
multiple times. So one option may be to add a lazy function object cache to
`TransformationFunctionFactory` to reuse function objects. Although, this is
probably better handled in its own PR.
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]