danepitkin commented on code in PR #44056:
URL: https://github.com/apache/arrow/pull/44056#discussion_r1759072522
##########
java/gandiva/src/main/java/org/apache/arrow/gandiva/evaluator/Filter.java:
##########
@@ -42,7 +42,10 @@ public class Filter {
private final JniWrapper wrapper;
private final long moduleId;
+
+ @SuppressWarnings("UnusedVariable")
Review Comment:
This actually looks unused. It's private, and not used after object
instantiation. Should we remove it?
##########
java/gandiva/src/main/java/org/apache/arrow/gandiva/evaluator/Projector.java:
##########
@@ -45,7 +45,10 @@ public class Projector {
private JniWrapper wrapper;
private final long moduleId;
+
+ @SuppressWarnings("UnusedVariable")
Review Comment:
We can probably delete this, too.
##########
java/flight/flight-sql/src/main/java/org/apache/arrow/flight/sql/FlightSqlClient.java:
##########
@@ -1240,7 +1240,7 @@ public static class PreparedStatement implements
AutoCloseable {
* used in the {@code PreparedStatement} setters.
*/
public void setParameters(final VectorSchemaRoot parameterBindingRoot) {
- if (parameterBindingRoot == this.parameterBindingRoot) {
+ if (parameterBindingRoot.equals(this.parameterBindingRoot)) {
Review Comment:
I would recommend adding a comment alongside warning suppressions for future
readers. It's not always obvious if a Warning suppression is the result of a
temporary bug, laziness from the developers (not you, just in general :P), or
is truly an unhelpful warning.
##########
java/performance/src/main/java/org/apache/arrow/adapter/jdbc/JdbcAdapterBenchmarks.java:
##########
@@ -154,13 +154,13 @@ public static class ConsumeState {
private JdbcConsumer<BitVector> bitConsumer;
- private JdbcToArrowConfig config;
+ // private JdbcToArrowConfig config;
Review Comment:
Should this be deleted?
##########
java/vector/src/main/java/org/apache/arrow/vector/validate/ValidateVectorBufferVisitor.java:
##########
@@ -283,6 +287,7 @@ public Void visit(NullVector vector, Void value) {
}
@Override
+ @SuppressWarnings("VoidUsed")
Review Comment:
You could use `vector.getUnderlyingVector().accept(this, null);` and get rid
of the suppressed warning.
##########
java/vector/src/main/java/org/apache/arrow/vector/validate/ValidateVectorDataVisitor.java:
##########
@@ -202,6 +202,7 @@ public Void visit(NullVector vector, Void value) {
}
@Override
+ @SuppressWarnings("VoidUsed")
Review Comment:
same here. `vector.getUnderlyingVector().accept(this, null);`
##########
java/vector/src/main/java/org/apache/arrow/vector/validate/ValidateVectorVisitor.java:
##########
@@ -314,6 +316,7 @@ public Void visit(NullVector vector, Void value) {
}
@Override
+ @SuppressWarnings("VoidUsed")
Review Comment:
and here
##########
java/memory/pom.xml:
##########
@@ -34,4 +34,29 @@ under the License.
<module>memory-netty-buffer-patch</module>
<module>memory-netty</module>
</modules>
+
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <configuration>
+ <compilerArgs combine.children="override">
Review Comment:
Are we trying to override and disable warnings-as-errors for this module? If
so, it should be `combine.self="override"`.
##########
java/flight/flight-sql/src/main/java/org/apache/arrow/flight/sql/FlightSqlClient.java:
##########
@@ -1240,7 +1240,7 @@ public static class PreparedStatement implements
AutoCloseable {
* used in the {@code PreparedStatement} setters.
*/
public void setParameters(final VectorSchemaRoot parameterBindingRoot) {
- if (parameterBindingRoot == this.parameterBindingRoot) {
+ if (parameterBindingRoot.equals(this.parameterBindingRoot)) {
Review Comment:
For example, we would never want to un-suppress this warning because we
_want_ to compare object references, but for the Flatbuffers module name
warning in an older PR, we want to eventually remove it once fixed.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
