You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "Copilot (via GitHub)" <gi...@apache.org> on 2026/10/19 21:11:57 UTC
Re: [PR] [MINOR][VL] Configure Gluten for all Delta Spark test contexts [gluten]
Copilot commented on code in PR #13131:
URL: https://github.com/apache/gluten/pull/13131#discussion_r4318917398
##########
.github/workflows/util/delta-spark-ut/apply-delta-test-patches.sh:
##########
@@ -82,6 +82,34 @@ cherry_pick_delta_fix 46bd45d57eadd7e528002a0ae7bd36ce5a456eca "#7104 (ScanRepor
cherry_pick_delta_fix 959e00e15f41f56afc1c9bb95d160c55c6dc7068 "#7105 (9 more test suites)"
echo "::endgroup::"
+echo "::group::Using Spark's writer for the Hadoop-only mock S3 fixture"
+# This test maps s3: paths to local files through a Hadoop FileSystem mock.
+# Velox's native writer cannot use that mock. Only prepare the fixture with
+# Spark's writer; keep Gluten enabled for the conversion and path-scheme check.
+if ! sed 's/^__BLANK_CONTEXT__$/ /' <<'PATCH' | git -C "$DELTA_DIR" apply -
+diff --git a/spark/src/test/scala/org/apache/spark/sql/delta/ConvertToDeltaSuiteBase.scala b/spark/src/test/scala/org/apache/spark/sql/delta/ConvertToDeltaSuiteBase.scala
Review Comment:
The `sed` substitution replaces `__BLANK_CONTEXT__` with a single space, which inserts a whitespace-only line into the patched Scala source. This can create trailing-whitespace noise and may trip whitespace-sensitive linters. Prefer replacing with an empty string (true blank line) or avoid the placeholder entirely (e.g., keep an actual blank line in the heredoc).
##########
.github/workflows/util/delta-spark-ut/apply-delta-test-patches.sh:
##########
@@ -82,6 +82,34 @@ cherry_pick_delta_fix 46bd45d57eadd7e528002a0ae7bd36ce5a456eca "#7104 (ScanRepor
cherry_pick_delta_fix 959e00e15f41f56afc1c9bb95d160c55c6dc7068 "#7105 (9 more test suites)"
echo "::endgroup::"
+echo "::group::Using Spark's writer for the Hadoop-only mock S3 fixture"
+# This test maps s3: paths to local files through a Hadoop FileSystem mock.
+# Velox's native writer cannot use that mock. Only prepare the fixture with
+# Spark's writer; keep Gluten enabled for the conversion and path-scheme check.
+if ! sed 's/^__BLANK_CONTEXT__$/ /' <<'PATCH' | git -C "$DELTA_DIR" apply -
+diff --git a/spark/src/test/scala/org/apache/spark/sql/delta/ConvertToDeltaSuiteBase.scala b/spark/src/test/scala/org/apache/spark/sql/delta/ConvertToDeltaSuiteBase.scala
+--- a/spark/src/test/scala/org/apache/spark/sql/delta/ConvertToDeltaSuiteBase.scala
++++ b/spark/src/test/scala/org/apache/spark/sql/delta/ConvertToDeltaSuiteBase.scala
+@@ -1322,7 +1322,9 @@
+ withTempDir { dir =>
+ withTable("externalTable") {
+ withSQLConf(("fs.s3.impl", classOf[S3LikeLocalFileSystem].getCanonicalName)) {
+- sql(s"CREATE TABLE externalTable USING parquet LOCATION 's3://$dir' AS SELECT 1")
++ withSQLConf("spark.gluten.sql.native.writer.enabled" -> "false") {
++ sql(s"CREATE TABLE externalTable USING parquet LOCATION 's3://$dir' AS SELECT 1")
++ }
+__BLANK_CONTEXT__
+ // Ideally we would test a successful conversion with a remote filesystem, but there's
+ // no good way to set one up in unit tests. So instead we delete the data, and let the
+PATCH
+then
+ echo "ERROR: mock S3 fixture patch did not apply." >&2
+ echo "The patch expects the Delta v4.2.0 path-scheme fixture;" \
+ "ref '${DELTA_REF}' must remain source-compatible." >&2
Review Comment:
This error message hard-codes `Delta v4.2.0`, but the workflow supports running against different Delta refs/tags. This can be misleading when the patch fails on newer/older versions. Consider rewording to describe the expected code pattern/file (e.g., the specific `ConvertToDeltaSuiteBase.scala` fixture block) without tying it to a specific Delta version.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org