aokolnychyi commented on a change in pull request #2214:
URL: https://github.com/apache/iceberg/pull/2214#discussion_r599171235
##########
File path: core/src/main/java/org/apache/iceberg/avro/Avro.java
##########
@@ -284,6 +286,11 @@ public DeleteWriteBuilder equalityFieldIds(int...
fieldIds) {
return this;
}
+ public DeleteWriteBuilder withSortOrder(SortOrder newSortOrder) {
Review comment:
Oh, yeah, you are right. I think we should refactor this to make it
clear (independently of this PR).
There are a few points to discuss.
The first one is `FileAppenderFactory`. It was originally created to produce
`FileAppender` instances only, which made sense. Now, it also creates data and
delete writers too, which makes less sense to me. I'd consider deprecating
methods for creating writers in `FileAppenderFactory` and would create
`WriterFactory` instead.
```
WriterFactory {
newDataWriter(...)
newEqualityDeleteWriter(...)
newPositionDeleteWriter(...)
}
```
Second, I'd consider adding a method called `buildWriter` to our Parquet,
Avro, ORC classes that would create and initialize `DataWriter`, making it
consistent with our delete writers. Otherwise, it looks a bit weird.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
