felipepessoto commented on code in PR #13129:
URL: https://github.com/apache/gluten/pull/13129#discussion_r4286003909
##########
.github/workflows/util/delta-spark-ut/apply-delta-test-patches.sh:
##########
@@ -82,43 +83,31 @@ cherry_pick_delta_fix
46bd45d57eadd7e528002a0ae7bd36ce5a456eca "#7104 (ScanRepor
cherry_pick_delta_fix 959e00e15f41f56afc1c9bb95d160c55c6dc7068 "#7105 (9 more
test suites)"
echo "::endgroup::"
-echo "::group::Capping DeltaParquetFileFormat fixture row groups by row count"
-# DeltaParquetFileFormatSuite generates one 20,000-row Parquet file and sets a
-# 50 KiB block size to ensure that it contains multiple row groups. Velox sizes
-# row groups by buffered bytes after writing each input batch. Because this
-# fixture arrives in one batch, lowering the byte threshold cannot split it.
-# Scope Gluten's native row-count limit around the fixture write so Arrow
splits
-# the 20,000 rows deterministically while keeping the native write path
enabled.
+echo "::group::Applying temporary Delta test patches"
+# Keep local patches after the cherry-picks, which require an unmodified
worktree
+# for the files they touch. Patch paths are relative to this script, not Delta.
+for patch in \
+ cdf-pushed-filter-assertions.patch \
+ parquet-fixture-row-groups.patch \
+ dv-predicate-pushdown-row-groups.patch \
+ dv-2b-row-fail-fast.patch; do
+ echo "Applying $patch"
+ if ! git -C "$DELTA_DIR" apply "$PATCH_DIR/$patch"; then
+ echo "ERROR: Delta test patch '$patch' did not apply." >&2
+ echo "The patch expects Delta v4.2.0 test sources;" \
+ "ref '${DELTA_REF}' must remain source-compatible." >&2
Review Comment:
The message already includes both: `The patch expects Delta v4.2.0 test
sources; ref '${DELTA_REF}' must remain source-compatible.` Here `v4.2.0`
identifies the source version the patches were written against, while
`${DELTA_REF}` reports the actual requested ref. Keeping both helps explain
compatibility failures, so no change is needed.
##########
.github/workflows/util/delta-spark-ut/patches/cdf-pushed-filter-assertions.patch:
##########
@@ -0,0 +1,46 @@
+Subject: [PATCH] Match Delta CDF pushed-filter assertions to Gluten
+
+Gluten's file scans omit Spark's handled-filter '*' marker
+(apache/gluten#12753). They also quote special-character column names,
+unlike Delta's vanilla CDF scan.
+
+Adjust only these three shared assertions, covering 13 concrete test cases;
+keep their predicate, column-pruning, and result checks intact. Remove this
+workaround when #12753 is fixed or Delta no longer requires the starred form.
+
+diff --git
a/spark/src/test/scala/org/apache/spark/sql/delta/DeltaCDCSuite.scala
b/spark/src/test/scala/org/apache/spark/sql/delta/DeltaCDCSuite.scala
+--- a/spark/src/test/scala/org/apache/spark/sql/delta/DeltaCDCSuite.scala
++++ b/spark/src/test/scala/org/apache/spark/sql/delta/DeltaCDCSuite.scala
+@@ -1045,7 +1045,7 @@ class DeltaCDCScalaSuite extends DeltaCDCSuiteBase {
+ .withColumn("_change_type", lit("insert")))
+ }
+ assert(plans.map(_.executedPlan).toString
+- .contains("PushedFilters: [*IsNotNull(id), *LessThan(id,5)]"))
++ .contains("PushedFilters: [IsNotNull(id), LessThan(id,5)]"))
+ }
+ }
+
+diff --git
a/spark/src/test/scala/org/apache/spark/sql/delta/DeltaCDCSQLSuite.scala
b/spark/src/test/scala/org/apache/spark/sql/delta/DeltaCDCSQLSuite.scala
+--- a/spark/src/test/scala/org/apache/spark/sql/delta/DeltaCDCSQLSuite.scala
++++ b/spark/src/test/scala/org/apache/spark/sql/delta/DeltaCDCSQLSuite.scala
+@@ -145,7 +145,7 @@ class DeltaCDCSQLSuite extends DeltaCDCSuiteBase with
DeltaColumnMappingTestUtils {
+ .withColumn("_change_type", lit("insert")))
+ }
+ assert(plans.map(_.executedPlan).toString
+- .contains("PushedFilters: [*IsNotNull(id), *LessThan(id,5)]"))
++ .contains("PushedFilters: [IsNotNull(id), LessThan(id,5)]"))
+ }
+ }
+
+diff --git
a/spark/src/test/scala/org/apache/spark/sql/delta/DeltaCDCColumnMappingSuite.scala
b/spark/src/test/scala/org/apache/spark/sql/delta/DeltaCDCColumnMappingSuite.scala
+---
a/spark/src/test/scala/org/apache/spark/sql/delta/DeltaCDCColumnMappingSuite.scala
++++
b/spark/src/test/scala/org/apache/spark/sql/delta/DeltaCDCColumnMappingSuite.scala
+@@ -668,7 +668,7 @@ trait DeltaCDCColumnMappingSuiteBase extends
DeltaColumnMappingSelectedTestMixin {
+ .withColumn("_change_type", lit("insert")))
+ }
+ assert(plans.map(_.executedPlan).toString
+- .contains("PushedFilters: [*IsNotNull(id with space), *LessThan(id
with space,5)]"))
++ .contains("PushedFilters: [IsNotNull(`id with space`), LessThan(`id
with space`,5)]"))
Review Comment:
The formatting sensitivity is intentional for this temporary workaround. We
preserve Delta's existing assertion structure and change only the
handled-filter markers and special-character quoting to match Gluten's current
output. Looser matching or normalization would broaden the patch and could hide
changes that signal the workaround needs updating or removal. Predicate,
column-pruning, and result checks remain intact, so we're retaining the exact
expectations.
##########
.github/workflows/util/delta-spark-ut/apply-delta-test-patches.sh:
##########
@@ -82,43 +83,31 @@ cherry_pick_delta_fix
46bd45d57eadd7e528002a0ae7bd36ce5a456eca "#7104 (ScanRepor
cherry_pick_delta_fix 959e00e15f41f56afc1c9bb95d160c55c6dc7068 "#7105 (9 more
test suites)"
echo "::endgroup::"
-echo "::group::Capping DeltaParquetFileFormat fixture row groups by row count"
-# DeltaParquetFileFormatSuite generates one 20,000-row Parquet file and sets a
-# 50 KiB block size to ensure that it contains multiple row groups. Velox sizes
-# row groups by buffered bytes after writing each input batch. Because this
-# fixture arrives in one batch, lowering the byte threshold cannot split it.
-# Scope Gluten's native row-count limit around the fixture write so Arrow
splits
-# the 20,000 rows deterministically while keeping the native write path
enabled.
+echo "::group::Applying temporary Delta test patches"
+# Keep local patches after the cherry-picks, which require an unmodified
worktree
+# for the files they touch. Patch paths are relative to this script, not Delta.
+for patch in \
+ cdf-pushed-filter-assertions.patch \
+ parquet-fixture-row-groups.patch \
+ dv-predicate-pushdown-row-groups.patch \
+ dv-2b-row-fail-fast.patch; do
+ echo "Applying $patch"
+ if ! git -C "$DELTA_DIR" apply "$PATCH_DIR/$patch"; then
+ echo "ERROR: Delta test patch '$patch' did not apply." >&2
+ echo "The patch expects Delta v4.2.0 test sources;" \
+ "ref '${DELTA_REF}' must remain source-compatible." >&2
+ exit 1
+ fi
Review Comment:
`git apply` already reports the missing file and its path: `error: can't
open patch '<path>': No such file or directory`. We preserve that stderr in the
CI log and exit on failure. An additional existence/readability check would
duplicate Git's handling, so we're keeping this path simple.
##########
.github/workflows/util/delta-spark-ut/apply-delta-test-patches.sh:
##########
@@ -34,6 +34,7 @@ fi
DELTA_REF="$1"
DELTA_DIR="$2"
+PATCH_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/patches"
Review Comment:
This is already a Bash script (`#!/usr/bin/env bash`). The Delta workflow
explicitly invokes `setup-delta.sh` with `bash`, and that script explicitly
invokes this patcher with `bash` as well. POSIX `sh` compatibility isn't
required, so we're keeping `BASH_SOURCE` unchanged.
--
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]
