You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/01/26 18:58:12 UTC
[GitHub] [incubator-pinot] fx19880617 opened a new pull request #6490: Adding scalar function JsonPathArray to extract arrays from json
fx19880617 opened a new pull request #6490:
URL: https://github.com/apache/incubator-pinot/pull/6490
## Description
Adding scalar function JsonPathArray to extract arrays from 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.
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
[GitHub] [incubator-pinot] kishoreg commented on a change in pull request #6490: Adding scalar function JsonPathArray to extract arrays from json
Posted by GitBox <gi...@apache.org>.
kishoreg commented on a change in pull request #6490:
URL: https://github.com/apache/incubator-pinot/pull/6490#discussion_r564820569
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java
##########
@@ -21,6 +21,7 @@
import com.fasterxml.jackson.core.JsonProcessingException;
import com.jayway.jsonpath.JsonPath;
import java.util.Map;
+import net.minidev.json.JSONArray;
Review comment:
why another json library?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #6490: Adding scalar function JsonPathArray to extract arrays from json
Posted by GitBox <gi...@apache.org>.
fx19880617 commented on a change in pull request #6490:
URL: https://github.com/apache/incubator-pinot/pull/6490#discussion_r564890271
##########
File path: pinot-common/src/test/java/org/apache/pinot/common/function/JsonFunctionsTest.java
##########
@@ -65,4 +65,42 @@ public void testJsonFunction()
assertEquals(JsonFunctions.jsonPathDouble(jsonString, "$.actor.aaa", 53.2), 53.2);
}
+ @Test
+ public void testJsonFunctionExtractingArray()
+ throws JsonProcessingException {
+ String jsonString = "{\n" +
+ " \"name\": \"Pete\",\n" +
+ " \"age\": 24,\n" +
+ " \"subjects\": [\n" +
+ " {\n" +
+ " \"name\": \"maths\",\n" +
+ " \"grade\": \"A\"\n" +
+ " },\n" +
+ " {\n" +
+ " \"name\": \"maths\",\n" +
+ " \"grade\": \"B\"\n" +
+ " }\n" +
+ " ]\n" +
+ "}";
+ assertEquals(JsonFunctions.jsonPathArray(jsonString, "$.subjects[*].name"), new String[]{"maths", "maths"});
+ assertEquals(JsonFunctions.jsonPathArray(jsonString, "$.subjects[*].grade"), new String[]{"A", "B"});
+ }
+
+ @Test
+ public void testJsonFunctionOnJsonArray()
+ throws JsonProcessingException {
+ String jsonArrayString =
+ "[\n" +
+ " {\n" +
+ " \"name\": \"maths\",\n" +
+ " \"grade\": \"A\"\n" +
+ " },\n" +
Review comment:
added.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #6490: Adding scalar function JsonPathArray to extract arrays from json
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #6490:
URL: https://github.com/apache/incubator-pinot/pull/6490#discussion_r564874873
##########
File path: pinot-common/src/test/java/org/apache/pinot/common/function/JsonFunctionsTest.java
##########
@@ -65,4 +65,42 @@ public void testJsonFunction()
assertEquals(JsonFunctions.jsonPathDouble(jsonString, "$.actor.aaa", 53.2), 53.2);
}
+ @Test
+ public void testJsonFunctionExtractingArray()
+ throws JsonProcessingException {
+ String jsonString = "{\n" +
+ " \"name\": \"Pete\",\n" +
+ " \"age\": 24,\n" +
+ " \"subjects\": [\n" +
+ " {\n" +
+ " \"name\": \"maths\",\n" +
+ " \"grade\": \"A\"\n" +
+ " },\n" +
+ " {\n" +
+ " \"name\": \"maths\",\n" +
+ " \"grade\": \"B\"\n" +
+ " }\n" +
+ " ]\n" +
+ "}";
+ assertEquals(JsonFunctions.jsonPathArray(jsonString, "$.subjects[*].name"), new String[]{"maths", "maths"});
+ assertEquals(JsonFunctions.jsonPathArray(jsonString, "$.subjects[*].grade"), new String[]{"A", "B"});
+ }
+
+ @Test
+ public void testJsonFunctionOnJsonArray()
+ throws JsonProcessingException {
+ String jsonArrayString =
+ "[\n" +
+ " {\n" +
+ " \"name\": \"maths\",\n" +
+ " \"grade\": \"A\"\n" +
+ " },\n" +
Review comment:
Also, scalar functions should also have e2e query execution unit tests. Alternatively, may be add a query compilation unit test as well to ensure it gets recognized/compiled as scalar
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #6490: Adding scalar function JsonPathArray to extract arrays from json
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #6490:
URL: https://github.com/apache/incubator-pinot/pull/6490#discussion_r564979320
##########
File path: pinot-common/src/test/java/org/apache/pinot/common/function/JsonFunctionsTest.java
##########
@@ -65,4 +65,42 @@ public void testJsonFunction()
assertEquals(JsonFunctions.jsonPathDouble(jsonString, "$.actor.aaa", 53.2), 53.2);
}
+ @Test
+ public void testJsonFunctionExtractingArray()
+ throws JsonProcessingException {
+ String jsonString = "{\n" +
+ " \"name\": \"Pete\",\n" +
+ " \"age\": 24,\n" +
+ " \"subjects\": [\n" +
+ " {\n" +
+ " \"name\": \"maths\",\n" +
+ " \"grade\": \"A\"\n" +
+ " },\n" +
+ " {\n" +
+ " \"name\": \"maths\",\n" +
+ " \"grade\": \"B\"\n" +
+ " }\n" +
+ " ]\n" +
+ "}";
+ assertEquals(JsonFunctions.jsonPathArray(jsonString, "$.subjects[*].name"), new String[]{"maths", "maths"});
+ assertEquals(JsonFunctions.jsonPathArray(jsonString, "$.subjects[*].grade"), new String[]{"A", "B"});
+ }
+
+ @Test
+ public void testJsonFunctionOnJsonArray()
+ throws JsonProcessingException {
+ String jsonArrayString =
+ "[\n" +
+ " {\n" +
+ " \"name\": \"maths\",\n" +
+ " \"grade\": \"A\"\n" +
+ " },\n" +
Review comment:
Should have mentioned earlier itself. Can we please add one test for empty array as well?
Also, Can we chain the jsonPathArray with jsonExtract as a transform and extract a particular value out of the array? Or for such cases, the recommended way is to directly use jsonExtract as opposed to doing jsonExtract(jsonPathArray(....)) ?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #6490: Adding scalar function JsonPathArray to extract arrays from json
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #6490:
URL: https://github.com/apache/incubator-pinot/pull/6490#discussion_r564872686
##########
File path: pinot-common/src/test/java/org/apache/pinot/common/function/JsonFunctionsTest.java
##########
@@ -65,4 +65,42 @@ public void testJsonFunction()
assertEquals(JsonFunctions.jsonPathDouble(jsonString, "$.actor.aaa", 53.2), 53.2);
}
+ @Test
+ public void testJsonFunctionExtractingArray()
+ throws JsonProcessingException {
+ String jsonString = "{\n" +
+ " \"name\": \"Pete\",\n" +
+ " \"age\": 24,\n" +
+ " \"subjects\": [\n" +
+ " {\n" +
+ " \"name\": \"maths\",\n" +
+ " \"grade\": \"A\"\n" +
+ " },\n" +
+ " {\n" +
+ " \"name\": \"maths\",\n" +
+ " \"grade\": \"B\"\n" +
+ " }\n" +
+ " ]\n" +
+ "}";
+ assertEquals(JsonFunctions.jsonPathArray(jsonString, "$.subjects[*].name"), new String[]{"maths", "maths"});
+ assertEquals(JsonFunctions.jsonPathArray(jsonString, "$.subjects[*].grade"), new String[]{"A", "B"});
+ }
+
+ @Test
+ public void testJsonFunctionOnJsonArray()
+ throws JsonProcessingException {
+ String jsonArrayString =
+ "[\n" +
+ " {\n" +
+ " \"name\": \"maths\",\n" +
+ " \"grade\": \"A\"\n" +
+ " },\n" +
Review comment:
Can we try nested array as well?
Something like subjects
`{ name, array of lastTwoSemesterGrades}`
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #6490: Adding scalar function JsonPathArray to extract arrays from json
Posted by GitBox <gi...@apache.org>.
fx19880617 commented on a change in pull request #6490:
URL: https://github.com/apache/incubator-pinot/pull/6490#discussion_r564890469
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java
##########
@@ -21,6 +21,7 @@
import com.fasterxml.jackson.core.JsonProcessingException;
import com.jayway.jsonpath.JsonPath;
import java.util.Map;
+import net.minidev.json.JSONArray;
Review comment:
Update the jayway config to use jackson by default.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #6490: Adding scalar function JsonPathArray to extract arrays from json
Posted by GitBox <gi...@apache.org>.
fx19880617 commented on a change in pull request #6490:
URL: https://github.com/apache/incubator-pinot/pull/6490#discussion_r564846949
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java
##########
@@ -21,6 +21,7 @@
import com.fasterxml.jackson.core.JsonProcessingException;
import com.jayway.jsonpath.JsonPath;
import java.util.Map;
+import net.minidev.json.JSONArray;
Review comment:
it's the internal impl for jsonPath lib
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #6490: Adding scalar function JsonPathArray to extract arrays from json
Posted by GitBox <gi...@apache.org>.
fx19880617 commented on a change in pull request #6490:
URL: https://github.com/apache/incubator-pinot/pull/6490#discussion_r564846949
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java
##########
@@ -21,6 +21,7 @@
import com.fasterxml.jackson.core.JsonProcessingException;
import com.jayway.jsonpath.JsonPath;
import java.util.Map;
+import net.minidev.json.JSONArray;
Review comment:
it's the internal impl for jayway jsonPath lib
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 merged pull request #6490: Adding scalar function JsonPathArray to extract arrays from json
Posted by GitBox <gi...@apache.org>.
fx19880617 merged pull request #6490:
URL: https://github.com/apache/incubator-pinot/pull/6490
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org