eliaperantoni commented on issue #14432: URL: https://github.com/apache/datafusion/issues/14432#issuecomment-2757082084
@Chen-Yuan-Lai I only filed the ticket, but it's @jsai28 working on that feature :) You're definitely right though: these two tickets do have some overlap and we should make sure the resulting API makes sense overall. In my personal view, #15276 is more about giving custom functions the ability to _optionally_ implement a `diagnose` function and _optionally_ use `Diagnostic`s in `invoke_with_args` to make pretty errors. But I think Datafusion should be able to return `Diagnostic`-ed errors for "wrong number of arguments" even for all existing functions whose `Signature` already brings the necessary information; without requiring them to implement `diagnose`. For example, the `log` function already today returns an error if you call it with three arguments:  The goal of this ticket is to make similar errors `Diagnostic`-ed. --- I think this ticket actually overlaps more with #14431 than with #15276 so perhaps you and @dentiny could align on it? 😊 The difficulty lies in `Signature::OneOf` I think. For example `log` accepts `[Float]` and `[Float, Float]`. So the error Datafusion returns today for `[Int]` is simply that it didn't match any of the possibilities. While that's correct, perhaps it's not the best UX. Maybe that error message should be a fallback, more than anything. Proposal: - If the actual arguments don't match the length of any possibility (e.g. `log(1,2,3)`, then we error out with "log cannot take 3 arguments". - If the actual arguments do match in length one of the possibility, but the types are misaligned (e.g. `log('foo')`) then we error out with "log cannot take argument types [utf8]" Does that make sense? Sorry I realize that this must be very confusing. In general it's going to be a bit difficult to map the concept of "arguments don't match any of the available signatures" to a nicer user message. My proposal is not really a complete plan but more of a spark to ignite a discussion together with @Chen-Yuan-Lai and @dentiny so we can find a good solution ❤️ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
