[core] Add JSON serialization support for Predicate and Transform#6993
[core] Add JSON serialization support for Predicate and Transform#6993JingsongLi merged 11 commits intoapache:masterfrom
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! |
plusplusjiajia
changed the title
plusplusjiajia
force-pushed
the
predicate-parse
branch
5 times, most recently
from
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.
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.
Something like DataTypeJsonParserTest.
There was a problem hiding this comment.
DataTypeJsonParserTest
Thansk for you review. It's really a useful suggestion, I've updated the test.
plusplusjiajia
force-pushed
the
predicate-parse
branch
from
6dfd792 to
09f286d
Compare
|
I try to unify them in #7015 |
|
Please rebase master. |
good job! |
plusplusjiajia
force-pushed
the
predicate-parse
branch
from
09f286d to
0e43d41
Compare
| include = JsonTypeInfo.As.PROPERTY, | ||
| property = Transform.Types.FIELD_TYPE) | ||
| @JsonSubTypes({ | ||
| @JsonSubTypes.Type(value = FieldTransform.class, name = Transform.Types.FIELD), |
There was a problem hiding this comment.
Can we use Transform.name here? Maybe we can extract a static field for sub Transform.name, and ref it here.
plusplusjiajia
force-pushed
the
predicate-parse
branch
from
b58f6ce to
6f7dd90
Compare
| // LeafPredicate - Equal | ||
| TestSpec.forPredicate(builder.equal(0, 1)) | ||
| .expectJson( | ||
| "{\"type\":\"LEAF\",\"transform\":{\"type\":\"FIELD_REF\",\"fieldRef\":{\"index\":0,\"name\":\"f0\",\"type\":\"INT\"}},\"function\":{\"type\":\"EQUAL\"},\"literals\":[1]}"), |
There was a problem hiding this comment.
Can we just let function: EQUAL? Just a string, not a struct.
| @JsonSubTypes.Type(value = UpperTransform.class, name = UpperTransform.NAME) | ||
| }) | ||
| public interface Transform extends Serializable { | ||
| String FIELD_TYPE = "type"; |
There was a problem hiding this comment.
"transform"? "type" is confused to Data Type.
| }) | ||
| public interface Predicate extends Serializable { | ||
|
|
||
| String FIELD_TYPE = "type"; |
There was a problem hiding this comment.
"predicate"? "type" is confused to Data Type.
…orm." This reverts commit d7bf092.
plusplusjiajia
force-pushed
the
predicate-parse
branch
from
de8416f to
ceec8a0
Compare
| @JsonSubTypes.Type(value = UpperTransform.class, name = UpperTransform.NAME) | ||
| }) | ||
| public interface Transform extends Serializable { | ||
| String FIELD_TRANSFORM = "transform"; |
There was a problem hiding this comment.
"name" is better.
done
| }) | ||
| public interface Predicate extends Serializable { | ||
|
|
||
| String FIELD_PREDICATE = "predicate"; |
There was a problem hiding this comment.
Let's modify it to kind?
There was a problem hiding this comment.
Let's modify it to
kind?
done
2 participants
Purpose
Add Predicate and Transform json serialization utils