-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Core: Deprecate Avro.ReadBuilder.createReaderFunc, ProjectionDatumReader #17853
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,11 @@ | |
| import org.apache.iceberg.mapping.NameMapping; | ||
| import org.apache.iceberg.types.TypeUtil; | ||
|
|
||
| /** | ||
| * @deprecated since 1.12.0, will be removed in 1.13.0; use {@link | ||
| * org.apache.iceberg.data.avro.PlannedDataReader} instead. | ||
| */ | ||
| @Deprecated | ||
| public class ProjectionDatumReader<D> implements DatumReader<D>, SupportsRowPosition { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a public class. Wouldn't it be better to deprecate it first? https://iceberg.apache.org/contribute/#minor-version-deprecations-required
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds right to me, but I don't have a strong opinion about it. Let's wait to hear what other community members prefer. |
||
| private final Function<Schema, DatumReader<?>> getReader; | ||
| private final org.apache.iceberg.Schema expectedSchema; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reads like a drop-in replacement but I think new function takes different parameters, as iceberg schema instead of Avro schema, and it rely on caller to call
setSchema(avroSchema)first.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, a user that isn't satisfied with
GenericAvroReaderandPlannedDataReadershould convert the passed read schema first:As far as I understand, calling
setSchema(avroFileSchema)is not required because Avro'sDataFileReaderwill call it automatically.