rdblue commented on a change in pull request #2336:
URL: https://github.com/apache/iceberg/pull/2336#discussion_r595605786
##########
File path:
parquet/src/main/java/org/apache/iceberg/parquet/ParquetValueReaders.java
##########
@@ -696,8 +738,20 @@ public final T read(T reuse) {
I intermediate = newStructData(reuse);
for (int i = 0; i < readers.length; i += 1) {
- set(intermediate, i, readers[i].read(get(intermediate, i)));
- // setters[i].set(intermediate, i, get(intermediate, i));
+ final ParquetValueReader<?> reader = readers[i];
+ if (reader instanceof ParquetValueReader.OfBoolean) {
+ setBoolean(intermediate, i, ((OfBoolean) reader).readBoolean());
+ } else if (reader instanceof ParquetValueReader.OfInt) {
+ setInteger(intermediate, i, ((OfInt) reader).readInteger());
+ } else if (reader instanceof ParquetValueReader.OfLong) {
+ setLong(intermediate, i, ((OfLong) reader).readLong());
+ } else if (reader instanceof ParquetValueReader.OfFloat) {
+ setFloat(intermediate, i, ((OfFloat) reader).readFloat());
+ } else if (reader instanceof ParquetValueReader.OfDouble) {
+ setDouble(intermediate, i, ((OfDouble) reader).readDouble());
+ } else {
+ set(intermediate, i, reader.read(get(intermediate, i)));
+ }
Review comment:
I think it would be better to continue using the `setters` produced by
`newSetter` here because this logic will test each reader several times. I
think that avoiding `if` statements by using a `Setter` implementation will be
faster in the long run because it will incur invocation cost but not have near
as many branches.
Feel free to run the benchmarks and find which one is better, though. I'd be
interested in testing the `setter` approach (uncomment the previous line 700)
against this.
----------------------------------------------------------------
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]
