apache / apache/datafusion

custom `SubstraitEncoder` cannot convert to `proto::Plan`

Open
#18,266 2 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Rust
Stars
9.3k
Forks
2.4k
Avg merge
3d 7h
Merged PRs (30d)
344

Description

### Describe the bug

The current approach allows to create a Rel plan of the proto kind, but then to create the actual `prost::plan` it requires the `RelRoot` which requires names, which are currently constructed in the DefaultImpl via [`to_substrait_named_struct`](https://github.com/apache/datafusion/blob/22c4214fe1ca3953932f3f12ccd5b68dbfbefdf3/datafusion/substrait/src/logical_plan/producer/types.rs#L349).

### To Reproduce

example code for a producer inspired by [`to_substrait_plan`](https://github.com/apache/datafusion/blob/22c4214fe1ca3953932f3f12ccd5b68dbfbefdf3/datafusion/substrait/src/logical_plan/producer/plan.rs#L27):
```rust

pub fn to_plan(mut self, plan: LogicalPlan) -> anyhow::Result> {
let transformed_plan = self.handle_plan(&plan)?;

let relations = vec![PlanRel {
rel_type: Some(plan_rel::RelType::Root(RelRoot {
input: Some(*transformed_plan),
names: producer::to_substrait_named_struct(&mut self, plan.schema())?.names,
})),
}];

Ok(Box::new(proto::Plan {
version: Some(version::version_with_producer("daedalus")),
extension_uris: vec![],
extensions: self.extensions.into(),
relations,
advanced_extensions: None,
expected_type_urls: vec![],
parameter_bindings: vec![],
}))
}
```

### Expected behavior

`to_substrait_named_struct` can just become public?

### Additional context

in my opinion this is a bug, since it prevents the use of custom substrait to pass down to another execution engine

Contributor guide

Open the contributing guide

Research direction

Start with datafusion/substrait/src/logical_plan/producer/types.rs, where to_substrait_named_struct is defined, and compare its use with to_substrait_plan in producer/plan.rs. Reproduce the custom to_plan example and determine the smallest public API change that lets it construct proto::Plan. Done means a custom SubstraitEncoder can convert a logical plan to proto::Plan without inaccessible internals.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend-api-design
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.