-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Make migration to Signature::nullary
in 44.0.0 easier / less confusing
#13763
Comments
Signature::nullary
easier to findSignature::nullary
easier / less confusing
Signature::nullary
easier / less confusingSignature::nullary
in 44.0.0 easier / less confusing
|
I don't know. It would also be fine if that format kept working as before (maybe better). |
The reason why |
In my opinion the important thing is that someone who has an existing Ideas (I can make PRs for this once we agree) Option 1: Backwards compatibility:
Option 2: Better errors
Option 3: API w/ compile time checksWe could also take a more drastic approach and verify this at compile time by making the compiler reject non zero numbers (I think this API churn is more expensive than the value saves than worth it at the moment): pub enum Signature {
Exact(NonZeroUsize)
...
} |
Option2 seems like a better choice |
Is your feature request related to a problem or challenge?
While working to reproduce the issue from @kylebarron here:
He had this signature for a UDF that has no arguments
This compiled without error / incident
However, when I ran that on latest main I got a very strange (to me) error:
To fix it I found I had to:
I think this will make upgrading to 44 a pain
Describe the solution you'd like
I would like DataFusion to either:
nullary
Describe alternatives you've considered
No response
Additional context
Nullary appears to have been added in this PR by @jayzhan211
The text was updated successfully, but these errors were encountered: