[#3347] improvement(jdbc-doris): support creating Doris table with partition #3350
Conversation
xiaozcy
changed the title
|
@mchades please help to review this. |
...ris/src/main/java/com/datastrato/gravitino/catalog/doris/operation/DorisTableOperations.java
Outdated
Show resolved
Hide resolved
| Set<String> columnNames = | ||
| Arrays.stream(columns).map(JdbcColumn::name).collect(Collectors.toSet()); | ||
|
|
||
| if (partitioning[0] instanceof Transforms.RangeTransform) { |
There was a problem hiding this comment.
@zhoukangcn
Could you help to review this PR if you have time? I see no problem here.
|
I found that the previous implementation of multi-column range partitioning may conflict with the current API design of Gravitino, so we will not support multi-column range partitioning in Doris for the time being. |
...ris/src/main/java/com/datastrato/gravitino/catalog/doris/operation/DorisTableOperations.java
Show resolved
Hide resolved
| "The partition field must be one of the columns"); | ||
|
|
||
| String partitionColumn = BACK_QUOTE + rangePartition.fieldName()[0] + BACK_QUOTE; | ||
| // TODO we currently do not support pre-assign partition when creating range partitioning |
There was a problem hiding this comment.
Pre-assign partition has been available since merge #3189.
Would you like to implement it in this PR or a separate one? If not in this PR, kindly open a new issue for tracking this.
There was a problem hiding this comment.
Thanks for informing, I will do it in another PR, issue #3368 is created.
| Distributions.hash(DEFAULT_BUCKET_SIZE, NamedReference.field("col_1")); | ||
| Index[] indexes = new Index[] {}; | ||
|
|
||
| // create table with range partition |
There was a problem hiding this comment.
It's better to add a read/write test for the partitioned table in com.datastrato.gravitino.catalog.doris.integration.test.CatalogDorisIT
There was a problem hiding this comment.
You mean adding integration tests for creating and loading partitioned table in CatalogDorisIT?
There was a problem hiding this comment.
And add read/write tests like HiveCatalogIT
There was a problem hiding this comment.
Since we do not yet support the operations of pre-assign and add partitions for Doris, the created partitioned table cannot be written into directly.
In addition, we would have to bypass Gravitino for tests that read and write data, e.g. using spark or JDBC, which doesn't seem to be necessary for Gravitino in my opinion.
There was a problem hiding this comment.
Because partition operations are not supported at the moment, it is impossible to test the read and write of partitioned tables. I think this makes sense.
In addition, we would have to bypass Gravitino for tests that read and write data, e.g. using spark or JDBC, which doesn't seem to be necessary for Gravitino in my opinion.
We need to ensure that tables created through Gravitino can be read and written smoothly by other engines, so it is very necessary to test reading and writing partitioned tables when the Doris catalog supports partition operations.
There was a problem hiding this comment.
@yuqi1129 @zhoukangcn Overall LGTM, do you have further comments?
…rtition (#3350) ### What changes were proposed in this pull request? support creating Doris table with partition ### Why are the changes needed? Fix: #3347 ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? IT --------- Co-authored-by: zhanghan18 <zhanghan18@xiaomi.com>
What changes were proposed in this pull request?
support creating Doris table with partition
Why are the changes needed?
Fix: #3347
Does this PR introduce any user-facing change?
no
How was this patch tested?
IT