apache / apache/datafusion-comet
Fall back instead of asserting when native scan partition values cannot be serialized
- Dominant language
- Scala
- Stars
- 1.3k
- Forks
- 373
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 198
Description
### What is the problem the feature request solves?
`partition2Proto` assumes that accepting a partition schema guarantees every concrete partition value can be serialized:
```scala
val valueProto = exprToProto(Literal(value, attr.dataType), Seq.empty)
assert(
valueProto.isDefined,
s"Unsupported partition value: $value, type: ${attr.dataType}")
valueProto.get
```
That assumption spans separate support checks. `CometScanRule` validates the partition schema through `CometScanTypeChecker`, while `partition2Proto` later relies on `CometLiteral` / `exprToProto` supporting each concrete value. If those capabilities drift, Comet has already selected the native scan and raises `AssertionError` instead of falling back to Spark.
PR #5161 exposed this with `YearMonthIntervalType`: the scan checker admitted the partition type before the literal path supported it. That specific interval gap is fixed by the PR, but the generic assertion remains. There is no currently known unsupported partition literal that passes the gate at the PR head; this issue tracks hardening the invariant before another type exposes it.
The helper is shared by the native Parquet and CSV scan paths.
### Describe the potential solution
- Use one shared capability check for partition-schema admission and partition-literal serialization so the two cannot drift.
- Validate partition-value serialization before committing to the native scan, or otherwise propagate serialization failure to a point where Spark fallback is still possible.
- Record a useful fallback reason instead of surfacing `AssertionError` as an internal failure.
- Add a regression test that deliberately exercises an unsupported partition literal and verifies clean fallback.
### Additional context
- `partition2Proto`: https://github.com/apache/datafusion-comet/blob/2963b0871436766fd14eb15fbe08f57ad50af30e/spark/src/main/scala/org/apache/comet/serde/operator/package.scala#L48-L65
- Parquet caller: https://github.com/apache/datafusion-comet/blob/2963b0871436766fd14eb15fbe08f57ad50af30e/spark/src/main/scala/org/apache/spark/sql/comet/CometNativeScanExec.scala#L215-L228
- CSV caller: https://github.com/apache/datafusion-comet/blob/2963b0871436766fd14eb15fbe08f57ad50af30e/spark/src/main/scala/org/apache/spark/sql/comet/CometCsvNativeScanExec.scala#L95-L105
- PR: https://github.com/apache/datafusion-comet/pull/5161
- Review discussion and original reproduction: https://github.com/apache/datafusion-comet/pull/5161#discussion_r3690464235
Contributor guide
Assessment
This issue has not been assessed yet.