Substrait: roundtrip_logical_plan shouldn't optimize plans
- Dominant language
- Rust
- Stars
- 9.3k
- Forks
- 2.4k
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 344
Description
Currently helper functions in `roundtrip_logical_plan.rs` (`roundtrip`, `test_alias` and others) optimize DF plans before substrait conversion and comparison. Because of this some tests aren't actually testing what one would intuitively assume is being tested. For example [between_integers](https://github.com/apache/datafusion/blob/577e4bba0f5838846862621e1f5318c949cff2cb/datafusion/substrait/tests/cases/roundtrip_logical_plan.rs#L352) and [not_between_integers](https://github.com/apache/datafusion/blob/577e4bba0f5838846862621e1f5318c949cff2cb/datafusion/substrait/tests/cases/roundtrip_logical_plan.rs#L361) tests pass even when I tried removing all code handling Between operator from the producer because DF throws away the operator during optimization.
When I tried to simply drop optimization from all comparisons, most of the tests started failing for various reasons so I don't think this can be fixed in one go. I'm thinking of adding alternate "unoptimized" helper functions and porting tests over to them over a number of PRs.
Contributor guide
Research direction
Read datafusion/substrait/tests/cases/roundtrip_logical_plan.rs, focusing on roundtrip, test_alias, and the between_integers and not_between_integers cases. Run the existing roundtrip logical-plan tests to identify failures without optimization, then add alternate unoptimized helpers and port tests over incrementally until comparisons exercise the original plans.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend, testing-qa
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100