felipepessoto commented on code in PR #13129:
URL: https://github.com/apache/gluten/pull/13129#discussion_r4273526141
##########
.github/workflows/util/delta-spark-ut/apply-delta-test-patches.sh:
##########
@@ -82,43 +83,55 @@ 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
+done
+echo "::endgroup::"
+
+echo "::group::Checking Delta CDF pushed-filter assertions"
+CDF_TEST_DIR="$DELTA_DIR/spark/src/test/scala/org/apache/spark/sql/delta"
+for suite in DeltaCDCSuite DeltaCDCSQLSuite DeltaCDCColumnMappingSuite; do
+ if [ ! -f "$CDF_TEST_DIR/$suite.scala" ]; then
+ echo "Expected file not found in Delta clone: $CDF_TEST_DIR/$suite.scala"
>&2
+ echo "The Delta directory layout for ref '${DELTA_REF}' may have changed."
>&2
+ exit 1
+ fi
+ EXPECTED=' .contains("PushedFilters: [IsNotNull(id),
LessThan(id,5)]"))'
+ if [ "$suite" = DeltaCDCColumnMappingSuite ]; then
+ EXPECTED=' .contains("PushedFilters: [IsNotNull(`id with space`),
LessThan(`id with space`,5)]"))'
+ fi
+ MATCHED=$(grep -Fxc "$EXPECTED" "$CDF_TEST_DIR/$suite.scala" || true)
+ ASSERTIONS=$(grep -Fc '.contains("PushedFilters:'
"$CDF_TEST_DIR/$suite.scala" || true)
+ if [ "$MATCHED" != "1" ] || [ "$ASSERTIONS" != "1" ]; then
+ echo "ERROR: expected exactly one Gluten pushed-filter assertion in
$suite.scala;" \
+ "found ${MATCHED} matching out of ${ASSERTIONS} assertions." >&2
+ echo "The CDF assertions may have changed in Delta ref '${DELTA_REF}'." >&2
+ exit 1
+ fi
+done
Review Comment:
These are defensive source-drift checks: they verify that each suite
contains exactly one expected patched assertion and no additional PushedFilters
assertions that the patch leaves untouched.
They’re not strictly necessary for patch application—git apply already fails
if the files or required context are missing. The additional coverage is
detecting assertions added elsewhere in the suite, outside the patch hunks, so
setup fails early rather than during the tests.
Given the pinned Delta version, we could remove these checks if we prefer
relying on patch application and the test results.
--
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]
