-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[datafusion] Update to the latest version #1673
Conversation
e08b202
to
6a50dcc
Compare
Needs a bit more testing. |
@igalshilman let me know when I should take a look at it. |
@tillrohrmann you can if you like, I'm now testing the CLI queries with it |
6a50dcc
to
a167a27
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for upgrading our DF dependencies @igalshilman :-) The changes look good to me. +1 for merging.
// hash_join | ||
// .left() | ||
// .properties() | ||
// .output_ordering() | ||
// .map(|s| s.to_vec()), | ||
// hash_join | ||
// .right() | ||
// .properties() | ||
// .output_ordering() | ||
// .map(|s| s.to_vec()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be removed or do we want to keep it for documentation purposes?
} | ||
|
||
fn name(&self) -> &str { | ||
"join_rewrite" | ||
} | ||
|
||
fn schema_check(&self) -> bool { | ||
true | ||
false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity: What is changing the schema so that we need to disable the schema check?
@igalshilman what were the problems you ran into regarding joins? We have encountered a problem with a Datafusion dependency which would be solved by upgrading our Datafusion dependency (see #1749 for more details). |
a167a27
to
7c92b6f
Compare
edf5bfd
to
12fae09
Compare
Updated data fusion to the latest version as part of #1655
Had to also update arrow-flight and transitively tonic.