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


##########
.github/workflows/util/delta-spark-ut/patches/dv-2b-row-fail-fast.patch:
##########
@@ -0,0 +1,34 @@
+Subject: [PATCH] Fail fast in memory-hog DeletionVectorsSuite 2B-row tests
+
+These tests read from or delete from a 2-billion-row table. With Gluten,
+row-index materialization balloons the forked test JVM to about 13 GiB of
+native memory and the kernel/cgroup OOM-kills it. The dead fork wedges sbt
+until the workflow watchdog intervenes, skipping suites queued after it.
+
+Fail fast rather than silently ignoring these tests so the gap stays visible
+in test reports and the known-failures baseline. Remove this patch and refresh
+the baseline when the native memory issue is fixed.
+
+Keep the fail calls first in each test body and below Delta's 100-character
+line limit. Delta does not enable fatal dead-code warnings, so the original
+test bodies can remain unchanged.
+
+diff --git 
a/spark/src/test/scala/org/apache/spark/sql/delta/deletionvectors/DeletionVectorsSuite.scala
 
b/spark/src/test/scala/org/apache/spark/sql/delta/deletionvectors/DeletionVectorsSuite.scala
+--- 
a/spark/src/test/scala/org/apache/spark/sql/delta/deletionvectors/DeletionVectorsSuite.scala
++++ 
b/spark/src/test/scala/org/apache/spark/sql/delta/deletionvectors/DeletionVectorsSuite.scala
+@@ -664,6 +664,7 @@ class DeletionVectorsSuite extends QueryTest
+   }
+
+   test("huge table: read from tables of 2B rows with existing DV of many 
zeros") {
++    fail("[Gluten CI] Force-failed: 2B-row DV read OOMs the test JVM; see 
setup-delta.sh")

Review Comment:
   The failure message references `setup-delta.sh` without a path or link, 
which is harder to act on when reading Delta test output in isolation. Consider 
including the relative path within this repo (e.g., 
`.github/workflows/util/delta-spark-ut/setup-delta.sh`) and/or an issue link so 
the log remains actionable even outside the workflow context.



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

Review Comment:
   `grep -Fxc` requires an exact full-line match including 
indentation/whitespace, which makes the setup guard brittle to harmless 
formatting changes (e.g., Scala formatter reflow). Consider matching on the 
stable substring (the `PushedFilters: [...]` payload) and counting occurrences 
of that substring, so the check still detects real drift but doesn’t fail on 
whitespace-only changes.



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

Review Comment:
   The failure message conflates “patch file missing/unreadable” with “patch 
failed to apply to sources.” For clearer diagnostics (and faster fixes when 
moving files/renaming), consider checking `-f "$PATCH_DIR/$patch"` before `git 
apply` and emitting a dedicated error if the patch file isn’t present.



##########
.github/workflows/util/delta-spark-ut/patches/dv-2b-row-fail-fast.patch:
##########
@@ -0,0 +1,34 @@
+Subject: [PATCH] Fail fast in memory-hog DeletionVectorsSuite 2B-row tests
+
+These tests read from or delete from a 2-billion-row table. With Gluten,
+row-index materialization balloons the forked test JVM to about 13 GiB of
+native memory and the kernel/cgroup OOM-kills it. The dead fork wedges sbt
+until the workflow watchdog intervenes, skipping suites queued after it.
+
+Fail fast rather than silently ignoring these tests so the gap stays visible
+in test reports and the known-failures baseline. Remove this patch and refresh
+the baseline when the native memory issue is fixed.
+
+Keep the fail calls first in each test body and below Delta's 100-character
+line limit. Delta does not enable fatal dead-code warnings, so the original
+test bodies can remain unchanged.
+
+diff --git 
a/spark/src/test/scala/org/apache/spark/sql/delta/deletionvectors/DeletionVectorsSuite.scala
 
b/spark/src/test/scala/org/apache/spark/sql/delta/deletionvectors/DeletionVectorsSuite.scala
+--- 
a/spark/src/test/scala/org/apache/spark/sql/delta/deletionvectors/DeletionVectorsSuite.scala
++++ 
b/spark/src/test/scala/org/apache/spark/sql/delta/deletionvectors/DeletionVectorsSuite.scala
+@@ -664,6 +664,7 @@ class DeletionVectorsSuite extends QueryTest
+   }
+
+   test("huge table: read from tables of 2B rows with existing DV of many 
zeros") {
++    fail("[Gluten CI] Force-failed: 2B-row DV read OOMs the test JVM; see 
setup-delta.sh")
+     val canonicalTable5Path = new File(table5Path).getCanonicalPath
+
+     val predicatePushDownEnabled =
+@@ -784,6 +785,7 @@ class DeletionVectorsSuite extends QueryTest
+   for (deleteSpec <- Seq(deleteUsingDvSmallScale)) {
+     test(
+       s"huge table: delete a ${deleteSpec.scale} number of rows from tables 
of 2B rows with DVs") {
++      fail("[Gluten CI] Force-failed: 2B-row DV delete OOMs the test JVM; see 
setup-delta.sh")

Review Comment:
   The failure message references `setup-delta.sh` without a path or link, 
which is harder to act on when reading Delta test output in isolation. Consider 
including the relative path within this repo (e.g., 
`.github/workflows/util/delta-spark-ut/setup-delta.sh`) and/or an issue link so 
the log remains actionable even outside the workflow context.



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