You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/08/16 17:44:08 UTC
[GitHub] [iceberg] kbendick opened a new pull request, #5550: Spark - Add missing override in Spark 3.1 JMH
kbendick opened a new pull request, #5550:
URL: https://github.com/apache/iceberg/pull/5550
Error prone is complaining of a missing override in Spark 3.1 JMH.
It's a little strange because it's an `abstract` method, but here's the warning.
```
$ ./gradlew -DsparkVersions=3.1 :iceberg-spark:iceberg-spark-3.1_2.12:compileJmhJava
/Users/kylebendickson/repos/iceberg/spark/v3.1/spark/src/jmh/java/org/apache/iceberg/spark/source/WritersBenchmark.java:82: warning: [MissingOverride] fileFormat overrides method in IcebergSourceBenchmark; expected @Override
protected abstract FileFormat fileFormat();
```
I did not see this error for other Spark versions
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #5550: Spark - Remove Redundant Redeclaration of Abstract Method in JMH WritersBenchmark Class
Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #5550:
URL: https://github.com/apache/iceberg/pull/5550#issuecomment-1217169436
Actually, attempting to run the method without the function redeclared makes it not usable:
See the output from running:
```
./gradlew -DsparkVersions=2.4 :iceberg-spark:iceberg-spark-2.4:jmh \
-PjmhIncludeRegex=AvroWritersBenchmark \
-PjmhOutputPath=benchmark/avro-writers-benchmark-result.txt
....
> Task :iceberg-spark:iceberg-spark-2.4:compileTestJava
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
> Task :iceberg-spark:iceberg-spark-2.4:compileJmhJava FAILED
/Users/kylebendickson/repos/iceberg/spark/v2.4/spark/src/jmh/java/org/apache/iceberg/spark/source/WritersBenchmark.java:130: error: cannot find symbol
.dataFileFormat(fileFormat())
^
symbol: method fileFormat()
location: class WritersBenchmark
/Users/kylebendickson/repos/iceberg/spark/v2.4/spark/src/jmh/java/org/apache/iceberg/spark/source/WritersBenchmark.java:162: error: cannot find symbol
unpartitionedSpec, fileFormat(), appenders, fileFactory, io, TARGET_FILE_SIZE_IN_BYTES);
^
symbol: method fileFormat()
location: class WritersBenchmark
/Users/kylebendickson/repos/iceberg/spark/v2.4/spark/src/jmh/java/org/apache/iceberg/spark/source/WritersBenchmark.java:181: error: cannot find symbol
.dataFileFormat(fileFormat())
^
symbol: method fileFormat()
location: class WritersBenchmark
/Users/kylebendickson/repos/iceberg/spark/v2.4/spark/src/jmh/java/org/apache/iceberg/spark/source/WritersBenchmark.java:219: error: cannot find symbol
fileFormat(),
^
symbol: method fileFormat()
location: class WritersBenchmark
/Users/kylebendickson/repos/iceberg/spark/v2.4/spark/src/jmh/java/org/apache/iceberg/spark/source/WritersBenchmark.java:244: error: cannot find symbol
.dataFileFormat(fileFormat())
^
symbol: method fileFormat()
location: class WritersBenchmark
/Users/kylebendickson/repos/iceberg/spark/v2.4/spark/src/jmh/java/org/apache/iceberg/spark/source/WritersBenchmark.java:282: error: cannot find symbol
fileFormat(),
^
symbol: method fileFormat()
location: class WritersBenchmark
/Users/kylebendickson/repos/iceberg/spark/v2.4/spark/src/jmh/java/org/apache/iceberg/spark/source/WritersBenchmark.java:310: error: cannot find symbol
.dataFileFormat(fileFormat())
^
symbol: method fileFormat()
location: class WritersBenchmark
/Users/kylebendickson/repos/iceberg/spark/v2.4/spark/src/jmh/java/org/apache/iceberg/spark/source/WritersBenchmark.java:341: error: cannot find symbol
SparkFileWriterFactory.builderFor(table()).dataFileFormat(fileFormat()).build();
^
symbol: method fileFormat()
location: class WritersBenchmark
/Users/kylebendickson/repos/iceberg/spark/v2.4/spark/src/jmh/java/org/apache/iceberg/spark/source/WritersBenchmark.java:361: error: cannot find symbol
return OutputFileFactory.builderFor(table(), 1, 1).format(fileFormat()).build();
^
symbol: method fileFormat()
location: class WritersBenchmark
/Users/kylebendickson/repos/iceberg/spark/v2.4/spark/src/jmh/java/org/apache/iceberg/spark/source/avro/AvroWritersBenchmark.java:36: error: method does not override or implement a method from a supertype
@Override
^
/Users/kylebendickson/repos/iceberg/spark/v2.4/spark/src/jmh/java/org/apache/iceberg/spark/source/parquet/ParquetWritersBenchmark.java:35: error: method does not override or implement a method from a supertype
@Override
^
11 errors
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':iceberg-spark:iceberg-spark-2.4:compileJmhJava'.
> Compilation failed; see the compiler error output for details.
* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.
* Get more help at https://help.gradle.org
Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.
You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.
See https://docs.gradle.org/7.4.2/userguide/command_line_interface.html#sec:command_line_warnings
BUILD FAILED in 7s
35 actionable tasks: 12 executed, 23 up-to-date
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a diff in pull request #5550: Spark - Add missing override in Spark 3.1 JMH
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5550:
URL: https://github.com/apache/iceberg/pull/5550#discussion_r947239622
##########
spark/v3.1/spark/src/jmh/java/org/apache/iceberg/spark/source/WritersBenchmark.java:
##########
@@ -79,6 +79,7 @@ public abstract class WritersBenchmark extends IcebergSourceBenchmark {
private PartitionSpec unpartitionedSpec;
private PartitionSpec partitionedSpec;
+ @Override
protected abstract FileFormat fileFormat();
Review Comment:
Ok. I’ll try removing all of them as it is very strange to have an override for an abstract method and still be abstract.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a diff in pull request #5550: Spark - Remove Redundant Redeclaration of Abstract Method in JMH WritersBenchmark Class
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5550:
URL: https://github.com/apache/iceberg/pull/5550#discussion_r947250459
##########
spark/v3.1/spark/src/jmh/java/org/apache/iceberg/spark/source/WritersBenchmark.java:
##########
@@ -79,6 +79,7 @@ public abstract class WritersBenchmark extends IcebergSourceBenchmark {
private PartitionSpec unpartitionedSpec;
private PartitionSpec partitionedSpec;
+ @Override
protected abstract FileFormat fileFormat();
Review Comment:
Updated to remove the redeclaration instead. Thanks @aokolnychyi!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #5550: Spark - Add missing override in Spark 3.1 JMH
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #5550:
URL: https://github.com/apache/iceberg/pull/5550#discussion_r947238231
##########
spark/v3.1/spark/src/jmh/java/org/apache/iceberg/spark/source/WritersBenchmark.java:
##########
@@ -79,6 +79,7 @@ public abstract class WritersBenchmark extends IcebergSourceBenchmark {
private PartitionSpec unpartitionedSpec;
private PartitionSpec partitionedSpec;
+ @Override
protected abstract FileFormat fileFormat();
Review Comment:
We can probably do that for all Spark versions. It is a bit awkward to have an abstract method that overrides a method in the parent class.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a diff in pull request #5550: Spark - Add missing override in Spark 3.1 JMH
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5550:
URL: https://github.com/apache/iceberg/pull/5550#discussion_r947238730
##########
spark/v3.1/spark/src/jmh/java/org/apache/iceberg/spark/source/WritersBenchmark.java:
##########
@@ -79,6 +79,7 @@ public abstract class WritersBenchmark extends IcebergSourceBenchmark {
private PartitionSpec unpartitionedSpec;
private PartitionSpec partitionedSpec;
+ @Override
protected abstract FileFormat fileFormat();
Review Comment:
It is odd to declare an Override of an abstract method and keep it abstract.
This matches the same file in Spark 3.2 code, so I prefer this for consistency reasons.
But I don’t have a strong preference. I could update this one and then leave the other versions alone one or update all of them to remove it (since it’s in the parent class).
##########
spark/v3.1/spark/src/jmh/java/org/apache/iceberg/spark/source/WritersBenchmark.java:
##########
@@ -79,6 +79,7 @@ public abstract class WritersBenchmark extends IcebergSourceBenchmark {
private PartitionSpec unpartitionedSpec;
private PartitionSpec partitionedSpec;
+ @Override
protected abstract FileFormat fileFormat();
Review Comment:
It _is_ definitely odd to declare an Override of an abstract method and keep it abstract.
This matches the same file in Spark 3.2 code, so I prefer this for consistency reasons.
But I don’t have a strong preference. I could update this one and then leave the other versions alone one or update all of them to remove it (since it’s in the parent class).
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #5550: Spark - Add missing override in Spark 3.1 JMH
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #5550:
URL: https://github.com/apache/iceberg/pull/5550#discussion_r947236355
##########
spark/v3.1/spark/src/jmh/java/org/apache/iceberg/spark/source/WritersBenchmark.java:
##########
@@ -79,6 +79,7 @@ public abstract class WritersBenchmark extends IcebergSourceBenchmark {
private PartitionSpec unpartitionedSpec;
private PartitionSpec partitionedSpec;
+ @Override
protected abstract FileFormat fileFormat();
Review Comment:
What about just removing this method? We already have one in the parent class.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a diff in pull request #5550: Spark - Remove Redundant Redeclaration of Abstract Method in JMH WritersBenchmark Class
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5550:
URL: https://github.com/apache/iceberg/pull/5550#discussion_r947256212
##########
spark/v3.1/spark/src/jmh/java/org/apache/iceberg/spark/source/WritersBenchmark.java:
##########
@@ -79,6 +79,7 @@ public abstract class WritersBenchmark extends IcebergSourceBenchmark {
private PartitionSpec unpartitionedSpec;
private PartitionSpec partitionedSpec;
+ @Override
protected abstract FileFormat fileFormat();
Review Comment:
Trying that, I got errors running it (see the comment on the main thread).
So I've added back in the overridden abstract method.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a diff in pull request #5550: Spark - Add Missing Override for fileFormat function in JMH WriterBenchmark Suite
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5550:
URL: https://github.com/apache/iceberg/pull/5550#discussion_r947273270
##########
spark/v3.1/spark/src/jmh/java/org/apache/iceberg/spark/source/WritersBenchmark.java:
##########
@@ -79,6 +79,7 @@ public abstract class WritersBenchmark extends IcebergSourceBenchmark {
private PartitionSpec unpartitionedSpec;
private PartitionSpec partitionedSpec;
+ @Override
protected abstract FileFormat fileFormat();
Review Comment:
For spark versions 3.0 and lower, the function is _not_ declared in the base class `IcebergSourceBenchmark`. So I added it there so that all of the versions would be consistent.
I don't _love_ this approach, and I'm happy to try removing it from there and simply making it part of the `WritersBenchmark` instead. But at least this is consistent and works.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #5550: Spark - Add Missing Override for fileFormat function in JMH WriterBenchmark Suite
Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #5550:
URL: https://github.com/apache/iceberg/pull/5550#issuecomment-1217194179
Ok. So _not_ having the redeclaration in the subclasses doesn't work if it's declared in `IcebergSourceBenchmark`.
It _is_ declared in the JMH base class in Spark 3.3 and 3.2 and 3.1.
So I've added it to the highest level abstract base class, `IcebergSourceBenchmark`, for Spark 2.4 and 3.0 as well.
I don't _love_ this fix, but at least it's consistent.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org