You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "felipepessoto (via GitHub)" <gi...@apache.org> on 2026/09/16 08:13:17 UTC
[PR] [GLUTEN-12743][CI] Make Delta DV row-group fixtures deterministic [gluten]
felipepessoto opened a new pull request, #13036:
URL: https://github.com/apache/gluten/pull/13036
## What changes are proposed in this pull request?
References #12743.
Patch Delta v4.2.0 after cloning so `DeltaParquetFileFormatSuite.generateData` sets `parquet.block.rows=10000` in addition to its existing 50 KiB block-size target.
The fixture writes 20,000 highly compressible integer rows. Gluten forwards `parquet.block.size`, but Velox bases the byte flush decision on compressed buffered bytes, so the fixture can remain a single row group and fail before exercising the deletion-vector reads. The independent row-count cap deterministically creates at least two row groups while keeping Gluten's native writer enabled.
Remove the 24 `DeltaParquetFileFormatSuite` and `DeltaParquetFileFormatWithPredicatePushdownSuite` tests from the known-failure baseline, reducing it from 707 to 683 entries. The source patch is guarded so Delta source drift fails setup explicitly instead of silently losing coverage.
## How was this patch tested?
- `bash -n .github/workflows/util/delta-spark-ut/setup-delta.sh`
- Applied the exact `sed` transformation to the upstream Delta v4.2.0 source and verified exactly one correctly indented `parquet.block.rows` setting was inserted
- Verified the baseline contains 683 non-comment entries and no entries for either affected suite
- `git diff --check`
The full native Delta test matrix is delegated to CI. The repository header checker could not run locally because Python is unavailable in this Windows environment; both modified files retain their existing ASF headers.
## Was this patch authored or co-authored using generative AI tooling?
Generated-by: GitHub Copilot App 1.0.84-5
Fixes: #12743
--
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
Re: [PR] [GLUTEN-12743][CI] Make Delta DV row-group fixtures deterministic [gluten]
Posted by "felipepessoto (via GitHub)" <gi...@apache.org>.
felipepessoto commented on code in PR #13036:
URL: https://github.com/apache/gluten/pull/13036#discussion_r4023987854
##########
.github/workflows/util/delta-spark-ut/setup-delta.sh:
##########
@@ -172,6 +172,33 @@ 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 compressed buffered bytes, so the highly compressible integer
+# fixture remains a single row group and the DV read tests fail before reaching
+# their assertions. Gluten's native writer also supports parquet.block.rows;
+# cap the fixture at 10,000 rows to create at least two row groups regardless of
+# compression while keeping the native write path enabled.
+DPFFS="$DELTA_DIR/spark/src/test/scala/org/apache/spark/sql/delta/DeltaParquetFileFormatSuite.scala"
+if [ ! -f "$DPFFS" ]; then
+ echo "Expected file not found in Delta clone: $DPFFS" >&2
+ echo "The Delta directory layout for ref '${DELTA_REF}' may have changed." >&2
+ exit 1
+fi
+sed -i '/hadoopConf().set("parquet.block.size", (1024 \* 50).toString)/a\
+ hadoopConf().set("parquet.block.rows", "10000")' "$DPFFS"
+ROW_CAPS=$(grep -c 'hadoopConf().set("parquet.block.rows", "10000")' "$DPFFS" || true)
Review Comment:
Fixed in b0b6183b8. The guard now counts the exact adjacent, fully shaped `parquet.block.size` and `parquet.block.rows` lines instead of counting row-cap text anywhere in the file. I validated it against Delta v4.2.0 and with a simulated source-drift case where the expected block-size line moved but an unrelated row-cap line remained; the latter is correctly rejected.
--
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
Re: [PR] [GLUTEN-12743][CI] Make Delta DV row-group fixtures deterministic [gluten]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13036:
URL: https://github.com/apache/gluten/pull/13036#discussion_r4023948108
##########
.github/workflows/util/delta-spark-ut/setup-delta.sh:
##########
@@ -172,6 +172,33 @@ 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 compressed buffered bytes, so the highly compressible integer
+# fixture remains a single row group and the DV read tests fail before reaching
+# their assertions. Gluten's native writer also supports parquet.block.rows;
+# cap the fixture at 10,000 rows to create at least two row groups regardless of
+# compression while keeping the native write path enabled.
+DPFFS="$DELTA_DIR/spark/src/test/scala/org/apache/spark/sql/delta/DeltaParquetFileFormatSuite.scala"
+if [ ! -f "$DPFFS" ]; then
+ echo "Expected file not found in Delta clone: $DPFFS" >&2
+ echo "The Delta directory layout for ref '${DELTA_REF}' may have changed." >&2
+ exit 1
+fi
+sed -i '/hadoopConf().set("parquet.block.size", (1024 \* 50).toString)/a\
+ hadoopConf().set("parquet.block.rows", "10000")' "$DPFFS"
+ROW_CAPS=$(grep -c 'hadoopConf().set("parquet.block.rows", "10000")' "$DPFFS" || true)
Review Comment:
This guard counts the row-cap text anywhere in the file, not whether it was inserted immediately after the expected block-size setting. If Delta moves that setting while an unrelated occurrence of the same row-cap line remains elsewhere, `sed` matches nothing and setup still succeeds without capping `generateData`, silently restoring the single-row-group failure. Validate the exact adjacent code lines (and their line shape) instead.
--
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
Re: [PR] [GLUTEN-12743][CI] Make Delta DV row-group fixtures deterministic [gluten]
Posted by "felipepessoto (via GitHub)" <gi...@apache.org>.
felipepessoto commented on PR #13036:
URL: https://github.com/apache/gluten/pull/13036#issuecomment-5718723265
@zhouyuan, @philo-he, when you have a chance, could you take a look?
--
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
Re: [PR] [GLUTEN-12743][CI] Make Delta DV row-group fixtures deterministic [gluten]
Posted by "zhouyuan (via GitHub)" <gi...@apache.org>.
zhouyuan commented on PR #13036:
URL: https://github.com/apache/gluten/pull/13036#issuecomment-5729800200
@felipepessoto thanks for the continuous efforts on maintaining the delta CI!
--
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
Re: [PR] [GLUTEN-12743][CI] Make Delta DV row-group fixtures deterministic [gluten]
Posted by "felipepessoto (via GitHub)" <gi...@apache.org>.
felipepessoto commented on code in PR #13036:
URL: https://github.com/apache/gluten/pull/13036#discussion_r4032776678
##########
.github/workflows/util/delta-spark-ut/setup-delta.sh:
##########
@@ -172,6 +172,59 @@ 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.
+DPFFS="$DELTA_DIR/spark/src/test/scala/org/apache/spark/sql/delta/DeltaParquetFileFormatSuite.scala"
+if [ ! -f "$DPFFS" ]; then
+ echo "Expected file not found in Delta clone: $DPFFS" >&2
+ echo "The Delta directory layout for ref '${DELTA_REF}' may have changed." >&2
+ exit 1
+fi
+if ! sed 's/^__BLANK_CONTEXT__$/ /' <<'PATCH' | git -C "$DELTA_DIR" apply -
+diff --git a/spark/src/test/scala/org/apache/spark/sql/delta/DeltaParquetFileFormatSuite.scala b/spark/src/test/scala/org/apache/spark/sql/delta/DeltaParquetFileFormatSuite.scala
+--- a/spark/src/test/scala/org/apache/spark/sql/delta/DeltaParquetFileFormatSuite.scala
++++ b/spark/src/test/scala/org/apache/spark/sql/delta/DeltaParquetFileFormatSuite.scala
+@@ -68,9 +68,11 @@ trait DeltaParquetFileFormatSuiteBase
+ protected def generateData(tablePath: String): Unit = {
+ // This is to generate a Parquet file with two row groups
+ hadoopConf().set("parquet.block.size", (1024 * 50).toString)
+__BLANK_CONTEXT__
+ // Keep the number of partitions to 1 to generate a single Parquet data file
+ val df = Seq.range(0, 20000).toDF().repartition(1)
+- df.write.format("delta").mode("append").save(tablePath)
++ withSQLConf("spark.gluten.sql.native.parquet.write.blockRows" -> "10000") {
++ df.write.format("delta").mode("append").save(tablePath)
++ }
+__BLANK_CONTEXT__
+ // Set DFS block size to be less than Parquet rowgroup size, to allow
+PATCH
+then
+ echo "ERROR: DeltaParquetFileFormat fixture patch did not apply." >&2
+ echo "The patch expects the Delta v4.2.0 generateData fixture shape;" \
+ "ref '${DELTA_REF}' must remain source-compatible." >&2
+ exit 1
+fi
+ROW_CAP_SCOPES=$(
+ grep -Fxc \
+ ' withSQLConf("spark.gluten.sql.native.parquet.write.blockRows" -> "10000") {' \
+ "$DPFFS" || true
+)
Review Comment:
The preceding `git apply` already enforces this complete sequence atomically: the `generateData` method, 50 KiB setting, 20,000-row DataFrame, fixture write, and following DFS comment are all context in one hunk. If that fixture is rearranged, the patch fails and setup exits before this postcondition. This count only verifies the applied result; an unrelated occurrence cannot make a failed `git apply` succeed, and an additional occurrence would make the count 2 and fail. Repeating the full-block validation after an already successful contextual apply would therefore add redundant parsing without strengthening the guard.
--
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
Re: [PR] [GLUTEN-12743][CI] Make Delta DV row-group fixtures deterministic [gluten]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13036:
URL: https://github.com/apache/gluten/pull/13036#discussion_r4032764009
##########
.github/workflows/util/delta-spark-ut/setup-delta.sh:
##########
@@ -172,6 +172,59 @@ 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.
+DPFFS="$DELTA_DIR/spark/src/test/scala/org/apache/spark/sql/delta/DeltaParquetFileFormatSuite.scala"
+if [ ! -f "$DPFFS" ]; then
+ echo "Expected file not found in Delta clone: $DPFFS" >&2
+ echo "The Delta directory layout for ref '${DELTA_REF}' may have changed." >&2
+ exit 1
+fi
+if ! sed 's/^__BLANK_CONTEXT__$/ /' <<'PATCH' | git -C "$DELTA_DIR" apply -
+diff --git a/spark/src/test/scala/org/apache/spark/sql/delta/DeltaParquetFileFormatSuite.scala b/spark/src/test/scala/org/apache/spark/sql/delta/DeltaParquetFileFormatSuite.scala
+--- a/spark/src/test/scala/org/apache/spark/sql/delta/DeltaParquetFileFormatSuite.scala
++++ b/spark/src/test/scala/org/apache/spark/sql/delta/DeltaParquetFileFormatSuite.scala
+@@ -68,9 +68,11 @@ trait DeltaParquetFileFormatSuiteBase
+ protected def generateData(tablePath: String): Unit = {
+ // This is to generate a Parquet file with two row groups
+ hadoopConf().set("parquet.block.size", (1024 * 50).toString)
+__BLANK_CONTEXT__
+ // Keep the number of partitions to 1 to generate a single Parquet data file
+ val df = Seq.range(0, 20000).toDF().repartition(1)
+- df.write.format("delta").mode("append").save(tablePath)
++ withSQLConf("spark.gluten.sql.native.parquet.write.blockRows" -> "10000") {
++ df.write.format("delta").mode("append").save(tablePath)
++ }
+__BLANK_CONTEXT__
+ // Set DFS block size to be less than Parquet rowgroup size, to allow
+PATCH
+then
+ echo "ERROR: DeltaParquetFileFormat fixture patch did not apply." >&2
+ echo "The patch expects the Delta v4.2.0 generateData fixture shape;" \
+ "ref '${DELTA_REF}' must remain source-compatible." >&2
+ exit 1
+fi
+ROW_CAP_SCOPES=$(
+ grep -Fxc \
+ ' withSQLConf("spark.gluten.sql.native.parquet.write.blockRows" -> "10000") {' \
+ "$DPFFS" || true
+)
Review Comment:
This guard only counts the `withSQLConf` line anywhere in the cloned file. If the upstream fixture is rearranged and an unrelated occurrence remains, setup can still pass even though `generateData` was not capped, so the claimed source-drift fail-fast check is not enforced. Validate one complete sequence tying the 50 KiB `hadoopConf` setting to the 20,000-row `df.write` inside this scope, rather than counting this standalone line.
--
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
Re: [PR] [GLUTEN-12743][CI] Make Delta DV row-group fixtures deterministic [gluten]
Posted by "zhouyuan (via GitHub)" <gi...@apache.org>.
zhouyuan merged PR #13036:
URL: https://github.com/apache/gluten/pull/13036
--
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