thinkharderdev commented on PR #47: URL: https://github.com/apache/arrow-ballista/pull/47#issuecomment-1142319461
> > @andygrove Why remove the `AsLogicalPlan` type param from the codec? In our project we use a custom logical plan representation (based on a different query AST) and it is very helpful to be able to plugin our own `LogicalPlan` type. > > @thinkharderdev the main goal was to delegate to `datafusion-proto` for logical plan serde so that we don't have to duplicate all of this code in Ballista. A secondary goal was to use the `datafusion-proto` API for converting plans to bytes and bytes to plans without having dependencies on internals of that crate (so that we can easily change the format in the future). If we create a trait for this API does that make it easy for you to plug in your own implementation? Yeah, my understanding is that `AsLogicalPlan` is that trait. So `datafusion-proto` can provide a set of protobuf definitions for serializing a `LogicalPlan` in a "canonical" way and implement the `AsLogicalPlan` trait for those definitions. I think it makes sense to continue using that implementation in Ballista as the default representation and rely on the implementation in `datafusion-proto`. -- 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]
