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

Explore Updating VariadicAny Signature to take 0 Args #11522

Open
tshauck opened this issue Jul 17, 2024 · 3 comments
Open

Explore Updating VariadicAny Signature to take 0 Args #11522

tshauck opened this issue Jul 17, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@tshauck
Copy link
Contributor

tshauck commented Jul 17, 2024

Is your feature request related to a problem or challenge?

Currently VariadicAny represents 1 or more arguments, but given the name and seemingly intention, it seems like 0 or more would make more sense (though there a number of counter examples).

See https://github.com/apache/datafusion/pull/11229/files#r1680888279 for more discussion.

Describe the solution you'd like

VariadicAny is updated to support 0 arguments.

Describe alternatives you've considered

See the linked discussion for a few alternatives. It's unclear if VariadicAny w/ 0 args is the best option here, but was an issue implementing COUNT().

Additional context

/// One or more arguments with arbitrary types
VariadicAny,

@tshauck tshauck added the enhancement New feature or request label Jul 17, 2024
@2010YOUY01
Copy link
Contributor

Does that work for your use case? I vaguely remember VariadicAny is set to >=1 argument for some reason 🤔

/// # Examples
/// Function `make_array` takes 0 or more arguments with arbitrary types, its `TypeSignature`
/// is `OneOf(vec![Any(0), VariadicAny])`.

@tshauck
Copy link
Contributor Author

tshauck commented Jul 18, 2024

That's actually what I did do in

signature: Signature::one_of(
// TypeSignature::Any(0) is required to handle `Count()` with no args
vec![TypeSignature::VariadicAny, TypeSignature::Any(0)],
Volatility::Immutable,
),
-- it was more the discussion in https://github.com/apache/datafusion/pull/11229/files#r1680888279. To your point, I took a quick look at duckdb and they also represent their variadic any as >= 1, so maybe there is a good reason, and if someone can remember it (or rediscover it), it could be good to document.

@jayzhan211
Copy link
Contributor

jayzhan211 commented Aug 8, 2024

The reason is that we currently use VariadicAny for functions that expect at least one args. If we now accept empty args for VariadicAny, we need to do tons of non-zero length check for them. functions that accept 0 is little, therefore adding Any(0) is easy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants