Copilot commented on code in PR #12776:
URL: https://github.com/apache/gluten/pull/12776#discussion_r3784989219
##########
gluten-ut/spark40/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/GlutenParquetFilterSuite.scala:
##########
@@ -68,44 +68,6 @@ abstract class GlutenParquetFilterSuite extends
ParquetFilterSuite with GlutenSQ
getWorkspaceFilePath("sql", "core", "src", "test", "resources").toString
+ "/" + name)
}
Review Comment:
Removing the "filter pushdown - timestamp" / "filter pushdown - date" tests
leaves several imports in this file unused. The build uses `-Wunused:imports`
with `-Wconf:any:e` (see pom.xml), so these unused imports will fail
compilation.
Please remove the now-unused imports (e.g.,
`LegacyBehaviorPolicy.{CORRECTED, LEGACY}`,
`SQLConf.ParquetOutputTimestampType.INT96`, `java.sql.{Date, Timestamp}`,
`java.time.LocalDate`, `scala.reflect.{ClassTag, TypeTag}`, and the Parquet
`Operators` imports if no longer referenced).
##########
gluten-ut/spark34/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/GlutenParquetFilterSuite.scala:
##########
@@ -66,44 +66,6 @@ abstract class GlutenParquetFilterSuite extends
ParquetFilterSuite with GlutenSQ
getWorkspaceFilePath("sql", "core", "src", "test", "resources").toString
+ "/" + name)
}
- testGluten("filter pushdown - timestamp") {
- Seq(true, false).foreach {
- java8Api =>
- Seq(CORRECTED, LEGACY).foreach {
- rebaseMode =>
- val millisData = Seq(
- "1000-06-14 08:28:53.123",
- "1582-06-15 08:28:53.001",
- "1900-06-16 08:28:53.0",
- "2018-06-17 08:28:53.999")
- // INT96 doesn't support pushdown
- withSQLConf(
- SQLConf.DATETIME_JAVA8API_ENABLED.key -> java8Api.toString,
- SQLConf.PARQUET_INT96_REBASE_MODE_IN_WRITE.key ->
rebaseMode.toString,
- SQLConf.PARQUET_OUTPUT_TIMESTAMP_TYPE.key -> INT96.toString
- ) {
- import testImplicits._
- withTempPath {
- file =>
- millisData
- .map(i => Tuple1(Timestamp.valueOf(i)))
- .toDF
- .write
- .format(dataSourceName)
- .save(file.getCanonicalPath)
- readParquetFile(file.getCanonicalPath) {
- df =>
- val schema = new
SparkToParquetSchemaConverter(conf).convert(df.schema)
- assertResult(None) {
-
createParquetFilters(schema).createFilter(sources.IsNull("_1"))
- }
- }
- }
- }
- }
- }
- }
-
testGluten("SPARK-12218: 'Not' is included in Parquet filter pushdown") {
import testImplicits._
Review Comment:
Removing the "filter pushdown - timestamp" / "filter pushdown - date" tests
leaves several imports in this file unused. The build uses `-Wunused:imports`
with `-Wconf:any:e` (see pom.xml), so these unused imports will fail
compilation.
Please remove the now-unused imports (e.g.,
`LegacyBehaviorPolicy.{CORRECTED, LEGACY}`,
`SQLConf.ParquetOutputTimestampType.INT96`, `java.sql.{Date, Timestamp}`,
`java.time.LocalDate`, `scala.reflect.{ClassTag, TypeTag}`, and the Parquet
`Operators` imports if no longer referenced).
##########
gluten-ut/spark33/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/GlutenParquetFilterSuite.scala:
##########
@@ -67,44 +67,6 @@ abstract class GlutenParquetFilterSuite extends
ParquetFilterSuite with GlutenSQ
getWorkspaceFilePath("sql", "core", "src", "test", "resources").toString
+ "/" + name)
}
- testGluten("filter pushdown - timestamp") {
- Seq(true, false).foreach {
- java8Api =>
- Seq(CORRECTED, LEGACY).foreach {
- rebaseMode =>
- val millisData = Seq(
- "1000-06-14 08:28:53.123",
- "1582-06-15 08:28:53.001",
- "1900-06-16 08:28:53.0",
- "2018-06-17 08:28:53.999")
- // INT96 doesn't support pushdown
- withSQLConf(
- SQLConf.DATETIME_JAVA8API_ENABLED.key -> java8Api.toString,
- SQLConf.PARQUET_INT96_REBASE_MODE_IN_WRITE.key ->
rebaseMode.toString,
- SQLConf.PARQUET_OUTPUT_TIMESTAMP_TYPE.key -> INT96.toString
- ) {
- import testImplicits._
- withTempPath {
- file =>
- millisData
- .map(i => Tuple1(Timestamp.valueOf(i)))
- .toDF
- .write
- .format(dataSourceName)
- .save(file.getCanonicalPath)
- readParquetFile(file.getCanonicalPath) {
- df =>
- val schema = new
SparkToParquetSchemaConverter(conf).convert(df.schema)
- assertResult(None) {
-
createParquetFilters(schema).createFilter(sources.IsNull("_1"))
- }
- }
- }
- }
- }
- }
- }
-
testGluten("SPARK-12218: 'Not' is included in Parquet filter pushdown") {
import testImplicits._
Review Comment:
Removing the "filter pushdown - timestamp" / "filter pushdown - date" tests
leaves several imports in this file unused. The build uses `-Wunused:imports`
with `-Wconf:any:e` (see pom.xml), so these unused imports will fail
compilation.
Please remove the now-unused imports (e.g.,
`LegacyBehaviorPolicy.{CORRECTED, LEGACY}`,
`SQLConf.ParquetOutputTimestampType.INT96`, `java.sql.{Date, Timestamp}`,
`java.time.LocalDate`, `scala.reflect.{ClassTag, TypeTag}`, and the Parquet
`Operators` imports if no longer referenced).
##########
gluten-ut/spark41/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/GlutenParquetFilterSuite.scala:
##########
@@ -68,44 +68,6 @@ abstract class GlutenParquetFilterSuite extends
ParquetFilterSuite with GlutenSQ
getWorkspaceFilePath("sql", "core", "src", "test", "resources").toString
+ "/" + name)
}
- testGluten("filter pushdown - timestamp") {
- Seq(true, false).foreach {
- java8Api =>
- Seq(CORRECTED, LEGACY).foreach {
- rebaseMode =>
- val millisData = Seq(
- "1000-06-14 08:28:53.123",
- "1582-06-15 08:28:53.001",
- "1900-06-16 08:28:53.0",
- "2018-06-17 08:28:53.999")
- // INT96 doesn't support pushdown
- withSQLConf(
- SQLConf.DATETIME_JAVA8API_ENABLED.key -> java8Api.toString,
- SQLConf.PARQUET_INT96_REBASE_MODE_IN_WRITE.key ->
rebaseMode.toString,
- SQLConf.PARQUET_OUTPUT_TIMESTAMP_TYPE.key -> INT96.toString
- ) {
- import testImplicits._
- withTempPath {
- file =>
- millisData
- .map(i => Tuple1(Timestamp.valueOf(i)))
- .toDF
- .write
- .format(dataSourceName)
- .save(file.getCanonicalPath)
- readParquetFile(file.getCanonicalPath) {
- df =>
- val schema = new
SparkToParquetSchemaConverter(conf).convert(df.schema)
- assertResult(None) {
-
createParquetFilters(schema).createFilter(sources.IsNull("_1"))
- }
- }
- }
- }
- }
- }
- }
-
testGluten("SPARK-12218: 'Not' is included in Parquet filter pushdown") {
Review Comment:
Removing the "filter pushdown - timestamp" / "filter pushdown - date" tests
leaves several imports in this file unused. The build uses `-Wunused:imports`
with `-Wconf:any:e` (see pom.xml), so these unused imports will fail
compilation.
Please remove the now-unused imports (e.g.,
`LegacyBehaviorPolicy.{CORRECTED, LEGACY}`,
`SQLConf.ParquetOutputTimestampType.INT96`, `java.sql.{Date, Timestamp}`,
`java.time.LocalDate`, `scala.reflect.{ClassTag, TypeTag}`, and the Parquet
`Operators` imports if no longer referenced).
##########
gluten-ut/spark35/src/test/scala/org/apache/gluten/utils/velox/VeloxTestSettings.scala:
##########
@@ -399,43 +387,43 @@ class VeloxTestSettings extends BackendTestSettings {
.exclude("filter pushdown - StringPredicate")
.exclude("SPARK-38825: in and notIn filters")
enableSuite[GlutenParquetInteroperabilitySuite]
- .exclude("parquet timestamp conversion")
+ .include("parquet timestamp conversion")
Review Comment:
`SuiteSettings.include(...)` switches the suite into *include mode*, so
**only** the listed test(s) will run and all other tests in
`GlutenParquetInteroperabilitySuite` will be skipped (see
`BackendTestSettings.shouldRun`). If the goal is to start running "parquet
timestamp conversion" again, it should be removed from the exclusion list
instead of converting the suite to include mode.
##########
gluten-ut/spark35/src/test/scala/org/apache/gluten/utils/velox/VeloxTestSettings.scala:
##########
@@ -482,16 +470,13 @@ class VeloxTestSettings extends BackendTestSettings {
enableSuite[GlutenParquetV1AggregatePushDownSuite]
enableSuite[GlutenParquetV2AggregatePushDownSuite]
// TODO: Timestamp columns stats will lost if using int64 in parquet
writer.
- .exclude("aggregate push down - different data types")
+ .include("aggregate push down - different data types")
Review Comment:
`SuiteSettings.include(...)` switches the suite into *include mode*, so
**only** the listed test(s) will run and all other tests in
`GlutenParquetV2AggregatePushDownSuite` will be skipped (see
`BackendTestSettings.shouldRun`). This looks like it was intended to
*un-exclude* this test rather than to restrict the whole suite to a single 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
