rdblue commented on a change in pull request #1790:
URL: https://github.com/apache/iceberg/pull/1790#discussion_r533746885
##########
File path: data/src/main/java/org/apache/iceberg/data/orc/GenericOrcWriters.java
##########
@@ -77,12 +80,12 @@ private GenericOrcWriters() {
return LongWriter.INSTANCE;
}
- public static OrcValueWriter<Float> floats() {
- return FloatWriter.INSTANCE;
+ public static OrcValueWriter<Float> floats(Integer id) {
Review comment:
The id is required, right? If so, then I think it could be `int`
instead. It's an `int` in the constructor that gets called.
This could add a precondition to check that the id is non-null, but I think
it would be better to do that before calling this method because the caller
would probably know the field name rather than just ID. Using the field name
would produce a better error message.
----------------------------------------------------------------
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]
