kbendick commented on a change in pull request #1572:
URL: https://github.com/apache/iceberg/pull/1572#discussion_r502841925
##########
File path:
spark/src/main/java/org/apache/iceberg/spark/data/SparkParquetReaders.java
##########
@@ -655,6 +653,11 @@ public int numElements() {
return numElements;
}
+ @Override
+ public int getNumElements() {
Review comment:
It looks like the function `numElements` is still there, just above this
one
##########
File path:
spark/src/main/java/org/apache/iceberg/spark/data/SparkParquetReaders.java
##########
@@ -655,6 +653,11 @@ public int numElements() {
return numElements;
}
+ @Override
+ public int getNumElements() {
Review comment:
It looks like the function `numElements` is still there (it's part of
the catalyst abstract class), in the lines just above these ones.
But unless there's something going on with Spark, I agree with @rdblue.
I looked to see if `getNumElements` were needed on the Flink side, as Flink
highly favors POJOs for performance reasons and then for style reasons they
also tend to use POJO-y like names, but the Flink `ArrayData` interface uses
`size` instead, so keeping the iceberg ReusableArrayData interface to use
`numElements` seems like it would kill two birds with one stone by also
fulfilling the catalyst contract.
----------------------------------------------------------------
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]
