You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "Zouxxyy (via GitHub)" <gi...@apache.org> on 2025/10/16 13:19:02 UTC
[PR] [VL] Check whether the write is a supported iceberg write before executing offload [incubator-gluten]
Zouxxyy opened a new pull request, #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900
<!--
Thank you for submitting a pull request! Here are some tips:
1. For first-time contributors, please read our contributing guide:
https://github.com/apache/incubator-gluten/blob/main/CONTRIBUTING.md
2. If necessary, create a GitHub issue for discussion beforehand to avoid duplicate work.
3. If the PR is specific to a single backend, include [VL] or [CH] in the PR title to indicate the
Velox or ClickHouse backend, respectively.
4. If the PR is not ready for review, please mark it as a draft.
-->
## What changes are proposed in this pull request?
<!--
Provide a clear and concise description of the changes introduced in this PR.
Ensure the PR description aligns with the code changes, especially after updates.
If applicable, include "Fixes #<GitHub_Issue_ID>" to automatically close the corresponding issue
when the PR is merged.
-->
## How was this patch tested?
<!--
Describe how the changes were tested, if applicable.
Include new tests to validate the functionality, if necessary.
For UI-related changes, attach screenshots to demonstrate the updates.
-->
--
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] [VL] Validate Iceberg write support before executing offload [incubator-gluten]
Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on code in PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#discussion_r2446868970
##########
gluten-iceberg/src/main/scala/org/apache/iceberg/spark/source/IcebergWriteUtil.scala:
##########
@@ -27,8 +28,8 @@ import org.apache.iceberg.types.Types.{ListType, MapType}
object IcebergWriteUtil {
- def isDataWrite(write: Write): Boolean = {
- write.isInstanceOf[SparkWrite]
+ def supportsWrite(write: Write): Boolean = {
+ SparkReflectionUtil.isInstanceOfClassName(write, "org.apache.iceberg.spark.source.SparkWrite")
Review Comment:
For example, if a user downloads a bundled Gluten package that includes the Iceberg module, but their runtime environment does not have Iceberg, then if we keep use `write.isInstanceOf[SparkWrite]`, below will throw an class not found exception:
`Case a: AppendDataExec if supportsWrite(a.write) =>`
like what we do in offload scan: https://github.com/apache/incubator-gluten/blob/71bcdb5b14858a25d860cfd819131c4888384994/gluten-paimon/src-paimon/main/scala/org/apache/gluten/execution/OffloadPaimonScan.scala#L26
--
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] [VL] Check whether the write is a supported iceberg write before executing offload [incubator-gluten]
Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#issuecomment-3413699274
The failed CK CI should be unrelated, it seems that CK's iceberg compilation is not performed in jdk11+, while iceberg 1.8 need jdk11+. CC @jinchengchenghh
```
[2025-10-17T01:12:29.610Z] [ERROR] /home/jenkins/agent/workspace/gluten/gluten-ci-arm/ut-stage-5/gluten-iceberg/src/main/java/org/apache/gluten/substrait/rel/IcebergLocalFilesNode.java:[22,25] error: cannot access DeleteFile
[2025-10-17T01:12:29.610Z] bad class file: /root/.m2/repository/org/apache/iceberg/iceberg-spark-runtime-3.5_2.13/1.8.0/iceberg-spark-runtime-3.5_2.13-1.8.0.jar(org/apache/iceberg/DeleteFile.class)
[2025-10-17T01:12:29.610Z] class file has wrong version 55.0, should be 52.0
[2025-10-17T01:12:29.610Z] Please remove or make sure it appears in the correct subdirectory of the classpath.
[2025-10-17T01:12:29.610Z] [INFO] 1 error
[2025-10-17T01:12:29.610Z] [INFO] -------------------------------------------------------------
[2025-10-17T01:12:29.610Z] [INFO] ------------------------------------------------------------------------
[2025-10-17T01:12:29.610Z] [INFO] Reactor Summary for Gluten Parent Pom 1.6.0-SNAPSHOT:
[2025-10-17T01:12:29.610Z] [INFO]
[2025-10-17T01:12:29.610Z] [INFO] Gluten Parent Pom .................................. SUCCESS [ 20.933 s]
[2025-10-17T01:12:29.610Z] [INFO] Gluten Ras ......................................... SUCCESS [ 18.400 s]
[2025-10-17T01:12:29.610Z] [INFO] Gluten Ras Common .................................. SUCCESS [01:05 min]
[2025-10-17T01:12:29.610Z] [INFO] Gluten Core ........................................ SUCCESS [ 38.461 s]
[2025-10-17T01:12:29.610Z] [INFO] Gluten Shims ....................................... SUCCESS [ 0.348 s]
[2025-10-17T01:12:29.610Z] [INFO] Gluten Shims Common ................................ SUCCESS [ 8.182 s]
[2025-10-17T01:12:29.610Z] [INFO] Gluten Shims for Spark 3.5 ......................... SUCCESS [ 13.639 s]
[2025-10-17T01:12:29.610Z] [INFO] Gluten UI .......................................... SUCCESS [ 5.587 s]
[2025-10-17T01:12:29.610Z] [INFO] Gluten Substrait ................................... SUCCESS [01:13 min]
[2025-10-17T01:12:29.610Z] [INFO] Gluten Celeborn .................................... SUCCESS [ 5.869 s]
[2025-10-17T01:12:29.610Z] [INFO] Gluten Iceberg ..................................... FAILURE [ 8.210 s]
[2025-10-17T01:12:29.610Z] [INFO] Gluten DeltaLake ................................... SKIPPED
[2025-10-17T01:12:29.610Z] [INFO] Gluten Package ..................................... SKIPPED
[2025-10-17T01:12:29.610Z] [INFO] Gluten Ras Planner ................................. SKIPPED
[2025-10-17T01:12:29.610Z] [INFO] Gluten Kafka ....................................... SKIPPED
[2025-10-17T01:12:29.610Z] [INFO] Gluten Backends ClickHouse ......................... SKIPPED
[2025-10-17T01:12:29.610Z] [INFO] Gluten Unit Test Parent ............................ SKIPPED
[2025-10-17T01:12:29.610Z] [INFO] Gluten Unit Test Common ............................ SKIPPED
[2025-10-17T01:12:29.610Z] [INFO] Gluten Unit Test ................................... SKIPPED
[2025-10-17T01:12:29.611Z] [INFO] Gluten Unit Test Spark35 ........................... SKIPPED
[2025-10-17T01:12:29.611Z] [INFO] ------------------------------------------------------------------------
[2025-10-17T01:12:29.611Z] [INFO] BUILD FAILURE
[2025-10-17T01:12:29.611Z] [INFO] ------------------------------------------------------------------------
[2025-10-17T01:12:29.611Z] [INFO] Total time: 04:19 min
[2025-10-17T01:12:29.611Z] [INFO] Finished at: 2025-10-17T01:12:29Z
[2025-10-17T01:12:29.611Z] [INFO] ------------------------------------------------------------------------
[2025-10-17T01:12:29.611Z] [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.12.1:compile (default-compile) on project gluten-iceberg: Compilation failure
[2025-10-17T01:12:29.611Z] [ERROR] /home/jenkins/agent/workspace/gluten/gluten-ci-arm/ut-stage-5/gluten-iceberg/src/main/java/org/apache/gluten/substrait/rel/IcebergLocalFilesNode.java:[22,25] error: cannot access DeleteFile
[2025-10-17T01:12:29.611Z] [ERROR] bad class file: /root/.m2/repository/org/apache/iceberg/iceberg-spark-runtime-3.5_2.13/1.8.0/iceberg-spark-runtime-3.5_2.13-1.8.0.jar(org/apache/iceberg/DeleteFile.class)
[2025-10-17T01:12:29.611Z] [ERROR] class file has wrong version 55.0, should be 52.0
[2025-10-17T01:12:29.611Z] [ERROR] Please remove or make sure it appears in the correct subdirectory of the classpath.
```
--
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] [VL] Check whether the write is a supported iceberg write before executing offload [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#issuecomment-3413755876
Run Gluten ClickHouse CI on ARM
--
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] [VL] Check whether the write is a supported iceberg write before executing offload [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#issuecomment-3415574370
Run Gluten ClickHouse CI on x86
--
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] [VL] Check whether the write is a supported iceberg write before executing offload [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#issuecomment-3413804393
Run Gluten ClickHouse CI on ARM
--
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] [VL] Check whether the write is a supported iceberg write before executing offload [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#issuecomment-3415570213
I see there's a [1/10 chance](https://github.com/apache/incubator-gluten/blob/a2f90c2bdf90e38d45acb7f1eaa330ac171aad29/.github/workflows/clickhouse_be_trigger.yml#L55-L59) that a PR's CH CI will be run on ARM rather than on x86. And ARM CI has been seen as having this flaky result issue for a period of time.
I've re-triggered with `Run Gluten ClickHouse CI` to see whether we can run the CI on x86 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] [VL] Check whether the write is a supported iceberg write before executing offload [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#issuecomment-3411096318
> Is this by designed, or am I missing something? @zhztheplayer thanks
Yes, this sounds like one of the major issues we have for making different components (e.g., paimon + iceberg) work together. [This validator](https://github.com/apache/incubator-gluten/blob/8dc5d471a2fb8a61d5c33846f98fc20c93ab1eac/gluten-substrait/src/main/scala/org/apache/gluten/extension/columnar/validator/Validators.scala#L99-L107) was written for keeping up with some legacy code in our query planner, and I do think it should be eventually removed or redesigned.
We should make one component try to offload a plan node that the previous component couldn't handle, not skip it.
Maybe we can just make an attempt to remove that validator to see what happens then.
--
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] [VL] Check whether the write is a supported iceberg write before executing offload [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#issuecomment-3415542795
Run Gluten ClickHouse 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] [VL] Validate Iceberg write support before executing offload [incubator-gluten]
Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on code in PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#discussion_r2446908152
##########
gluten-iceberg/src/main/scala/org/apache/iceberg/spark/source/IcebergWriteUtil.scala:
##########
@@ -27,8 +28,8 @@ import org.apache.iceberg.types.Types.{ListType, MapType}
object IcebergWriteUtil {
- def isDataWrite(write: Write): Boolean = {
- write.isInstanceOf[SparkWrite]
+ def supportsWrite(write: Write): Boolean = {
+ SparkReflectionUtil.isInstanceOfClassName(write, "org.apache.iceberg.spark.source.SparkWrite")
Review Comment:
Our internal UT also encountered a scenario: if the iceberg profile was enabled (build velox backend with iceberg) and the submodule which depend the velox backend but had no iceberg test dependency, if `AppendDataExec` was used in the UT, such as V2 noop write, it would also report class not found.
--
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] [VL] Check whether the write is a supported iceberg write before executing offload [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#issuecomment-3413374187
Run Gluten ClickHouse CI on ARM
--
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] [VL] Check whether the write is a supported iceberg write before executing offload [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#issuecomment-3413718460
Run Gluten ClickHouse CI on ARM
--
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] [VL] Check whether the write is a supported iceberg write before executing offload [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#issuecomment-3410862502
Run Gluten ClickHouse CI on ARM
--
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] [VL] Check whether the write is a supported iceberg write before executing offload [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#issuecomment-3411163036
Run Gluten ClickHouse CI on ARM
--
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] [VL] Validate Iceberg write support before executing offload [incubator-gluten]
Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on code in PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#discussion_r2446810333
##########
gluten-iceberg/src/main/scala/org/apache/iceberg/spark/source/IcebergWriteUtil.scala:
##########
@@ -27,8 +28,8 @@ import org.apache.iceberg.types.Types.{ListType, MapType}
object IcebergWriteUtil {
- def isDataWrite(write: Write): Boolean = {
- write.isInstanceOf[SparkWrite]
+ def supportsWrite(write: Write): Boolean = {
+ SparkReflectionUtil.isInstanceOfClassName(write, "org.apache.iceberg.spark.source.SparkWrite")
Review Comment:
It is a prerequisite for offloading:
`Case a: AppendDataExec if supportsWrite(a.write) =>`
The reason for modifying it is to maintain a unified supportsWrite method.
--
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] [VL] Validate Iceberg write support before executing offload [incubator-gluten]
Posted by "PHILO-HE (via GitHub)" <gi...@apache.org>.
PHILO-HE commented on code in PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#discussion_r2446829928
##########
gluten-iceberg/src/main/scala/org/apache/iceberg/spark/source/IcebergWriteUtil.scala:
##########
@@ -27,8 +28,8 @@ import org.apache.iceberg.types.Types.{ListType, MapType}
object IcebergWriteUtil {
- def isDataWrite(write: Write): Boolean = {
- write.isInstanceOf[SparkWrite]
+ def supportsWrite(write: Write): Boolean = {
+ SparkReflectionUtil.isInstanceOfClassName(write, "org.apache.iceberg.spark.source.SparkWrite")
Review Comment:
I meant why not just using the following code. Could you clarify?
```
def supportsWrite(write: Write): Boolean = {
write.isInstanceOf[SparkWrite]
}
```
--
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] [VL] Check whether the write is a supported iceberg write before executing offload [incubator-gluten]
Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on code in PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#discussion_r2435929668
##########
backends-velox/src-iceberg/main/scala/org/apache/gluten/extension/OffloadIcebergWrite.scala:
##########
@@ -18,48 +18,56 @@ package org.apache.gluten.extension
import org.apache.gluten.config.GlutenConfig
import org.apache.gluten.execution.{VeloxIcebergAppendDataExec, VeloxIcebergOverwriteByExpressionExec, VeloxIcebergOverwritePartitionsDynamicExec, VeloxIcebergReplaceDataExec}
+import org.apache.gluten.extension.OffloadIcebergWrite._
import org.apache.gluten.extension.columnar.enumerated.RasOffload
import org.apache.gluten.extension.columnar.heuristic.HeuristicTransform
import org.apache.gluten.extension.columnar.offload.OffloadSingleNode
import org.apache.gluten.extension.columnar.validator.Validators
import org.apache.gluten.extension.injector.Injector
+import org.apache.spark.sql.connector.write.Write
import org.apache.spark.sql.execution.SparkPlan
import org.apache.spark.sql.execution.datasources.v2.{AppendDataExec, OverwriteByExpressionExec, OverwritePartitionsDynamicExec, ReplaceDataExec}
+import org.apache.spark.util.SparkReflectionUtil
case class OffloadIcebergAppend() extends OffloadSingleNode {
override def offload(plan: SparkPlan): SparkPlan = plan match {
- case a: AppendDataExec =>
+ case a: AppendDataExec if supportsWrite(a.write) =>
VeloxIcebergAppendDataExec(a)
case other => other
}
}
case class OffloadIcebergReplaceData() extends OffloadSingleNode {
override def offload(plan: SparkPlan): SparkPlan = plan match {
- case r: ReplaceDataExec =>
+ case r: ReplaceDataExec if supportsWrite(r.write) =>
VeloxIcebergReplaceDataExec(r)
case other => other
}
}
case class OffloadIcebergOverwrite() extends OffloadSingleNode {
override def offload(plan: SparkPlan): SparkPlan = plan match {
- case r: OverwriteByExpressionExec =>
+ case r: OverwriteByExpressionExec if supportsWrite(r.write) =>
VeloxIcebergOverwriteByExpressionExec(r)
case other => other
}
}
case class OffloadIcebergOverwritePartitionsDynamic() extends OffloadSingleNode {
override def offload(plan: SparkPlan): SparkPlan = plan match {
- case r: OverwritePartitionsDynamicExec =>
+ case r: OverwritePartitionsDynamicExec if supportsWrite(r.write) =>
VeloxIcebergOverwritePartitionsDynamicExec(r)
case other => other
}
}
object OffloadIcebergWrite {
+
+ def supportsWrite(write: Write): Boolean = {
Review Comment:
I added reflection here to ensure that this rule can still execute correctly when there's no Iceberg dependency. A similar approach is used in offloadBatchScan.
--
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] [VL] Validate Iceberg write support before executing offload [incubator-gluten]
Posted by "PHILO-HE (via GitHub)" <gi...@apache.org>.
PHILO-HE commented on code in PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#discussion_r2453803034
##########
gluten-iceberg/src/main/scala/org/apache/iceberg/spark/source/IcebergWriteUtil.scala:
##########
@@ -27,8 +28,8 @@ import org.apache.iceberg.types.Types.{ListType, MapType}
object IcebergWriteUtil {
- def isDataWrite(write: Write): Boolean = {
- write.isInstanceOf[SparkWrite]
+ def supportsWrite(write: Write): Boolean = {
+ SparkReflectionUtil.isInstanceOfClassName(write, "org.apache.iceberg.spark.source.SparkWrite")
Review Comment:
@Zouxxyy, it sounds good to me. I will draft a PR in response to the 1st proposal. Let's merge this PR first.
--
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] [VL] Validate Iceberg write support before executing offload [incubator-gluten]
Posted by "PHILO-HE (via GitHub)" <gi...@apache.org>.
PHILO-HE merged PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900
--
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] [VL] Check whether the write is a supported iceberg write before executing offload [incubator-gluten]
Posted by "PHILO-HE (via GitHub)" <gi...@apache.org>.
PHILO-HE commented on PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#issuecomment-3418063353
The current iceberg version for Spark 3.5 doesn't support JDK 8. It seems that CH CI job needs a fix by either using higher JDK version or disabling iceberg when building Gluten for Spark 3.5. @zzcclp
--
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] [VL] Validate Iceberg write support before executing offload [incubator-gluten]
Posted by "PHILO-HE (via GitHub)" <gi...@apache.org>.
PHILO-HE commented on code in PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#discussion_r2447034497
##########
gluten-iceberg/src/main/scala/org/apache/iceberg/spark/source/IcebergWriteUtil.scala:
##########
@@ -27,8 +28,8 @@ import org.apache.iceberg.types.Types.{ListType, MapType}
object IcebergWriteUtil {
- def isDataWrite(write: Write): Boolean = {
- write.isInstanceOf[SparkWrite]
+ def supportsWrite(write: Write): Boolean = {
+ SparkReflectionUtil.isInstanceOfClassName(write, "org.apache.iceberg.spark.source.SparkWrite")
Review Comment:
I see. The iceberg dependency is in provided scope. Thanks for the clarification.
Here are some quick thoughts. Please check if they make sense.
1. I'm assuming these iceberg/paimon rules can actually work only when the related dependency is available at runtime. If so, can we first do a check before injecting the corresponding rules? E.g., if some iceberg class is not available at runtime, we will not inject the related rules. Then ClassNotFound issue will not occur for the rules' internal code since the execution is always skipped. This seems also helpful to improve the plan transformation efficiency.
2. If we continue the proposed changes, considering the reflection overhead, can we just use the fully qualified class name string for comparison like what paimon scan validation does?
--
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] [VL] Check whether the write is a supported iceberg write before executing offload [incubator-gluten]
Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#issuecomment-3424557238
@zhztheplayer @PHILO-HE Thanks for the detailed explanation! Gluten ClickHouse CI on x86 works
--
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] [VL] Check whether the write is a supported iceberg write before executing offload [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#issuecomment-3411224690
> Do we have a plan for this part.
I haven't had a clear plan on it, perhaps I can take a closer look next week or so. Please feel free to take it on if you are interested.
--
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] [VL] Check whether the write is a supported iceberg write before executing offload [incubator-gluten]
Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#issuecomment-3410896994
I found that with the current offload + validate fallback:
Suppose we have two components offloading the `AppendExec`, for example, `OffloadIcebergWrite` and `OffloadPaimonWrite`. If validation fails for `VeloxIcebergAppendDataExec`, `AppendDataExec` will no longer be passed to `OffloadPaimonWrite`. Is this by designed, or am I missing something? @zhztheplayer thanks
--
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] [VL] Check whether the write is a supported iceberg write before executing offload [incubator-gluten]
Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#issuecomment-3411163082
> We should make one component try to offload a plan node that the previous component couldn't handle, not skip it.
I see, this sounds greate
--
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] [VL] Check whether the write is a supported iceberg write before executing offload [incubator-gluten]
Posted by "PHILO-HE (via GitHub)" <gi...@apache.org>.
PHILO-HE commented on code in PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#discussion_r2446781980
##########
gluten-iceberg/src/main/scala/org/apache/iceberg/spark/source/IcebergWriteUtil.scala:
##########
@@ -27,8 +28,8 @@ import org.apache.iceberg.types.Types.{ListType, MapType}
object IcebergWriteUtil {
- def isDataWrite(write: Write): Boolean = {
- write.isInstanceOf[SparkWrite]
+ def supportsWrite(write: Write): Boolean = {
+ SparkReflectionUtil.isInstanceOfClassName(write, "org.apache.iceberg.spark.source.SparkWrite")
Review Comment:
I note `SparkWrite` is directly referenced by some other code in this file. I'm wondering if it is still necessary to change to use reflection here.
--
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] [VL] Validate Iceberg write support before executing offload [incubator-gluten]
Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on code in PR #10900:
URL: https://github.com/apache/incubator-gluten/pull/10900#discussion_r2447516812
##########
gluten-iceberg/src/main/scala/org/apache/iceberg/spark/source/IcebergWriteUtil.scala:
##########
@@ -27,8 +28,8 @@ import org.apache.iceberg.types.Types.{ListType, MapType}
object IcebergWriteUtil {
- def isDataWrite(write: Write): Boolean = {
- write.isInstanceOf[SparkWrite]
+ def supportsWrite(write: Write): Boolean = {
+ SparkReflectionUtil.isInstanceOfClassName(write, "org.apache.iceberg.spark.source.SparkWrite")
Review Comment:
I think option 1 is more reasonable. It seems we need to add a method to the component to check whether the environment supports it, and then we can remove all the existing reflection-based judge logic.
The main purpose of this PR is to fix the issue where, after offload to Iceberg but verification failed, can't back to other format. I'd prefer to propel this PR first, and then proceed with the refactoring proposed in 1. What do you all think? @PHILO-HE @zhztheplayer
As for 2, the reason don't use name-based matching is that `org.apache.iceberg.spark.source.SparkWrite` is an abstract class and is implemented using anonymous classes.
--
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