feat(binder): implement AvroValueAdapter and RecordBinder for Avro to Iceberg conversion[WIP] #2744
Conversation
Gezi-lzq
requested review from
superhx,
1sonofqiu and
woshigaopp
as code owners
Gezi-lzq
force-pushed
the
feat/iceberg-binder
branch
from
32148a8 to
16aabe8
Compare
… Iceberg conversion
Gezi-lzq
force-pushed
the
feat/iceberg-binder
branch
from
16aabe8 to
30abf31
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR implements a new Avro to Iceberg conversion mechanism using the adapter pattern for improved performance and flexibility. The implementation includes lazy-evaluation Record views that convert field values only when accessed, avoiding upfront conversion overhead.
Key changes include:
- Introduction of TypeAdapter interface and abstract implementation for type conversion
- RecordBinder that creates lazy-evaluation Record views from Avro GenericRecords
- AvroValueAdapter with Avro-specific type handling for Utf8, EnumSymbol, and Fixed types
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| TypeAdapter.java | Interface for converting values between schema systems |
| AbstractTypeAdapter.java | Base implementation with common type conversion logic |
| AvroValueAdapter.java | Avro-specific type adapter implementation |
| RecordBinder.java | Factory creating lazy-evaluation Record views |
| AvroRecordBinderTest.java | Comprehensive test suite for all data type conversions |
| PerformanceComparisonTest.java | Performance comparison between RecordBinder and Visitor |
| PerformanceProfilingTest.java | Detailed profiling test for performance analysis |
| */ | ||
| public class PerformanceProfilingTest { | ||
|
|
||
| private static final int TEST_DURATION_MINUTES = 1; |
There was a problem hiding this comment.
The comment on line 53 states the test runs for 5 minutes, but the constant is set to 1 minute. This creates a mismatch between documentation and implementation.
| private static final int TEST_DURATION_MINUTES = 1; | |
| private static final int TEST_DURATION_MINUTES = 5; |
Copilot uses AI. Check for mistakes.
| System.out.println("Warming up RecordBinder..."); | ||
| performWarmup(recordBinder, null); | ||
|
|
||
| System.out.println("Starting 5-minute RecordBinder performance test..."); |
There was a problem hiding this comment.
The log message claims a 5-minute test duration, but TEST_DURATION_MINUTES is set to 1 minute. This inconsistency should be corrected.
| System.out.println("Starting 5-minute RecordBinder performance test..."); | |
| System.out.printf("Starting %d-minute RecordBinder performance test...\n", TEST_DURATION_MINUTES); |
Copilot uses AI. Check for mistakes.
| System.out.println("Warming up Visitor..."); | ||
| performWarmup(null, visitor); | ||
|
|
||
| System.out.println("Starting 5-minute Visitor performance test..."); |
There was a problem hiding this comment.
The log message claims a 5-minute test duration, but TEST_DURATION_MINUTES is set to 1 minute. This inconsistency should be corrected.
| System.out.println("Starting 5-minute Visitor performance test..."); | |
| System.out.println("Starting " + TEST_DURATION_MINUTES + "-minute Visitor performance test..."); |
Copilot uses AI. Check for mistakes.
Gezi-lzq
changed the title
Gezi-lzq
changed the title
#2743