You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "advancedxy (via GitHub)" <gi...@apache.org> on 2024/03/04 16:22:41 UTC

[PR] chore: Fix warnings in both compiler and test environments [arrow-datafusion-comet]

advancedxy opened a new pull request, #164:
URL: https://github.com/apache/arrow-datafusion-comet/pull/164

   ## Which issue does this PR close?
   Closes #.
   
   ## Rationale for this change
   When build comet locally, I noticed various warnings. Some are compiler warnings, some are test environment warnings. 
   Related code should be refactored for better code quality
   
   ## What changes are included in this PR?
   Fixes various warning in the compiler and test environments
   
   ## How are these changes tested?
   Manually verify the output of maven test


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] chore: Fix warnings in both compiler and test environments [arrow-datafusion-comet]

Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao merged PR #164:
URL: https://github.com/apache/arrow-datafusion-comet/pull/164


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] chore: Fix warnings in both compiler and test environments [arrow-datafusion-comet]

Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #164:
URL: https://github.com/apache/arrow-datafusion-comet/pull/164#discussion_r1511435295


##########
pom.xml:
##########
@@ -711,9 +711,9 @@ under the License.
           <artifactId>maven-surefire-plugin</artifactId>
           <version>3.1.0</version>
           <configuration>
-            <systemProperties>
+            <systemPropertyVariables>

Review Comment:
   `systemProperties` is deprecated.



##########
common/src/main/java/org/apache/comet/parquet/BatchReader.java:
##########
@@ -517,6 +517,7 @@ public void close() throws IOException {
     }
   }
 
+  @SuppressWarnings("deprecation")

Review Comment:
   `FileSystem.getAllStatistics` is deprecated.



##########
spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala:
##########
@@ -1472,14 +1472,14 @@ object QueryPlanSerde extends Logging with ShimQueryPlanSerde {
       // With Spark 3.4, CharVarcharCodegenUtils.readSidePadding gets called to pad spaces for char
       // types. Use rpad to achieve the behavior. See https://github.com/apache/spark/pull/38151
       case StaticInvoke(
-            _: Class[CharVarcharCodegenUtils],
+            clz: Class[_],
             _: StringType,
             "readSidePadding",
             arguments,
             _,
             true,
             false,
-            true) if arguments.size == 2 =>
+            true) if clz == classOf[CharVarcharCodegenUtils] && arguments.size == 2 =>

Review Comment:
   type erasure. Should check clz instead.



##########
spark/src/main/scala/org/apache/spark/sql/comet/CometCollectLimitExec.scala:
##########
@@ -29,6 +27,8 @@ import org.apache.spark.sql.execution.{ColumnarToRowExec, SparkPlan, UnaryExecNo
 import org.apache.spark.sql.execution.metric.{SQLMetric, SQLMetrics, SQLShuffleReadMetricsReporter, SQLShuffleWriteMetricsReporter}
 import org.apache.spark.sql.vectorized.ColumnarBatch
 
+import com.google.common.base.Objects

Review Comment:
   It was wrong to use `java.util.Objects`



##########
core/src/lib.rs:
##########
@@ -99,6 +99,7 @@ const LOG_PATTERN: &str = "{d(%y/%m/%d %H:%M:%S)} {l} {f}: {m}{n}";
 // Creates a default log4rs config, which logs to console with `INFO` level.
 fn default_logger_config() -> CometResult<Config> {
     let console_append = ConsoleAppender::builder()
+        .target(Target::Stderr)

Review Comment:
   Logging to stderr, so that surefire will not complain about: `Corrupted channel by directly writing to native stream in forked JVM`.



##########
spark/src/test/scala/org/apache/spark/sql/CometTPCHQuerySuite.scala:
##########
@@ -271,7 +271,7 @@ class CometTPCHQuerySuite extends QueryTest with CometTPCBase with SQLQueryTestH
   }
 
   // TODO: remove once Spark 3.2 & 3.3 is no longer supported
-  private val shouldRegenerateGoldenFiles: Boolean =
+  private def shouldRegenerateGoldenFiles: Boolean =

Review Comment:
   forward reference in L227, which is incorrect in runtime.



##########
spark/src/main/scala/org/apache/comet/CometSparkSessionExtensions.scala:
##########
@@ -115,7 +115,7 @@ class CometSparkSessionExtensions
           // data source V1
           case scanExec @ FileSourceScanExec(
                 HadoopFsRelation(_, partitionSchema, _, _, _: ParquetFileFormat, _),
-                _: Seq[AttributeReference],
+                _: Seq[_],

Review Comment:
   type erasure.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] chore: Fix warnings in both compiler and test environments [arrow-datafusion-comet]

Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #164:
URL: https://github.com/apache/arrow-datafusion-comet/pull/164#issuecomment-1977068899

   cc @sunchao @viirya 


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] chore: Fix warnings in both compiler and test environments [arrow-datafusion-comet]

Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao commented on PR #164:
URL: https://github.com/apache/arrow-datafusion-comet/pull/164#issuecomment-1977126076

   Merged, thanks


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org