andygrove opened a new pull request, #5111: URL: https://github.com/apache/datafusion-comet/pull/5111
## Which issue does this PR close? N/A — found while investigating why `SampleExec` appeared to be undocumented. ## Rationale for this change `operators.md` opens with "This page is the complete reference for how Apache Comet handles each Spark physical operator." It named 38 of the 180 concrete operators in `org.apache.spark.sql.execution` (union across the spark-3.4 through spark-4.2 profiles). That is worse than an incomplete page: an operator with no row reads as supported to anyone scanning the tables. `SampleExec` is the case that prompted this — it *was* mentioned, but only inside a prose bullet under "Not currently planned", so it did not show up when scanning for a table entry. ## What changes are included in this PR? ### Operators added - **Typed `Dataset[T]` API** — `MapPartitionsExec`, `MapElementsExec`, `MapGroupsExec`, `CoGroupExec`, `AppendColumnsExec`, and the object serde operators. Common in user code and previously absent, so there was no hint that `ds.map(...)` takes its whole stage back to Spark. - **Non-Parquet scans** — `RowDataSourceScanExec` (JDBC), `RDDScanExec`, `ExternalRDDScanExec`, `EmptyRelationExec`, `OneRowRelationExec`, `UnionLoopExec`. The scan table had 4 of 11 rows. - **Individually notable** — `CollectMetricsExec` (`df.observe`, which also splits a native stage in two), `CollectTailExec`, `MergingSessionsExec` / `UpdatingSessionsExec` (session windows, batch as well as streaming), `SparkScriptTransformationExec`. - **Writes** — DataSource V2 writes (`AppendDataExec`, `OverwriteByExpressionExec`, …) and row-level MERGE/UPDATE/DELETE plans. Worth stating explicitly given that Iceberg *reads* are documented. - **Python / pandas / R** — the remaining UDF operators, including pandas aggregate and window UDFs, Python UDTFs, `applyInArrow`, `AttachDistributedSequenceExec`, and the SparkR operators. - **Plan infrastructure** — a new section for AQE stage wrappers, transitions, and DPP subqueries. Comet has dedicated rules for these, so a reader who finds one in a plan now has somewhere to look. - **Commands and streaming** — grouped rows so the long tail is covered without 60 individual entries. ### Corrections - The PyArrow UDF row was stale in both directions: `mapInArrow` / `mapInPandas` landed as opt-in behind `spark.comet.exec.pyarrowUdf.enabled` (Spark 4.0+), while `ArrowEvalPythonExec` and `FlatMapGroupsInPandasExec` shared the same 🔜 row and do still fall back. - `WriteFilesExec` is converted together with `DataWritingCommandExec`; only the latter was documented. - Documented that `spark.comet.sparkToColumnar.enabled` can bridge `RangeExec`, `RDDScanExec`, `InMemoryTableScanExec`, and `OneRowRelationExec` into a native stage. The default operator list is not mentioned anywhere in the user guide today. ### Status vocabulary Two statuses replace the old catch-all so the tables do not read as a wall of failures: - ❌ **Not supported** — falls back today, and native execution would be worthwhile. A genuine gap. - ➖ **Not applicable** — falls back by design: catalog commands, streaming, arbitrary JVM closures, driver-side operations, or plan plumbing Comet handles while planning. ### Drift guard `CometOperatorDocSuite` enumerates concrete `SparkPlan` subclasses from the spark-sql jar on the test classpath and fails if any lacks an entry, with a message naming the missing operators. It also asserts the scan found >100 operators so it cannot pass vacuously if the scan breaks. The suite runs under every profile, so the doc covers the union across Spark versions; version-specific operators are annotated in the notes. When a new Spark version adds an operator, this fails with a list of what to document. ## How are these changes tested? `CometOperatorDocSuite` passes under `spark-3.4`, `spark-3.5`, `spark-4.0`, `spark-4.1`, and `spark-4.2`, and under `scala-2.12`. Verified that it fails with a useful message by running it against `spark-4.2` before the V2 view operators were added. Statuses were checked against `CometExecRule.nativeExecs` / `sinks`, `CometScanRule`, `EliminateRedundantTransitions`, and `CometConf` rather than assumed. Draft because the ❌ / ➖ split is a judgement call per operator and worth a second opinion — in particular `SampleExec`, `RowDataSourceScanExec` (JDBC), `CartesianProductExec`, and the SparkR operators, which could reasonably go either way. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
