apache / apache/datafusion-comet

Move contrib scan proto messages out of core operator.proto into contrib/<name>/proto

Open
#5,378 0 comments 0 reactions 0 assignees View on GitHub
area:scan enhancement
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.