Skip to content

Conversation

@andygrove
Copy link
Member

Which issue does this PR close?

Closes #2911

Rationale for this change

I would like integration tests in the optimizer crate where we can easily add SQL and expected output plans so we can catch regressions as new rules are added

What changes are included in this PR?

Single integration test as a template to follow

Are there any user-facing changes?

No

@github-actions github-actions bot added the optimizer Optimizer rules label Jul 14, 2022
@andygrove
Copy link
Member Author

@avantgardnerio This is a very rushed and hacky start, but do you think this could be the basis for useful tests?

I have benchmark SQL that I could add here to expose a number of existing optimizer bugs.

@andygrove andygrove marked this pull request as draft July 14, 2022 19:00
Copy link
Contributor

@avantgardnerio avantgardnerio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is useful, but I can see how one could go mad trying to debug t1.c0 = t2.c1. Do you think it would be easier to test against a known schema like in b2a0842 ?

@alamb
Copy link
Contributor

alamb commented Jul 15, 2022

I would like integration tests in the optimizer crate where we can easily add SQL and expected output plans so we can catch regressions as new rules are added

This would be awesome -- note I have something similar in IOx that has the queries and expected plans in external files (which makes updating them much easier -- you can just copy the new output file and verify the diff).

if you would like I can try and port some of that to datafusion (perhaps after this PR is merged): https://github.com/influxdata/influxdb_iox/tree/main/query_tests

@andygrove
Copy link
Member Author

@alamb that sounds great. Thank you.

@alamb
Copy link
Contributor

alamb commented Jul 26, 2022

@alamb that sounds great. Thank you.

I am a bit low on time for the moment -- I will try over the next few weeks (or maybe document / explain better the existing postgres integration test that @jimexist worked on)

@andygrove
Copy link
Member Author

I needed this style of integration test in #3229 so made the changes in that PR instead.

@andygrove andygrove closed this Aug 23, 2022
@andygrove andygrove deleted the optimizer-tests branch August 23, 2022 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

optimizer Optimizer rules

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create integration tests for optimizer crate

3 participants