Skip to content
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

fix: use correct arguments for Ibis' *trim expressions #154

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

ingomueller-net
Copy link
Contributor

@ingomueller-net ingomueller-net commented Dec 11, 2024

This PR fixes the usage of Ibis' *trim functions, which do not take any arguments but always remove whitespace instead. Unfortunately, this only changes the error that is produced since *trim is currently translated to Substrait correctly (see
ibis-project/ibis-substrait#1224).

This PR fixes the usage of Ibis` `*trim` functions, which do not take
any arguments but always remove whitespace instead. Unfortunately, this
only changes the error that is produced since `*trim` is currently
translated to Substrait correctly (see
ibis-project/ibis-substrait#1224).

Signed-off-by: Ingo Müller <ingomueller@google.com>
Copy link
Member

@EpsilonPrime EpsilonPrime left a comment

Choose a reason for hiding this comment

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

There's a PR in core to add the signature that Ibis uses for trim. It doesn't really matter here that trim is tested thoroughly (relation testing is more central to consumer-testing) as BFT handles all of the different trim variants.

So this works for me.

@ingomueller-net ingomueller-net merged commit 0f60e6b into substrait-io:main Dec 11, 2024
10 checks passed
@ingomueller-net ingomueller-net deleted the fix-ibis-trim branch December 11, 2024 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants