apache / apache/datafusion

Substrait: roundtrip_logical_plan shouldn't optimize plans

Open
#12,798 0 comments 0 reactions 0 assignees View on GitHub
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

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.