apache / apache/datafusion-comet

Centralize data-type support predicates so new types don't have to be added in multiple places

Open
#5,021 1 comment 0 reactions 1 assignee Claimed by @peterxcli View on GitHub
enhancement
Dominant language
Scala
Stars
1.3k
Forks
373
Avg merge
2d 4h
Merged PRs (30d)
198

Description

## Background

While reviewing #4976 (interval codegen dispatch for nested values and native shuffle), it became clear that Comet now has several independent data-type support predicates that each had to learn about `YearMonthIntervalType` and `DayTimeIntervalType` separately:

- `supportedSinkDataType` in `spark/src/main/scala/org/apache/comet/serde/operator/CometSink.scala`
- `supportedSerializableDataType` in `spark/src/main/scala/org/apache/spark/sql/comet/execution/shuffle/CometShuffleExchangeExec.scala`
- `isTypeSupported` in `spark/src/main/scala/org/apache/spark/sql/comet/CometLocalTableScanExec.scala`
- `supportedDataType` in `spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala`

In #4976, `supportedSinkDataType` is effectively a copy of `supportedDataType(_, allowComplex = true)` with intervals added at each nesting level. That was the least invasive option for that PR, but it means these predicates can drift out of sync as new types are added.

## Proposal

Consider consolidating so that adding support for a new data type only requires touching one place. Options to explore:

- Give `supportedDataType` an additional flag (for example `allowIntervals`) rather than forking the traversal in `CometSink`.
- Or converge the sink / shuffle / local-scan predicates onto a shared helper with explicit, named capability flags for the differences that genuinely need to vary per boundary (for example, interval file scans staying unsupported while LocalTableScan supports them).

## Notes

This is a maintainability cleanup, not a correctness bug. The predicates are correct today. Filed as follow-up from the #4976 review.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.