goldmedal commented on PR #10661: URL: https://github.com/apache/datafusion/pull/10661#issuecomment-2133291003
> `first_value` is not added in alias because the function is not fully converted to UDAF yet, so it will fail some tests. #10648 has alias for lower case > > so as `approx_median` since it is not UDAF yet. > Thanks for the information! @jayzhan211 > > I think adding a lowercase alias for MEDIAN is a workaround to handle the function name case insensitivity issue for UDAF > > Why adding a lowercase alias is a workaround for UDAF? Without alias, we will need to check the lowercase in many places. Since I thought we needed to list all case patterns for the function name (e.g., `Median`, `MEDian`), I called it a workaround. However, I found I was wrong. After some testing, I discovered something interesting. For DataFusion, if we query a function ``` SELECT Median(xx) FROM tt ``` The function call will be transferred to `median`. Even query it use ``` SELECT MEDIAN(xx) FROM tt ``` It will also be changed to `median`. The uppercase name is only be matched when invoke this function with qutoes ``` SELECT "MEDIAN"(xx) FROM tt ``` So, how about setting UDAF names to lowercase (e.g., `median`, `first_value`)? This way, we don't need another uppercase alias. What do you think? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
