Copilot commented on code in PR #12977:
URL: https://github.com/apache/gluten/pull/12977#discussion_r3967713668
##########
backends-velox/src/test/scala/org/apache/gluten/execution/VeloxOrcDataTypeValidationSuite.scala:
##########
@@ -465,16 +465,25 @@ class VeloxOrcDataTypeValidationSuite extends
VeloxWholeStageTransformerSuite {
}
}
- ignore("Velox Parquet Write") {
+ test("Velox Parquet Write") {
withSQLConf((GlutenConfig.NATIVE_WRITER_ENABLED.key, "true")) {
withTempDir {
dir =>
- val write_path = dir.toURI.getPath
- val data_path = getClass.getResource("/").getPath +
"/data-type-validation-data/type1"
- val df = spark.read.format("parquet").load(data_path)
- df.write.mode("append").format("parquet").save(write_path)
+ val writePath = dir.toURI.getPath
+ val dataPath = getClass.getResource("/").getPath +
"/data-type-validation-data/type1"
+ // Velox native write doesn't support Complex type.
+ val df = spark.read
+ .format("parquet")
+ .load(dataPath)
+ .drop("array")
+ .drop("struct")
+ .drop("map")
Review Comment:
These consecutive `.drop` calls can be consolidated into a single
`.drop("array", "struct", "map")` (or a shared Seq of column names) to reduce
repetition and make it easier to update the list of unsupported columns in one
place.
##########
backends-velox/src/test/scala/org/apache/gluten/execution/VeloxOrcDataTypeValidationSuite.scala:
##########
@@ -465,16 +465,25 @@ class VeloxOrcDataTypeValidationSuite extends
VeloxWholeStageTransformerSuite {
}
}
- ignore("Velox Parquet Write") {
+ test("Velox Parquet Write") {
withSQLConf((GlutenConfig.NATIVE_WRITER_ENABLED.key, "true")) {
withTempDir {
dir =>
- val write_path = dir.toURI.getPath
- val data_path = getClass.getResource("/").getPath +
"/data-type-validation-data/type1"
- val df = spark.read.format("parquet").load(data_path)
- df.write.mode("append").format("parquet").save(write_path)
+ val writePath = dir.toURI.getPath
+ val dataPath = getClass.getResource("/").getPath +
"/data-type-validation-data/type1"
Review Comment:
`toURI.getPath` and `getResource(...).getPath` can yield URL-encoded paths
(e.g., spaces become `%20`), which can break Spark read/write path resolution
on some environments. Prefer filesystem paths via
`dir.getCanonicalPath`/`dir.getAbsolutePath`, and build the resource path using
`getResource(...).toURI` (or similar) to avoid encoding issues.
##########
backends-velox/src/test/scala/org/apache/gluten/execution/VeloxOrcDataTypeValidationSuite.scala:
##########
@@ -465,16 +465,25 @@ class VeloxOrcDataTypeValidationSuite extends
VeloxWholeStageTransformerSuite {
}
}
- ignore("Velox Parquet Write") {
+ test("Velox Parquet Write") {
withSQLConf((GlutenConfig.NATIVE_WRITER_ENABLED.key, "true")) {
withTempDir {
dir =>
- val write_path = dir.toURI.getPath
- val data_path = getClass.getResource("/").getPath +
"/data-type-validation-data/type1"
- val df = spark.read.format("parquet").load(data_path)
- df.write.mode("append").format("parquet").save(write_path)
+ val writePath = dir.toURI.getPath
+ val dataPath = getClass.getResource("/").getPath +
"/data-type-validation-data/type1"
+ // Velox native write doesn't support Complex type.
Review Comment:
The comment is a bit ambiguous: consider clarifying that the native writer
does not support *complex types* in this test dataset and that the columns
being dropped are `array`, `struct`, and `map` (so future readers understand
why those specific columns are removed).
--
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]
