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
[FLINK-16627][json]Support ignore null fields when serializing into JSON #24430
Conversation
1df5347
to
43667dc
Compare
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.
@RubyChou generally looks good, I only left some minor comments about the styles and tests.
<td>选填</td> | ||
<td style="word-wrap: break-word;">false</td> | ||
<td>Boolean</td> | ||
<td>仅序列化非Null的列,默认情况下,会序列化所有列无论是否为Null。</td> |
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.
In Chinese document, we usually add spaces between english words, see more here: https://cwiki.apache.org/confluence/display/FLINK/Flink+Translation+Specifications
<td>仅序列化非Null的列,默认情况下,会序列化所有列无论是否为Null。</td> | |
<td>仅序列化非 Null 的列,默认情况下,会序列化所有列无论是否为 Null。</td> |
<td>optional</td> | ||
<td style="word-wrap: break-word;">false</td> | ||
<td>Boolean</td> | ||
<td>Encode only non-null fields. By default, all fields will be written.</td> |
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.
<td>Encode only non-null fields. By default, all fields will be written.</td> | |
<td>Encode only non-null fields. By default, all fields will be included.</td> |
@@ -626,6 +635,62 @@ void testSerializationDecimalEncode() throws Exception { | |||
assertThat(scientificDecimalResult).isEqualTo(scientificDecimalJson); | |||
} | |||
|
|||
@TestTemplate | |||
public void testSerDeMultiRowsWithNullValuesIgnored() throws Exception { |
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.
Since we have migrated to Junit5, public
is no longer needed, see more here: https://docs.google.com/document/d/1514Wa_aNB9bJUen4xm5uiuXOooOJTtXqS_Jqk9KJitU/edit#heading=h.4iilyfqpoiw2
public void testSerDeMultiRowsWithNullValuesIgnored() throws Exception { | |
void testSerDeMultiRowsWithNullValuesIgnored() throws Exception { |
@@ -89,6 +90,7 @@ void testUserDefinedOptions() { | |||
options.put("canal-json.map-null-key.mode", "LITERAL"); | |||
options.put("canal-json.map-null-key.literal", "nullKey"); | |||
options.put("canal-json.encode.decimal-as-plain-number", "true"); | |||
options.put("canal-json.encode.ignore-null-fields", "true"); |
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.
Why are you enable Canal json format to test "encode.ignore-null-fields" while leave others (debezium, maxwell, ogg) out?
hi @libenchao, I've resolved all comments above. Please help reviewing. Thanks. |
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.
+1, merging.
<tr> | ||
<td><h5>json.encode.ignore-null-fields</h5></td> | ||
<td>选填</td> | ||
<td style="word-wrap: break-word;">false</td> |
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 think this option can be forwarded.
What is the purpose of the change
This pull request introduces
json.encode.ignore-null-fields
for JSON formats which indicates whether to ignore null fields when serializing into JSON.Brief change log
Verifying this change
This change added tests and can be verified as follows:
JsonRowDataSerDeSchemaTest
to check whether the result of serialization is expected.Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: (no)Documentation