Copilot commented on code in PR #13131:
URL: https://github.com/apache/gluten/pull/13131#discussion_r4319473560


##########
.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__

Review Comment:
   The placeholder replacement uses a single space (`sed ... '$/ /'`), which 
turns `__BLANK_CONTEXT__` into a whitespace-only line in the patch body. Git 
apply context matching is sensitive to exact whitespace, so this can cause the 
patch to fail to apply if the upstream file has a truly empty blank line (most 
likely). Replace the placeholder with an actually empty line (i.e., delete the 
marker line entirely) to make the patch robust.



##########
.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") {

Review Comment:
   This patch disables `spark.gluten.sql.native.writer.enabled`, but the 
JVM-wide defaults being introduced enable Delta native write via 
`spark.gluten.sql.columnar.backend.velox.delta.enableNativeWrite=true` (in 
`java-test-args.sh`). If `spark.gluten.sql.native.writer.enabled` does not 
control the Delta native writer path, the mock-S3 fixture will still be written 
using the native writer and the test will keep failing (especially since the 
known-failure baseline entries for this test were removed). Prefer overriding 
the same config key that is enabled globally (or whatever the Delta writer 
actually honors) within this `withSQLConf` scope.



-- 
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]

Reply via email to