[#2266] fix(partition): enable preassign partition when creating range and list partitioning table #3189
Conversation
|
@mchades |
Corresponding to specific Doris catalog usage scenarios: |
|
@FANNG1 @jerryshao Please help to review when you have time, thanks! |
| default Expression[] assignments() { | ||
| return Expression.EMPTY_EXPRESSION; | ||
| default Partition[] assignments() { | ||
| return EMPTY_PARTITIONS; |
There was a problem hiding this comment.
What is the reason of this change?
There was a problem hiding this comment.
Because the previous API confused Partitioning (same as Transform) with Partition, which is a wrong implementation, the assignments should be partitions, so here is the Partition instead of Expression to fix the mistake.
common/src/test/java/com/datastrato/gravitino/json/TestDTOJsonSerDe.java
Outdated
Show resolved
Hide resolved
| .withLower(LiteralDTO.NULL) | ||
| .build(); | ||
| Partitioning rangePart = | ||
| RangePartitioningDTO.of(field1, new RangePartitionDTO[] {p20230101, p20230102}); |
There was a problem hiding this comment.
Can you please also add some failure tests for your change in serde.
There was a problem hiding this comment.
Also, can you please add a mock test both in server and client side to see if the serde works well.
yes, this PR only did the API refactor, if catalog(like Doris) has the requirement, it can add the pre-assigned partition when creating the table, I think this can be done in another PR |
…e and list partitioning table (#3189) ### What changes were proposed in this pull request? enable pre-assign partition when creating range and list partitioning table ### Why are the changes needed? Fix: #2266 ### Does this PR introduce _any_ user-facing change? yes, enable pre-assign partition when creating range and list partitioning table ### How was this patch tested? tests modified
…g range and list partitioning table (apache#3189) ### What changes were proposed in this pull request? enable pre-assign partition when creating range and list partitioning table ### Why are the changes needed? Fix: apache#2266 ### Does this PR introduce _any_ user-facing change? yes, enable pre-assign partition when creating range and list partitioning table ### How was this patch tested? tests modified
What changes were proposed in this pull request?
enable pre-assign partition when creating range and list partitioning table
Why are the changes needed?
Fix: #2266
Does this PR introduce any user-facing change?
yes, enable pre-assign partition when creating range and list partitioning table
How was this patch tested?
tests modified