Copilot commented on code in PR #12977:
URL: https://github.com/apache/gluten/pull/12977#discussion_r3965969554
##########
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"
Review Comment:
Building filesystem paths via `toURI.getPath` and
`getResource(\"/\").getPath + ...` is brittle (URL-encoding like spaces,
platform-specific path handling, and potential missing drive letters on
Windows). Prefer using `dir.getCanonicalPath` (or `dir.toPath.toString`) for
the temp directory, and resolve the resource via `getResource(...).toURI` and a
proper path conversion rather than string concatenation.
##########
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)
+ // Velox native write doesn't support complex types.
+ val df = spark.read
+ .format("parquet")
+ .load(data_path)
+ .drop("array")
+ .drop("struct")
+ .drop("map")
df.write.mode("append").format("parquet").save(write_path)
+ val parquetDf = spark.read
+ .format("parquet")
+ .load(write_path)
+ checkAnswer(parquetDf, df)
Review Comment:
`checkAnswer(parquetDf, df)` can be flaky because Parquet reads don’t
guarantee row ordering, and `checkAnswer` compares collected rows in-order. To
make the test deterministic, compare sorted results (e.g., order both
DataFrames by a stable set of columns) or use a set-equivalence approach (e.g.,
symmetric `except`/`subtract`).
##########
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"
Review Comment:
New local vals are introduced with snake_case (`write_path`, `data_path`),
which is inconsistent with Scala naming conventions and typical Spark/Scala
style. Rename to `writePath` / `dataPath` to keep the test code consistent and
easier to read.
--
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]
