-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[core] Add JSON serialization support for Predicate and Transform #6993
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?
Conversation
|
If possible, we'd better encapsulate the JSON serialization into specific classes, as this is the most effective way to avoid omissions. |
Excellent suggestion! |
5d8a1fe to
c9895bf
Compare
|
|
|
||
| private static Predicate roundTrip(Predicate predicate) { | ||
| String json = JsonSerdeUtil.toFlatJson(predicate); | ||
| return JsonSerdeUtil.fromJson(json, Predicate.class); |
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 have JSON expected result in the tests? This is a standard. We need to solidify serialization-related issues in the test at least.
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.
Something like DataTypeJsonParserTest.
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.
DataTypeJsonParserTest
Thansk for you review. It's really a useful suggestion, I've updated the test.
6dfd792 to
09f286d
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.
Only two types: leaf and compound, TransformPredicate should be leaf type too.
- type
leaf:transform,functionandliterals. - type
compound:functionandchildren.
|
I try to unify them in #7015 |
Purpose
Add Predicate and Transform json serialization utils