Data: Add UUID conversion support for InternalRecordWrapper #14208
Conversation
| import org.apache.iceberg.util.UUIDUtil; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| public class TestInternalRecordWrapper { |
There was a problem hiding this comment.
I noticed there is an abstract test class for the wrapper classes: https://github.com/apache/iceberg/blob/main/data/src/test/java/org/apache/iceberg/RecordWrapperTestBase.java
Do you think we should extend it here?
There was a problem hiding this comment.
Not sure extending it wouldn’t add much value in this case.
| import org.apache.iceberg.util.UUIDUtil; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| public class TestInternalRecordWrapper { |
There was a problem hiding this comment.
Not sure extending it wouldn’t add much value in this case.
|
Hi @pvary , could you take a quick look at this PR? |
|
@xxubai: Could we add a test which covers the original issue with the FanoutWriter? |
xxubai
force-pushed
the
fix-uuid-for-internal-record-wrapper
branch
from
eed1e2a to
12ab952
Compare
Signed-off-by: xuba <[email protected]>
Signed-off-by: xuba <[email protected]>
Thanks @pvary.I added a test to fix this issue by using a |
|
|
||
| @ParameterizedTest | ||
| @ValueSource(strings = {"parquet", "orc"}) | ||
| public void testWriteUuidWithFountWriter(String format) { |
There was a problem hiding this comment.
This one is running without the change.
Any ideas why?
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions. |
|
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
4 participants
In data files, the UUID type is written as a fixed-length 16-byte binary.
Therefore, to correctly convert the UUID type when implementing a custom PartitionedFanoutWriter, it is necessary to add a corresponding converter.
For details, see the related issue: apache/amoro#3797