[format] Support Map<X, Blob> type for Blob File Format#8635
Conversation
|
cc @leaves12138 to take a look. |
JingsongLi
left a comment
There was a problem hiding this comment.
Also support this in PrimaryKeyBlobExternalizer?
|
@JingsongLi Of course! But this PR only focus on file format layer modification. Wiring to Table & Engine layer will be brought in subsequent PRs. I'm afraid that modifying all layers in one PR is too big to review. |
@steFaiz It is OK in a single PR. It’s important to have a complete implementation; if we run into difficulties during the review, we’ll split the PR then. |
d897d41 to
1a541f6
Compare
|
@JingsongLi This PR is ready to review! PTAL if you have some time! Thanks! |
| return false; | ||
| } | ||
|
|
||
| private static boolean isSupportedMapBlobKeyType(DataType keyType) { |
There was a problem hiding this comment.
Why need to limit this? Just let runtime to check this.
There was a problem hiding this comment.
I was meant to fast-fail on creating table. Do you think we do not have to fast fail here?
There was a problem hiding this comment.
Ultimately, we should support all key types supported by Paimon.
| private final ManagedBlobField[] blobFields; | ||
| private final List<Path> uncommittedPacks; | ||
|
|
||
| public PrimaryKeyBlobExternalizer( |
There was a problem hiding this comment.
Do not add constructor for test codes.
|
|
||
| @Nullable public BlobConsumer writeConsumer; | ||
|
|
||
| public BlobFileFormat() { |
There was a problem hiding this comment.
Do not add constructors for tests. Please check whole PR.
There was a problem hiding this comment.
Thanks! My Bad, fixing it!
|
@JingsongLi Thanks for your review! All reviews addressed. |
Purpose
Supports writing Map<X, Blob> in BlobFormatTable, following #8181
This PR only focus on Blob File Format of Java and Python module, keeping this PR reasonably small.
This PR DO NOT involes:
Key Changes:
Xonly supports some simple non-nested type: String and Int family.Tests
See unit tests