Copilot commented on code in PR #12776:
URL: https://github.com/apache/gluten/pull/12776#discussion_r3988520868
##########
gluten-ut/spark34/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/GlutenParquetColumnIndexSuite.scala:
##########
@@ -16,50 +16,6 @@
*/
package org.apache.spark.sql.execution.datasources.parquet
-import org.apache.spark.sql.{DataFrame, GlutenSQLTestsBaseTrait}
+import org.apache.spark.sql.GlutenSQLTestsBaseTrait
-class GlutenParquetColumnIndexSuite extends ParquetColumnIndexSuite with
GlutenSQLTestsBaseTrait {
- private val actions: Seq[DataFrame => DataFrame] = Seq(
- "_1 = 500",
- "_1 = 500 or _1 = 1500",
- "_1 = 500 or _1 = 501 or _1 = 1500",
- "_1 = 500 or _1 = 501 or _1 = 1000 or _1 = 1500",
- "_1 >= 500 and _1 < 1000",
- "(_1 >= 500 and _1 < 1000) or (_1 >= 1500 and _1 < 1600)"
- ).map(f => (df: DataFrame) => df.filter(f))
-
- testGluten("test reading unaligned pages - test all types") {
- val df = spark
- .range(0, 2000)
- .selectExpr(
- "id as _1",
- "cast(id as short) as _3",
- "cast(id as int) as _4",
- "cast(id as float) as _5",
- "cast(id as double) as _6",
- "cast(id as decimal(20,0)) as _7",
- // We changed 1618161925000 to 1618161925 to avoid reaching the
limitation of Velox:
- // Timepoint is outside of supported year range.
- "cast(cast(1618161925 + id * 60 * 60 * 24 as timestamp) as date) as _9"
- )
- checkUnalignedPages(df)(actions: _*)
- }
-
- testGluten("test reading unaligned pages - test all types (dict encode)") {
- val df = spark
- .range(0, 2000)
- .selectExpr(
- "id as _1",
- "cast(id % 10 as byte) as _2",
- "cast(id % 10 as short) as _3",
- "cast(id % 10 as int) as _4",
- "cast(id % 10 as float) as _5",
- "cast(id % 10 as double) as _6",
- "cast(id % 10 as decimal(20,0)) as _7",
- "cast(id % 2 as boolean) as _8",
- "cast(cast(1618161925 + (id % 10) * 60 * 60 * 24 as timestamp) as
date) as _9",
- "cast(1618161925 + (id % 10) as timestamp) as _10"
- )
- checkUnalignedPages(df)(actions: _*)
- }
-}
+class GlutenParquetColumnIndexSuite extends ParquetColumnIndexSuite with
GlutenSQLTestsBaseTrait {}
Review Comment:
The removed implementation deliberately changed `1618161925000` to
`1618161925` because the original timestamp is outside Velox's supported
timepoint range. Replacing it with an empty subclass restores Spark's original
unaligned-page tests, and the corresponding Velox exclusions are also removed,
so these tests will feed the unsupported timestamp to native Velox. Keep the
backend-specific rewrite or retain the exclusions.
##########
gluten-ut/spark33/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/GlutenParquetFilterSuite.scala:
##########
@@ -0,0 +1,383 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.sql.execution.datasources.parquet
+
+import org.apache.spark.SparkConf
+import org.apache.spark.sql._
+import org.apache.spark.sql.catalyst.dsl.expressions._
+import org.apache.spark.sql.catalyst.expressions._
+import org.apache.spark.sql.catalyst.optimizer.InferFiltersFromConstraints
+import org.apache.spark.sql.catalyst.planning.PhysicalOperation
+import
org.apache.spark.sql.connector.catalog.CatalogV2Implicits.parseColumnPath
+import org.apache.spark.sql.execution.datasources.{DataSourceStrategy,
HadoopFsRelation, LogicalRelation, PushableColumnAndNestedColumn}
+import org.apache.spark.sql.execution.datasources.v2.DataSourceV2ScanRelation
+import org.apache.spark.sql.execution.datasources.v2.parquet.ParquetScan
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.types._
+import org.apache.spark.tags.ExtendedSQLTest
+import org.apache.spark.util.Utils
+
+import org.apache.hadoop.fs.Path
+import org.apache.parquet.filter2.predicate.{FilterApi, FilterPredicate}
+import org.apache.parquet.filter2.predicate.FilterApi._
+import org.apache.parquet.hadoop.{ParquetFileReader, ParquetInputFormat,
ParquetOutputFormat}
+import org.apache.parquet.hadoop.util.HadoopInputFile
+
+abstract class GlutenParquetFilterSuite extends ParquetFilterSuite with
GlutenSQLTestsBaseTrait {
Review Comment:
All tests in this newly added Spark 3.3 wrapper are registered through
`testGluten`, which prefixes their names with `Gluten - `. The existing
BoltTestSettings entries for `SPARK-23852`, `SPARK-17091`, and `Support Parquet
column index` use raw `.exclude(...)` names, so they do not match and cases
marked as rewrite/unsupported will now run. Update those settings to use
`excludeGlutenTest` (or mark inherently unsupported tests with `ignoreGluten`)
before enabling this wrapper.
##########
gluten-ut/spark35/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/GlutenParquetColumnIndexSuite.scala:
##########
@@ -16,50 +16,6 @@
*/
package org.apache.spark.sql.execution.datasources.parquet
-import org.apache.spark.sql.{DataFrame, GlutenSQLTestsBaseTrait}
+import org.apache.spark.sql.GlutenSQLTestsBaseTrait
-class GlutenParquetColumnIndexSuite extends ParquetColumnIndexSuite with
GlutenSQLTestsBaseTrait {
- private val actions: Seq[DataFrame => DataFrame] = Seq(
- "_1 = 500",
- "_1 = 500 or _1 = 1500",
- "_1 = 500 or _1 = 501 or _1 = 1500",
- "_1 = 500 or _1 = 501 or _1 = 1000 or _1 = 1500",
- "_1 >= 500 and _1 < 1000",
- "(_1 >= 500 and _1 < 1000) or (_1 >= 1500 and _1 < 1600)"
- ).map(f => (df: DataFrame) => df.filter(f))
-
- testGluten("test reading unaligned pages - test all types") {
- val df = spark
- .range(0, 2000)
- .selectExpr(
- "id as _1",
- "cast(id as short) as _3",
- "cast(id as int) as _4",
- "cast(id as float) as _5",
- "cast(id as double) as _6",
- "cast(id as decimal(20,0)) as _7",
- // We changed 1618161925000 to 1618161925 to avoid reaching the
limitation of Velox:
- // Timepoint is outside of supported year range.
- "cast(cast(1618161925 + id * 60 * 60 * 24 as timestamp) as date) as _9"
- )
- checkUnalignedPages(df)(actions: _*)
- }
-
- testGluten("test reading unaligned pages - test all types (dict encode)") {
- val df = spark
- .range(0, 2000)
- .selectExpr(
- "id as _1",
- "cast(id % 10 as byte) as _2",
- "cast(id % 10 as short) as _3",
- "cast(id % 10 as int) as _4",
- "cast(id % 10 as float) as _5",
- "cast(id % 10 as double) as _6",
- "cast(id % 10 as decimal(20,0)) as _7",
- "cast(id % 2 as boolean) as _8",
- "cast(cast(1618161925 + (id % 10) * 60 * 60 * 24 as timestamp) as
date) as _9",
- "cast(1618161925 + (id % 10) as timestamp) as _10"
- )
- checkUnalignedPages(df)(actions: _*)
- }
-}
+class GlutenParquetColumnIndexSuite extends ParquetColumnIndexSuite with
GlutenSQLTestsBaseTrait {}
Review Comment:
The removed implementation deliberately changed `1618161925000` to
`1618161925` because the original timestamp is outside Velox's supported
timepoint range. Replacing it with an empty subclass restores Spark's original
unaligned-page tests, and the corresponding Velox exclusions are also removed,
so these tests will feed the unsupported timestamp to native Velox. Keep the
backend-specific rewrite or retain the exclusions.
##########
gluten-ut/spark40/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/GlutenParquetFilterSuite.scala:
##########
@@ -68,44 +59,6 @@ abstract class GlutenParquetFilterSuite extends
ParquetFilterSuite with GlutenSQ
getWorkspaceFilePath("sql", "core", "src", "test", "resources").toString
+ "/" + name)
}
Review Comment:
This leaves the suite with Spark's unmodified `filter pushdown -
date`/timestamp tests. The removed backend-specific date rewrite deliberately
restricted rebase mode to `CORRECTED` because Velox does not support `LEGACY`,
while the corresponding Velox exclusions are also removed in this PR; those
inherited cases will therefore fail rather than add valid coverage. Please
retain the adapted tests or keep the exclusions until the backend semantics are
supported.
##########
gluten-ut/spark34/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/GlutenParquetFilterSuite.scala:
##########
@@ -66,44 +57,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:
This leaves the suite with Spark's unmodified `filter pushdown -
date`/timestamp tests. The removed backend-specific date rewrite deliberately
restricted rebase mode to `CORRECTED` because Velox does not support `LEGACY`,
while the corresponding Velox exclusions are also removed in this PR; those
inherited cases will therefore fail rather than add valid coverage. Please
retain the adapted tests or keep the exclusions until the backend semantics are
supported.
##########
gluten-ut/spark35/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/GlutenParquetFilterSuite.scala:
##########
@@ -66,44 +57,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:
This leaves the suite with Spark's unmodified `filter pushdown -
date`/timestamp tests. The removed backend-specific date rewrite deliberately
restricted rebase mode to `CORRECTED` because Velox does not support `LEGACY`,
while the corresponding Velox exclusions are also removed in this PR; those
inherited cases will therefore fail rather than add valid coverage. Please
retain the adapted tests or keep the exclusions until the backend semantics are
supported.
##########
gluten-ut/spark41/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/GlutenParquetFilterSuite.scala:
##########
@@ -68,44 +59,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:
This leaves the suite with Spark's unmodified `filter pushdown -
date`/timestamp tests. The removed backend-specific date rewrite deliberately
restricted rebase mode to `CORRECTED` because Velox does not support `LEGACY`,
while the corresponding Velox exclusions are also removed in this PR; those
inherited cases will therefore fail rather than add valid coverage. Please
retain the adapted tests or keep the exclusions until the backend semantics are
supported.
--
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]
