jsai28 commented on issue #15276: URL: https://github.com/apache/datafusion/issues/15276#issuecomment-2742070297
Regarding your first two points, I do think that `Vec<Spans>` may be the way to do this. Mainly as it supports handling the case of literal values out of the box. If sqlparser is eventually updated to support literals, then wrapping the `Span` with `Option` would have been unnecessary extra steps. Also, if in the future we need multiple `Span`s, then we would probably need to make significant changes to this API with the `Vec<Option<Span>>` implementation. So that's why I'm leaning towards `Vec<Spans>`. I'm also thinking that if we wanted more sophisticated span handling in the future (like maybe the ability to prioritize certain errors?), then `Vec<Spans>` might make it easier. Although I haven't fully thought about how that could work. For your third point, yes `return_type_from_args` makes more sense. > @alamb suggested using this new feature by creating a few Diagnostic in some of the built-in functions you see in datafusion::functions* so that this API is tested and provides value Ah I see what you mean. So we will first implement Diagnostic for a few functions in `datafusion::functions*`, see how it works, and then eventually create another ticket to add it to the rest maybe? -- 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
