Repository navigation
Conversation
Signed-off-by: Yuan <yuanzhou@apache.org>
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
11 open findings
-Piceberg -Phudi -Pdelta -Ppaimoncompared to the 3.5/4.0/4.1… · New-Piceberg -Phudi -Pdelta -Ppaimoncompared to the 3.5/4.0/4.1… · New-Piceberg -Phudi -Pdelta -Ppaimoncompared to the 3.5/4.0/4.1… · New-Piceberg -Phudi -Pdelta -Ppaimoncompared to the 3.5/4.0/4.1… · New-Piceberg -Phudi -Pdelta -Ppaimoncompared to the 3.5/4.0/4.1… · New.load(taskSetId)) with the staged-scan option… · Newactions/upload-artifact@v7elsewhere, but the newly added/reintroduced Spark… · Newactions/upload-artifact@v7elsewhere, but the newly added/reintroduced Spark… · New
What changed in this PR
Updates Velox nightly CI to include Spark 4.2 builds and adjusts Iceberg staged-scan testing to work with Iceberg 1.11’s removal of SparkReadOptions.SCAN_TASK_SET_ID.
Changes:
- Update Iceberg staged scan test to use
SparkTableCache+ string option key instead ofSparkReadOptions.SCAN_TASK_SET_ID. - Add Spark 4.2 build profiles to Velox nightly workflow (Java 17/21).
- Reorder/re-add Spark 3.5 SMJ test jobs in the workflow.
| File | Description |
|---|---|
| backends-velox/src-iceberg/test/scala/org/apache/gluten/execution/VeloxIcebergSuite.scala | Switch staged scan setup to Iceberg 1.11-compatible table caching and task-set ID loading. |
| .github/workflows/velox_nightly.yml | Add Spark 4.2 builds and adjust nightly job layout/test jobs. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| import org.apache.iceberg.hadoop.HadoopCatalog | ||
| import org.apache.iceberg.io.{ResolvingFileIO, StorageCredential} | ||
| import org.apache.iceberg.spark.{ScanTaskSetManager, Spark3Util, SparkCatalog, SparkReadOptions} | ||
| import org.apache.iceberg.spark.{ScanTaskSetManager, Spark3Util, SparkCatalog, SparkTableCache} |
Comment on lines
+122
to
+128
| val tableCache = SparkTableCache.get() | ||
| val tasks = table.newScan().planFiles() | ||
| try { | ||
| // Register the table under the task set ID and load by that ID, which resolves to a | ||
| // staged scan on both Iceberg 1.10 (needs the "scan-task-set-id" option) and Iceberg | ||
| // 1.11+ (removed SparkReadOptions.SCAN_TASK_SET_ID, resolves cached tables directly). | ||
| tableCache.add(taskSetId, table) |
Comment on lines
+134
to
+135
| .option("scan-task-set-id", taskSetId) | ||
| .load(taskSetId), |
| ./build/mvn clean install -Pspark-3.5 -Pjava-17 -Pbackends-velox -Pceleborn -Puniffle -Piceberg -Phudi -Pdelta -Ppaimon -DskipTests -Dmaven.source.skip | ||
| ./build/mvn clean install -Pspark-4.0 -Pscala-2.13 -Pjava-17 -Pbackends-velox -Pceleborn -Puniffle -Piceberg -Phudi -Pdelta -Ppaimon -DskipTests -Dmaven.source.skip | ||
| ./build/mvn clean install -Pspark-4.1 -Pscala-2.13 -Pjava-17 -Pbackends-velox -Pceleborn -Puniffle -Piceberg -Phudi -Pdelta -Ppaimon -DskipTests -Dmaven.source.skip | ||
| ./build/mvn clean install -Pspark-4.2 -Pscala-2.13 -Pjava-17 -Pbackends-velox -Pceleborn -Puniffle -DskipTests -Dmaven.source.skip |
| @@ -170,110 +171,14 @@ jobs: | |||
| cd $GITHUB_WORKSPACE/ && \ | |||
| ./build/mvn clean install -Pspark-4.0 -Pscala-2.13 -Pjava-21 -Pbackends-velox -Pceleborn -Puniffle -Piceberg -Phudi -Pdelta -Ppaimon -DskipTests -Dmaven.source.skip | |||
| ./build/mvn clean install -Pspark-4.1 -Pscala-2.13 -Pjava-21 -Pbackends-velox -Pceleborn -Puniffle -Piceberg -Phudi -Pdelta -DskipTests -Dmaven.source.skip | |||
| cd $GITHUB_WORKSPACE/ && \ | ||
| ./build/mvn clean install -Pspark-4.0 -Pscala-2.13 -Pjava-21 -Pbackends-velox -Pceleborn -Puniffle -Piceberg -Phudi -Pdelta -Ppaimon -DskipTests -Dmaven.source.skip | ||
| ./build/mvn clean install -Pspark-4.1 -Pscala-2.13 -Pjava-21 -Pbackends-velox -Pceleborn -Puniffle -Piceberg -Phudi -Pdelta -Ppaimon -DskipTests -Dmaven.source.skip | ||
| ./build/mvn clean install -Pspark-4.2 -Pscala-2.13 -Pjava-21 -Pbackends-velox -Pceleborn -Puniffle -DskipTests -Dmaven.source.skip |
| ./build/mvn clean install -Pspark-3.5 -Pjava-17 -Pbackends-velox -Pceleborn -Puniffle -Piceberg -Phudi -Pdelta -Ppaimon -DskipTests -Dmaven.source.skip | ||
| ./build/mvn clean install -Pspark-4.0 -Pscala-2.13 -Pjava-17 -Pbackends-velox -Pceleborn -Puniffle -Piceberg -Phudi -Pdelta -Ppaimon -DskipTests -Dmaven.source.skip | ||
| ./build/mvn clean install -Pspark-4.1 -Pscala-2.13 -Pjava-17 -Pbackends-velox -Pceleborn -Puniffle -Piceberg -Phudi -Pdelta -Ppaimon -DskipTests -Dmaven.source.skip | ||
| ./build/mvn clean install -Pspark-4.2 -Pscala-2.13 -Pjava-17 -Pbackends-velox -Pceleborn -Puniffle -DskipTests -Dmaven.source.skip |
Comment on lines
+134
to
+135
| .option("scan-task-set-id", taskSetId) | ||
| .load(taskSetId), |
Comment on lines
+538
to
+544
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: ${{ github.job }}-report | ||
| path: '**/surefire-reports/TEST-*.xml' | ||
| - name: Upload unit tests log files | ||
| if: ${{ !success() }} | ||
| uses: actions/upload-artifact@v4 |
Comment on lines
+538
to
+544
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: ${{ github.job }}-report | ||
| path: '**/surefire-reports/TEST-*.xml' | ||
| - name: Upload unit tests log files | ||
| if: ${{ !success() }} | ||
| uses: actions/upload-artifact@v4 |
Copilot stopped reviewing on behalf of
zhouyuan due to an error
October 15, 2026 15:57
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes are proposed in this pull request?
fix nightly build
SparkReadOptions.SCAN_TASK_SET_ID is removed in Iceberg 1.11. 1.11 finds staged scans by registering the table in SparkTableCache and loading by the set id.
How was this patch tested?
pass GHA
Was this patch authored or co-authored using generative AI tooling?