-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[FLINK-18590][json] Support json array explode to multi messages #26473
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
[FLINK-18590][json] Support json array explode to multi messages #26473
Conversation
ping @masteryhx |
Usually, we assume the top-level of json string is a json object. Then the json object is converted to one SQL row. | ||
|
||
There are some cases that, the top-level of json string is a json array, and we want to explode the array to | ||
multiple records, each one of the array is a json object which is converted to one row. Flink JSON Format supports |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit : each one of the array is a json object->each one of the array is a json object, primitive or a json array
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should explicity say what we are going with nested arrays in the docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that each element within the stringified JSON array can only be a JSON object. Moreover, the schema of these JSON objects must be consistent with what is defined in the SQL.
...link-json/src/main/java/org/apache/flink/formats/json/AbstractJsonDeserializationSchema.java
Outdated
Show resolved
Hide resolved
element2.put("f2", false); | ||
element2.put("f3", "newStr"); | ||
|
||
ArrayNode arrayNode = objectMapper.createArrayNode(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we test arrays of arrays of arrays and arrays of objects of arrays
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am curious with arrays of arrays, do we create multiple records only at the top level or should we have an option to expand the nested arrays to multiple records also. Should this be a format configuration option ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume that it's more complicated structure and not supported yet.
I'm fine that we remain it as unsupported and add related description in the doc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume that it's more complicated structure and not supported yet. I'm fine that we remain it as unsupported and add related description in the doc.
I have added related description in doc.
Each element within the array is a JSON object, the schema of every such JSON object is the same as defined in SQL, and each of these JSON objects can be converted into one row
...link-json/src/main/java/org/apache/flink/formats/json/AbstractJsonDeserializationSchema.java
Outdated
Show resolved
Hide resolved
...flink-json/src/main/java/org/apache/flink/formats/json/JsonRowDataDeserializationSchema.java
Outdated
Show resolved
Hide resolved
@flinkbot run azure |
@flinkbot run azure |
@flinkbot run azure |
@flinkbot run azure |
1 similar comment
@flinkbot run azure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
@lsyldliu @xuyangzhong Do you have other comments ?
What is the purpose of the change
Brief change log
Verifying this change
This change added tests and can be verified as follows:
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: (yes / no)Documentation