You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "edmondop (via GitHub)" <gi...@apache.org> on 2024/08/02 13:01:52 UTC
[I] Align UDF name to lowercase or uppercase [datafusion]
edmondop opened a new issue, #11779:
URL: https://github.com/apache/datafusion/issues/11779
### Describe the bug
As a part of performing [this](https://github.com/apache/datafusion/pull/11013) change, @jayzhan211 recommended to use `min` and `max` as a default name for the new UDFs rather than the lowercase. However, performing this change makes test fails because in several places we use MAX and MIN, for example:
https://github.com/apache/datafusion/blob/0332eb569a5428ac385fe892ce7b5fb40d52c8c0/datafusion/sqllogictest/test_files/tpch/q15.slt.part use MAX uppercase that would cause build failure when comparing LogicalPlan like in the following build:
https://github.com/apache/datafusion/actions/runs/10215600305/job/28265554129?pr=11013
In the rust tests as well we use `avg` but `MAX`, so we need to update those tests as well
https://github.com/apache/datafusion/blob/0332eb569a5428ac385fe892ce7b5fb40d52c8c0/datafusion/core/src/dataframe/mod.rs#L2059
### To Reproduce
_No response_
### Expected behavior
_No response_
### Additional context
I have not verified if MAX and MIN are the only UDF for which we use uppercase rather than lowercase. I personally prefer to have all SQL KEYWORDS (including UDF names) uppercase for readability
--
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-unsubscribe@datafusion.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Align UDF name to lowercase or uppercase [datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on issue #11779:
URL: https://github.com/apache/datafusion/issues/11779#issuecomment-2265371418
Sadly this is the last name to be lowercase, others are already converted. I don't have strong preference on upper case or lower case as long as they are all consistent
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Align UDF names to lowercase or uppercase [datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on issue #11779:
URL: https://github.com/apache/datafusion/issues/11779#issuecomment-2265423848
> Agreed,I think it's better to read
>
> SELECT MAX(stuff) rather than SELECT max(stuff) and keep the lowercase for columns and non-keyword
The lowercase/uppercase conversion is quite painful since you need to change many places. Especially for dataframe test
It is worth to find out an easy way to update the dataframe test beforehand https://github.com/apache/datafusion/issues/10373
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Align UDF names to lowercase or uppercase [datafusion]
Posted by "edmondop (via GitHub)" <gi...@apache.org>.
edmondop closed issue #11779: Align UDF names to lowercase or uppercase
URL: https://github.com/apache/datafusion/issues/11779
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Align UDF name to lowercase or uppercase [datafusion]
Posted by "edmondop (via GitHub)" <gi...@apache.org>.
edmondop commented on issue #11779:
URL: https://github.com/apache/datafusion/issues/11779#issuecomment-2265379132
Agreed,I think it's better to read
SELECT MAX(stuff) rather than SELECT max(stuff) and keep the lowercase for columns and non-keyword
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org