apache / apache/datafusion-comet
Move contrib scan proto messages out of core operator.proto into contrib/<name>/proto
- Dominant language
- Scala
- Stars
- 1.3k
- Forks
- 373
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 198
Description
Follow-up from review of #4952 ([thread](https://github.com/apache/datafusion-comet/pull/4952#discussion_r_operator_proto)).
### Background
#4952 adds a permanent, format-agnostic extension point to core's operator proto:
```proto
// in message Operator, oneof op_struct:
ContribScan contrib_scan = 200;
// A google.protobuf.Any-shaped envelope
message ContribScan {
string type_url = 1; // e.g. type.googleapis.com/comet.contrib.delta.DeltaScan
bytes value = 2; // the serialized contrib scan message
}
```
Core's oneof never grows per format again, and the native side dispatches purely on `type_url`, so core names no contrib.
What is *not* yet done: the concrete contrib scan messages (`DeltaScan`, `DeltaScanCommon`, `DeltaScanTask`, ...) still physically live in `native/proto/src/proto/operator.proto`. They are inert there — nothing in core references them, and they are only decoded inside a `#[cfg(feature = "contrib-delta")]` module — but core's `.proto` should not carry a contrib's message definitions at all.
### Ask
Relocate contrib scan messages into per-contrib proto files, e.g. `contrib/delta/proto/delta_scan.proto`, leaving only the `ContribScan` envelope in core.
### Why it was deferred
It needs a proto build pipeline that does not exist for contribs today:
- **Rust:** `contrib/delta/native` currently borrows the generated types from `datafusion-comet-proto`. Owning its own messages means standing up `prost-build` in the contrib crate.
- **JVM:** the Maven `protoc-jar-maven-plugin` invocation is configured in core; a contrib would need its own (likely manual) invocation, packaged only under its Maven profile.
This is shared infrastructure — Lance (#4633) will want the same thing — so it is better as its own change than folded into #4952.
### Not a wire-compat concern
Dispatch is keyed on a **contrib-owned** type name (`comet.contrib.delta.DeltaScan`), deliberately *not* on the message's current proto package (`spark.spark_operator`). The identifier reflects who owns the message, not where the `.proto` file sits, so the relocation is invisible on the wire.
---
🤖 Filed with [Claude Code](https://claude.com/claude-code).
Contributor guide
Research direction
Start with native/proto/src/proto/operator.proto, contrib/delta/native, and the core Maven protoc-jar-maven-plugin configuration. Trace how generated types are currently shared, then compare the required contrib/delta/proto/delta_scan.proto layout and contrib-specific Rust and Maven builds. Done means contrib messages are generated and packaged from contrib-owned proto files while core retains only the ContribScan envelope.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust, scala
- Domain
- build-system
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100