You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "rluvaton (via GitHub)" <gi...@apache.org> on 2025/05/26 13:57:20 UTC
[PR] chore: add assertion that not using comet scan but using native scan [datafusion-comet]
rluvaton opened a new pull request, #1793:
URL: https://github.com/apache/datafusion-comet/pull/1793
## Which issue does this PR close?
N/A
## Rationale for this change
Just add assertion that using datafusion scan when using datafusion filter making sure the code in https://github.com/apache/datafusion-comet/pull/1746 is always correct
## What changes are included in this PR?
Just assertion
## How are these changes tested?
Existing tests
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: add assertion that not using comet scan but using native scan [datafusion-comet]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1793:
URL: https://github.com/apache/datafusion-comet/pull/1793#issuecomment-2913249177
## [Codecov](https://app.codecov.io/gh/apache/datafusion-comet/pull/1793?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 58.05%. Comparing base [(`f09f8af`)](https://app.codecov.io/gh/apache/datafusion-comet/commit/f09f8af64c6599255e116a376f4f008f2fd63b43?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`cceb32a`)](https://app.codecov.io/gh/apache/datafusion-comet/commit/cceb32a2cc6b12a7190bbb0735dbcbed7f528fe8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 216 commits behind head on main.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #1793 +/- ##
============================================
+ Coverage 56.12% 58.05% +1.92%
- Complexity 976 1076 +100
============================================
Files 119 128 +9
Lines 11743 12443 +700
Branches 2251 2347 +96
============================================
+ Hits 6591 7224 +633
+ Misses 4012 4009 -3
- Partials 1140 1210 +70
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/datafusion-comet/pull/1793?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: add assertion that not using comet scan but using native scan [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #1793:
URL: https://github.com/apache/datafusion-comet/pull/1793#issuecomment-2916690342
> I thought everything that came from JVM is reusing buffers, if it's not the case than the copy should not always be added when there is a ScanExec, no? (looking at the wrap in copy exec code)
That's a good point. We currently add CopyExec for all ScanExecs, regardless of the source. This could be optimized.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: add assertion that not using comet scan but using native scan [datafusion-comet]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on code in PR #1793:
URL: https://github.com/apache/datafusion-comet/pull/1793#discussion_r2109425229
##########
spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala:
##########
@@ -2406,19 +2406,19 @@ object QueryPlanSerde extends Logging with CometExprShim {
// TODO this could be optimized more to stop walking the tree on hitting
// certain operators such as join or aggregate which will copy batches
- def containsNativeCometScan(plan: SparkPlan): Boolean = {
+ def containsNonDataFusionScan(plan: SparkPlan): Boolean = {
plan match {
- case w: CometScanWrapper => containsNativeCometScan(w.originalPlan)
- case scan: CometScanExec => scan.scanImpl == CometConf.SCAN_NATIVE_COMET
+ case _: CometScanWrapper => true
Review Comment:
Changed it to return true as native scan exec is not wrapped in CometScanWrapper
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: add assertion that not using comet scan but using native scan [datafusion-comet]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on code in PR #1793:
URL: https://github.com/apache/datafusion-comet/pull/1793#discussion_r2109560295
##########
spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala:
##########
@@ -2406,19 +2406,19 @@ object QueryPlanSerde extends Logging with CometExprShim {
// TODO this could be optimized more to stop walking the tree on hitting
// certain operators such as join or aggregate which will copy batches
- def containsNativeCometScan(plan: SparkPlan): Boolean = {
+ def containsNonDataFusionScan(plan: SparkPlan): Boolean = {
plan match {
- case w: CometScanWrapper => containsNativeCometScan(w.originalPlan)
- case scan: CometScanExec => scan.scanImpl == CometConf.SCAN_NATIVE_COMET
+ case _: CometScanWrapper => true
Review Comment:
and when not scan datafusion
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: add assertion that not using comet scan but using native scan [datafusion-comet]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on PR #1793:
URL: https://github.com/apache/datafusion-comet/pull/1793#issuecomment-2916409442
I thought everything that came from JVM is reusing buffers
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: add assertion that not using comet scan but using native scan [datafusion-comet]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on PR #1793:
URL: https://github.com/apache/datafusion-comet/pull/1793#issuecomment-2918851846
Also, isn't reuse of buffer only part of the reason to do copy? like if the spark side call the arrow release function in the FFI while the `Array` is still being used for example in aggregations
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: add assertion that not using comet scan but using native scan [datafusion-comet]
Posted by "parthchandra (via GitHub)" <gi...@apache.org>.
parthchandra commented on code in PR #1793:
URL: https://github.com/apache/datafusion-comet/pull/1793#discussion_r2109601362
##########
spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala:
##########
@@ -2406,19 +2406,19 @@ object QueryPlanSerde extends Logging with CometExprShim {
// TODO this could be optimized more to stop walking the tree on hitting
// certain operators such as join or aggregate which will copy batches
- def containsNativeCometScan(plan: SparkPlan): Boolean = {
+ def containsNonDataFusionScan(plan: SparkPlan): Boolean = {
plan match {
- case w: CometScanWrapper => containsNativeCometScan(w.originalPlan)
- case scan: CometScanExec => scan.scanImpl == CometConf.SCAN_NATIVE_COMET
+ case _: CometScanWrapper => true
+ case scan: CometScanExec => scan.scanImpl != CometConf.SCAN_NATIVE_DATAFUSION
case _: CometNativeScanExec => false
- case _ => plan.children.exists(containsNativeCometScan)
+ case _ => plan.children.isEmpty || plan.children.exists(containsNonDataFusionScan)
Review Comment:
> ScanExec of rust and not ScanExec of DataFusion
what does this mean?
##########
native/core/src/execution/planner.rs:
##########
@@ -956,6 +956,12 @@ impl PhysicalPlanner {
self.create_expr(filter.predicate.as_ref().unwrap(), child.schema())?;
let filter: Arc<dyn ExecutionPlan> = if filter.use_datafusion_filter {
+ assert_eq!(
+ scans.len(),
+ 0,
+ "Must not use ScanExec as it may requires a copy"
+ );
+
Review Comment:
That is correct. `native_iceberg_compat` does not reuse buffers.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: add assertion that not using comet scan but using native scan [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #1793:
URL: https://github.com/apache/datafusion-comet/pull/1793#issuecomment-2917789948
> I still think there is a bug here:
>
> For this test (when running on main):
>
> ```scala
> test("debug datafusion native filter") {
> val schema = StructType(
> Seq(
> StructField("row_idx", IntegerType, nullable = false),
> StructField("int", IntegerType, nullable = false)))
>
> val data = DataGenerator.DEFAULT.generateRows(1000, schema)
>
> withSQLConf(
> CometConf.COMET_EXPLAIN_VERBOSE_ENABLED.key -> "true",
> CometConf.COMET_EXPLAIN_NATIVE_ENABLED.key -> "true",
> CometConf.COMET_SPARK_TO_ARROW_SUPPORTED_OPERATOR_LIST.key -> "RDDScan") {
> val df = spark
> .createDataFrame(spark.sparkContext.parallelize(data, 1), schema)
> .where(col("row_idx") < 10000 || col("row_idx") > 10010)
>
> df.explain(true)
> df
> .show()
> }
> }
> ```
>
> The spark plan is:
>
> ```
> == Parsed Logical Plan ==
> 'Filter (('row_idx < 10000) OR ('row_idx > 10010))
> +- LogicalRDD [row_idx#2, int#3], false
>
> == Analyzed Logical Plan ==
> row_idx: int, int: int
> Filter ((row_idx#2 < 10000) OR (row_idx#2 > 10010))
> +- LogicalRDD [row_idx#2, int#3], false
>
> == Optimized Logical Plan ==
> Filter ((row_idx#2 < 10000) OR (row_idx#2 > 10010))
> +- LogicalRDD [row_idx#2, int#3], false
>
> == Physical Plan ==
> *(2) CometColumnarToRow
> +- CometFilter [row_idx#2, int#3], ((row_idx#2 < 10000) OR (row_idx#2 > 10010))
> +- CometSparkRowToColumnar
> +- *(1) Scan ExistingRDD[row_idx#2,int#3]
> ```
>
> and the datafusion plan is:
>
> ```
> 25/05/28 19:17:14 INFO core/src/execution/jni_api.rs: Comet native query plan:
> FilterExec: col_0@0 < 10000 OR col_0@0 > 10010
> ScanExec: source=[CometSparkRowToColumnar (unknown)], schema=[col_0: Int32, col_1: Int32]
> ```
>
> It is using DataFusion Filter and not CometFilter while it should use comet filter as there is reuse, no?
In this example, Spark (not Comet) is performing the scan. Comet is then performing the row-to-columnar conversion. The `native_comet` scan is not being used so there is no need to use Comet's filter.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: add assertion that not using comet scan but using native scan [datafusion-comet]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on code in PR #1793:
URL: https://github.com/apache/datafusion-comet/pull/1793#discussion_r2109560295
##########
spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala:
##########
@@ -2406,19 +2406,19 @@ object QueryPlanSerde extends Logging with CometExprShim {
// TODO this could be optimized more to stop walking the tree on hitting
// certain operators such as join or aggregate which will copy batches
- def containsNativeCometScan(plan: SparkPlan): Boolean = {
+ def containsNonDataFusionScan(plan: SparkPlan): Boolean = {
plan match {
- case w: CometScanWrapper => containsNativeCometScan(w.originalPlan)
- case scan: CometScanExec => scan.scanImpl == CometConf.SCAN_NATIVE_COMET
+ case _: CometScanWrapper => true
Review Comment:
and when not scan datafusion so it will not use df filter when compat iceberg
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: add assertion that not using comet scan but using native scan [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #1793:
URL: https://github.com/apache/datafusion-comet/pull/1793#discussion_r2109546495
##########
native/core/src/execution/planner.rs:
##########
@@ -956,6 +956,12 @@ impl PhysicalPlanner {
self.create_expr(filter.predicate.as_ref().unwrap(), child.schema())?;
let filter: Arc<dyn ExecutionPlan> = if filter.use_datafusion_filter {
+ assert_eq!(
+ scans.len(),
+ 0,
+ "Must not use ScanExec as it may requires a copy"
+ );
+
Review Comment:
My understanding is that we only reuse buffers when using `native_comet` to read the Parquet files. If `native_iceberg_compat` is used then we will still have a `ScanExec` reading from JVM but it is safe to use DataFusion's FilterExec in this case.
@parthchandra would you agree with this?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: add assertion that not using comet scan but using native scan [datafusion-comet]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on PR #1793:
URL: https://github.com/apache/datafusion-comet/pull/1793#issuecomment-2918699562
Now I'm **really** confused, where in the code there is reuse of buffers?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: add assertion that not using comet scan but using native scan [datafusion-comet]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on PR #1793:
URL: https://github.com/apache/datafusion-comet/pull/1793#issuecomment-2916913507
I still think there is a bug here:
For this test (when running on main):
```scala
test("debug datafusion native filter") {
val schema = StructType(
Seq(
StructField("row_idx", IntegerType, nullable = false),
StructField("int", IntegerType, nullable = false)))
val data = DataGenerator.DEFAULT.generateRows(1000, schema)
withSQLConf(
CometConf.COMET_EXPLAIN_VERBOSE_ENABLED.key -> "true",
CometConf.COMET_EXPLAIN_NATIVE_ENABLED.key -> "true",
CometConf.COMET_SPARK_TO_ARROW_SUPPORTED_OPERATOR_LIST.key -> "RDDScan") {
val df = spark
.createDataFrame(spark.sparkContext.parallelize(data, 1), schema)
.where(col("row_idx") < 10000 || col("row_idx") > 10010)
df.explain(true)
df
.show()
}
}
```
The spark plan is:
```
== Parsed Logical Plan ==
'Filter (('row_idx < 10000) OR ('row_idx > 10010))
+- LogicalRDD [row_idx#2, int#3], false
== Analyzed Logical Plan ==
row_idx: int, int: int
Filter ((row_idx#2 < 10000) OR (row_idx#2 > 10010))
+- LogicalRDD [row_idx#2, int#3], false
== Optimized Logical Plan ==
Filter ((row_idx#2 < 10000) OR (row_idx#2 > 10010))
+- LogicalRDD [row_idx#2, int#3], false
== Physical Plan ==
*(2) CometColumnarToRow
+- CometFilter [row_idx#2, int#3], ((row_idx#2 < 10000) OR (row_idx#2 > 10010))
+- CometSparkRowToColumnar
+- *(1) Scan ExistingRDD[row_idx#2,int#3]
```
and the datafusion plan is:
```
25/05/28 19:17:14 INFO core/src/execution/jni_api.rs: Comet native query plan:
FilterExec: col_0@0 < 10000 OR col_0@0 > 10010
ScanExec: source=[CometSparkRowToColumnar (unknown)], schema=[col_0: Int32, col_1: Int32]
```
It is using DataFusion Filter and not CometFilter while it should use comet filter as there is reuse, no?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: add assertion that not using comet scan but using native scan [datafusion-comet]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on code in PR #1793:
URL: https://github.com/apache/datafusion-comet/pull/1793#discussion_r2109431765
##########
spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala:
##########
@@ -2406,19 +2406,19 @@ object QueryPlanSerde extends Logging with CometExprShim {
// TODO this could be optimized more to stop walking the tree on hitting
// certain operators such as join or aggregate which will copy batches
- def containsNativeCometScan(plan: SparkPlan): Boolean = {
+ def containsNonDataFusionScan(plan: SparkPlan): Boolean = {
plan match {
- case w: CometScanWrapper => containsNativeCometScan(w.originalPlan)
- case scan: CometScanExec => scan.scanImpl == CometConf.SCAN_NATIVE_COMET
+ case _: CometScanWrapper => true
+ case scan: CometScanExec => scan.scanImpl != CometConf.SCAN_NATIVE_DATAFUSION
case _: CometNativeScanExec => false
- case _ => plan.children.exists(containsNativeCometScan)
+ case _ => plan.children.isEmpty || plan.children.exists(containsNonDataFusionScan)
Review Comment:
exists return false for empty children as it means that this is the data source, right? and if so it using the ScanExec of rust and not ScanExec of DataFusion
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: add assertion that not using comet scan but using native scan [datafusion-comet]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on code in PR #1793:
URL: https://github.com/apache/datafusion-comet/pull/1793#discussion_r2111820738
##########
spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala:
##########
@@ -2406,19 +2406,19 @@ object QueryPlanSerde extends Logging with CometExprShim {
// TODO this could be optimized more to stop walking the tree on hitting
// certain operators such as join or aggregate which will copy batches
- def containsNativeCometScan(plan: SparkPlan): Boolean = {
+ def containsNonDataFusionScan(plan: SparkPlan): Boolean = {
plan match {
- case w: CometScanWrapper => containsNativeCometScan(w.originalPlan)
- case scan: CometScanExec => scan.scanImpl == CometConf.SCAN_NATIVE_COMET
+ case _: CometScanWrapper => true
+ case scan: CometScanExec => scan.scanImpl != CometConf.SCAN_NATIVE_DATAFUSION
case _: CometNativeScanExec => false
- case _ => plan.children.exists(containsNativeCometScan)
+ case _ => plan.children.isEmpty || plan.children.exists(containsNonDataFusionScan)
Review Comment:
we must start from a scan, right? either [`ScanExec`](https://github.com/apache/datafusion-comet/blob/dbf2fb77b76e3f0683ab5a9cf612d20f5f67a53a/native/core/src/execution/operators/scan.rs) or DataFusion [`DataSourceExec`](https://github.com/apache/datafusion-comet/blob/bc92f6e50bf1e088992e92481504c0de25e506a5/native/core/src/execution/planner.rs#L1188) so if no children this means that we are at the root, for example `ExistingRDD` and therefore we must use `ScanExec` as otherwise we would have converted to `CometNativeScanExec`
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: add assertion that not using comet scan but using native scan [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #1793:
URL: https://github.com/apache/datafusion-comet/pull/1793#issuecomment-2919558936
> Now I'm **really** confused, where in the code there is reuse of buffers?
The key structs are `MutableBuffer` and `ParquetMutableBuffer`.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: add assertion that not using comet scan but using native scan [datafusion-comet]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton closed pull request #1793: chore: add assertion that not using comet scan but using native scan
URL: https://github.com/apache/datafusion-comet/pull/1793
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: add assertion that not using comet scan but using native scan [datafusion-comet]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on PR #1793:
URL: https://github.com/apache/datafusion-comet/pull/1793#issuecomment-2912824266
There are a lot of failures, I think I found the error, pushing a fix
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: add assertion that not using comet scan but using native scan [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #1793:
URL: https://github.com/apache/datafusion-comet/pull/1793#issuecomment-2916340347
Thanks @rluvaton but this PR does not appear to help with the CI issue (the tests are still failing - see https://github.com/apache/datafusion-comet/actions/runs/15278587551/job/42973846520?pr=1793).
The scans in Comet are pretty confusing. In the native plan we either have DataFusion's `DataSourceExec`, or a Comet `ScanExec` that reads data from the JVM. The source of data for `ScanExec` could be a broadcast or shuffle exchange, or a Parquet scan either using `native_comet` or `native_iceberg_compat`. When using `native_comet` there is buffer re-use and that is when we need to use Comet's `FilterExec`.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org