[CORE][VL][CH] Make Iceberg code implement component API #8192
Conversation
|
Thanks for opening a pull request! Could you open an issue for this pull request on Github Issues? https://github.com/apache/incubator-gluten/issues Then could you also rename commit message and pull request title in the following format? See also: |
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
zhztheplayer
changed the title
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
| injector.gluten.legacy.injectTransform { | ||
| c => | ||
| val offload = Seq(OffloadIcebergScan()) | ||
| HeuristicTransform.Simple( | ||
| Validators.newValidator(c.glutenConf, offload), | ||
| offload | ||
| ) | ||
| } | ||
|
|
||
| // Inject RAS rule. | ||
| injector.gluten.ras.injectRasRule { | ||
| c => | ||
| RasOffload.Rule( | ||
| RasOffload.from[BatchScanExec](OffloadIcebergScan()), | ||
| Validators.newValidator(c.glutenConf), | ||
| Nil) | ||
| } |
There was a problem hiding this comment.
The code can be simplified in future PRs.
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
| @@ -94,4 +98,8 @@ object IcebergScanTransformer { | |||
| commonPartitionValues = SparkShimLoader.getSparkShims.getCommonPartitionValues(batchScan) | |||
| ) | |||
| } | |||
|
|
|||
| def supportsBatchScan(scan: Scan): Boolean = { | |||
| scan.getClass.getName == "org.apache.iceberg.spark.source.SparkBatchQueryScan" | |||
There was a problem hiding this comment.
Can we just reference this class to get its name for comparison?
There was a problem hiding this comment.
SparkBatchQueryScan is package private.
cc @liujiayi771 if there are some better practices for this.
| class CHIcebergComponent extends Component { | ||
| override def name(): String = "clickhouse-iceberg" | ||
| override def buildInfo(): Component.BuildInfo = | ||
| Component.BuildInfo("ClickHouseIceberg", "N/A", "N/A", "N/A") |
There was a problem hiding this comment.
Seems buildInfo is not necessary for component except Backend. Maybe, just use it for Backend in future refactor.
There was a problem hiding this comment.
Right, this part needs some cleanup. BuildInfo is not that general indeed. I'd like to see it totally decoupled with Component API somehow.
Add
CHIcebergComponent/VeloxIcebergCompoentto replace the previous injections throughScanTransformerFactory.Component API was added in this PR: #8143.
With other minor refactors.