feat: add support for Ibis 9.x #1034
Conversation
|
Worked around arrow nightly failures by first using duckdb backend's P.S. Still fighting nix about other failures... |
|
Hey @tokoko -- I'm going to take a more thorough look at this soon, but just an FYI that the nix failures are due to an upstream thing that has been fixed but is still propagating down to the various nixos package repos, so we'll just have to wait another day or two for it to resolve itself. |
|
@gforsyth sure, thanks for the info. I think that's the reason for 39 and 310 failures. 311 failure is strange though, duckdb is failing to download substrait extension only when in nix environment, works without a problem in poetry. I'll also look into other failures as well. The windows one is not too surprising :) but I'm not sure what semantic-version is complaining about. Any ideas there? |
Ahh, that's a nix sandboxing thing -- we have test markers in Ibis (upstream) to skip tests that involve extensions when doing a nix build.
Might just need to be updated in CI, let me check |
We no longer require sqlalchemy upstream so we can drop these dependencies that were used for downstream testing here.
Ok, a bump to conventional commits sorted it out. |
Linux+Nix have sandboxing that prevents duckdb extensions from downloading.
|
Hey @tokoko -- I bumped the nix flake and it pulled in the upstream fix, so the nix builds are passing now (with the added xfails of the tests that require installing the duckdb substrait extension on linux) |
|
@gforsyth I saw that, thanks. I'll try to later reorganize tests with fixtures in a way that only duckdb+substrait portion can be skipped instead of entire tests. I think I added a "fix" for windows failure as well. |
|
I tried but substrait extension doesn't seem to work on windows, I'm not even able to download the binary from the browser. Acero seems to support more functionality, it wasn't adding much value anyway, so I went ahead and disabled it from tests for now. |
|
@gforsyth Hey, have you had an opportunity to dig through the changes? |
currently digging! I have a few small changes to add, but nothing big. I think the biggest pieces left are to get in tests for window functions and to figure out if we need to keep |
|
@gforsyth Sweet, sounds good.
This is a bit of a pain as substrait consumer engines I used in parity tests didn't support them. Pretty sure datafusion supports window functions though, just couldn't get datafusion tests working yet.
I sort of convinced myself that we don't, but more tests covering edge cases would be better. |
| "name": "extract:date" | ||
| } | ||
| }, | ||
| { | ||
| "extensionFunction": { | ||
| "extensionUriReference": 3, | ||
| "functionAnchor": 3, | ||
| "name": "multiply:dec_dec" | ||
| } | ||
| }, | ||
| { | ||
| "extensionFunction": { | ||
| "extensionUriReference": 3, | ||
| "functionAnchor": 4, | ||
| "name": "subtract:dec_dec" | ||
| } |
There was a problem hiding this comment.
these scalar functions disappearing is a bit odd -- I'll have to dig in deeper (in a follow-up) to make sure this is working correctly
There was a problem hiding this comment.
Tried following these, but it was too much. couldn't really make sense of the existing plans, at least some of the the functions that disappeared didn't look to be used in original ibis code at all.
Co-authored-by: Gil Forsyth <[email protected]>
|
Added handling for |
BREAKING CHANGE: ibis-substrait no longer supports versions of Ibis < 9.x. If you are using an older version of Ibis, you can pin `ibis-substrait<4` Co-authored-by: Gil Forsyth <[email protected]> Co-authored-by: Gil Forsyth <[email protected]>
test_parity.pychecking execution parity between duckdb (with ibis), duckdb (with substrait) and acero.