Flink: Migrate Flink TableSchema for IcebergSource #13072
Conversation
liamzwbao
force-pushed
the
issue-13054-tableschema-deprecations-iceberg-source
branch
from
c73881f to
e0f7eea
Compare
|
Hi @ajantha-bhat @nastra. PTAL, thank you! |
liamzwbao
changed the title
| columns.add( | ||
| Column.physical(field.getName(), TypeConversions.fromLogicalToDataType(field.getType()))); | ||
| } | ||
| return ResolvedSchema.of(columns); |
There was a problem hiding this comment.
nit: newline: https://iceberg.apache.org/contribute/#block-spacing
|
I remember previous attempts to remove TableSchema from Flink are failed. Could you please check if we were able to backport these changes to Flink 1.19/1.20? Thanks for working on this! |
| */ | ||
| @Deprecated | ||
| public static Schema convert(Schema baseSchema, TableSchema flinkSchema) { |
There was a problem hiding this comment.
Should we get rid of this method, at least for 2.0?
There was a problem hiding this comment.
I would try to keep the Flink 2.0 and 1.19/1.18 code as similar as possible, so we will not have problems with the backports between the different branches. So if this PR is working for 1.19/1.18, then I'm fine with deprecating, and later removing methods, but if not, then I would keep them until the all the Flink versions not supporting them is deprecated.
WDYT?
| */ | ||
| @Deprecated | ||
| public static TableSchema toSchema(RowType rowType) { |
There was a problem hiding this comment.
Should we get rid of this method? AFAIK it's only used in tests.
There was a problem hiding this comment.
Every public method should be handled as an API, unless it is marked with @Internal annotation.
Summary
This is part of #13054.
To deprecate
FlinkSchemaUtil.convert(TableSchema), we also need to deprecate its dependent methods and remove the use ofTableSchema. This PR contributes to that goal by removingTableSchemausage fromIcebergSource.