Add jq expression support in flattenSpec by knoguchi · Pull Request #4171 · apache/druid

knoguchi · 2017-04-14T02:38:35Z

This PR adds a new expression type "jq" to the flattenSpec. "jq" is a well known JSON manipulation tool. JSONPath is ok for extracting values out of the nested data structure but it lacks value translation capability. jq is pretty good at it and it can be used as ETL tool, effectively.

Example: a JSON has secondField and nanosecField fields where integer values are stored, and you want to extract the sum of the two fields, and get the derived field totalSecond. Another example is to apply regex on URL and get directory name. The flattenSpec can be written like this.

"flattenSpec": {
    "fields": [
      {
         "type": "jq",
         "name": "totalSecond",
         "expr": ".secondField + .nanosecField / 1000000000"
      },
      {
        "type": "jq",
        "name": "directory",
        "expr": ".url | capture(\"(?<dir>\/[^\/]+/\", \"\") | .dir)"
      } 
   ]
}

The benchmark that accesses nested fields shows no significant difference than JSONPath.

JsonPath related code has been modified to use JacksonJsonNodeJsonProvider and JsonNode instead of JacksonJsonProvider and Map<String, Object> so that both JsonPath and Jackson-JQ use the same JsonNode document.

The only ugly part is the newly added FlattenExpr adapter that wraps JsonPath and JsonQuery classes. I'm not sure the best way to create a common interface for the 3rd party classes. Please enlighten me if there is a better way.

Lastly, please note, the jq library used in this PR is jackson-jq that is a Java port, not the original jq nor libjq wrapper.

…add-jq-expr

knoguchi · 2017-07-23T16:35:24Z

@gianm I appreciate if you'd review. I'm using this in my production system.

himanshug · 2017-09-01T21:38:50Z

@knoguchi sounds like useful feature however use of https://github.com/eiiches/jackson-jq is questionable, I'm not really sure about all the legalities but mentioned repository doesn't have a LICENSE file or any apache etc headers on any of the source code.
also, it copies some things from https://github.com/stedolan/jq/blob/master/COPYING which is not apache licensed either.

knoguchi · 2017-09-01T22:41:26Z

@himanshug Thanks for the comment. jackson-jq itself is Apache2.0 per https://github.com/eiiches/jackson-jq/blob/develop/COPYING#L10. jq is MIT per manual. jq uses a library copyrighted by Lucent Technology -- the wording appears to be FOSS compatible but I'm not a lawyer to say for sure.

gianm

Thanks @knoguchi for the contribution. This looks good to me with some minor changes.

gianm · 2017-09-03T20:30:15Z

+      <dependency>
+        <groupId>net.thisptr</groupId>
+        <artifactId>jackson-jq</artifactId>
+        <version>RELEASE</version>


Please fix this to a specific version, and also, please put the version in the dependencyManagement section of the main pom.

gianm · 2017-09-03T20:30:37Z

+        <dependency>
+            <groupId>net.thisptr</groupId>
+            <artifactId>jackson-jq</artifactId>
+            <version>0.0.7</version>


The specific version should go in the dependencyManagement section of the main pom (it helps keep versions in sync across modules)

gianm · 2017-09-03T20:33:00Z

+          path = new FlattenExpr(JsonQuery.compile(fieldSpec.getExpr()));
+        }
+        catch (JsonQueryException e) {
+          throw new ParseException(e, "Unable to compile JQ expression [%s]", fieldSpec.getExpr());


It would be better for this to be an IllegalArgumentException. ParseExceptions are meant to represent parse errors on individual lines, but this error applies to the entire flatten spec.

gianm · 2017-09-03T20:34:12Z

+    fields.add(JSONPathFieldSpec.createRootField("ts"));
+
+    fields.add(JSONPathFieldSpec.createRootField("d1"));
+    //fields.add(JSONPathFieldSpec.createRootField("d2"));


Please don't include commented out code.

gianm · 2017-09-03T20:34:19Z

+    fields.add(JSONPathFieldSpec.createJqField("ae1[2].e1.d2", ".ae1[2].e1.d2"));
+
+    fields.add(JSONPathFieldSpec.createRootField("m3"));
+    //fields.add(JSONPathFieldSpec.createRootField("m4"));


Please don't include commented out code.

gianm · 2017-09-03T20:34:58Z

Based on the following evidence the licensing seems ok to me.

The Maven artifact lists an Apache license.
The COPYING file in the source repo spells out the licensing info in more detail. It says portions are copyright Eiichi Sato and are Apache licensed, and portions are copyright Stephen Dolan and are MIT licensed.
https://www.apache.org/legal/resolved.html says the ASF considers MIT licensed dependencies valid for use in Apache licensed projects. So the mix of Apache and MIT licensed code should be fine by that logic.

himanshug · 2017-09-05T17:35:50Z

@b-slim can you please check jackson-jq is alright to be added in apache license project.

b-slim · 2017-09-06T03:44:41Z

@himanshug this looks good to me, thanks !

himanshug · 2017-09-06T19:58:15Z

@b-slim thanks for checking that.

took another look today, LGTM . 👍 after existing comments are addressed.

knoguchi · 2017-09-06T20:19:15Z

Thanks all for the reviews and comments. I will fix them shortly.

…nted code

knoguchi · 2017-09-07T16:57:34Z

I've addressed the comments. Not sure why Travis failed for the last commit 79c7baa

Kenji Noguchi added 3 commits April 13, 2017 19:12

add jq expression in the flattenSpec

2054807

more tests

20fe0c2

add benchmark

a47efee

knoguchi changed the title ~~[experimental] add jq expression support in flattenSpec~~ Add jq expression support in flattenSpec May 25, 2017

Kenji Noguchi added 7 commits May 25, 2017 16:29

fix style

56e687d

Merge branch 'add-jq-expr' of https://github.com/knoguchi/druid into …

beb38ae

…add-jq-expr

use JsonNode for both JSONPath and JQ

5d8a343

clean up

70c0350

more clean up

47ba363

add documentation

d327858

fix style

a1bc0ee

gianm reviewed Sep 3, 2017

View reviewed changes

gianm added the Feature label Sep 3, 2017

gianm added this to the 0.11.0 milestone Sep 3, 2017

Kenji Noguchi added 5 commits September 6, 2017 14:21

move jackson-jq version to dependencyManagement section. remove comme…

0fe3d1d

…nted code

oops. revert wrong fix

c140788

throw IllegalArgumentException for JQ syntax error

24eeefa

remove e.printStackTrace() that is forbidden

5e4954d

touch

79c7baa

himanshug closed this Sep 7, 2017

himanshug reopened this Sep 7, 2017

pjain1 closed this Sep 8, 2017

pjain1 reopened this Sep 8, 2017

himanshug merged commit c0be050 into apache:master Sep 12, 2017

gianm mentioned this pull request Sep 13, 2017

Try to improve CI reliability. #4787

Merged

jon-wei mentioned this pull request Sep 28, 2017

Druid 0.11.0 release notes #4876

Closed

knoguchi deleted the add-jq-expr branch May 15, 2019 21:31

Conversation

knoguchi commented Apr 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

knoguchi commented Jul 23, 2017

Uh oh!

himanshug commented Sep 1, 2017

Uh oh!

knoguchi commented Sep 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gianm left a comment

Choose a reason for hiding this comment

Uh oh!

gianm Sep 3, 2017

Choose a reason for hiding this comment

Uh oh!

gianm Sep 3, 2017

Choose a reason for hiding this comment

Uh oh!

gianm Sep 3, 2017

Choose a reason for hiding this comment

Uh oh!

gianm Sep 3, 2017

Choose a reason for hiding this comment

Uh oh!

gianm Sep 3, 2017

Choose a reason for hiding this comment

Uh oh!

gianm commented Sep 3, 2017

Uh oh!

himanshug commented Sep 5, 2017

Uh oh!

b-slim commented Sep 6, 2017

Uh oh!

himanshug commented Sep 6, 2017

Uh oh!

knoguchi commented Sep 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

knoguchi commented Sep 7, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

knoguchi commented Apr 14, 2017 •

edited

Loading

knoguchi commented Sep 1, 2017 •

edited

Loading

knoguchi commented Sep 6, 2017 •

edited

Loading