schenksj opened a new issue, #5378:
URL: https://github.com/apache/datafusion-comet/issues/5378

   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).


-- 
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]

Reply via email to