[GitHub] [spark] cloud-fan commented on issue #24902: [SPARK-28093][SQL] Fix TRIM/LTRIM/RTRIM function parameter order issue
cloud-fan commented on issue #24902: [SPARK-28093][SQL] Fix TRIM/LTRIM/RTRIM function parameter order issue URL: https://github.com/apache/spark/pull/24902#issuecomment-586072349 Thanks @marmbrus for pointing out that this is already discussed officially on the mailing list. I'm trying to build a clear document about how to deal with behavior changes and make it more smooth for users to upgrade. But this may take a while as I need to collect many examples. @dongjoon-hyun shall we move forward to revert this kind of cosmetic changes from 3.0 first? I don't want to block 3.0 until I finish the document. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] cloud-fan commented on issue #24902: [SPARK-28093][SQL] Fix TRIM/LTRIM/RTRIM function parameter order issue
cloud-fan commented on issue #24902: [SPARK-28093][SQL] Fix TRIM/LTRIM/RTRIM function parameter order issue URL: https://github.com/apache/spark/pull/24902#issuecomment-585030960 After checking the codebase, we already support `TRIM ( [ characters FROM ] string )`. My question is: how can we silently change the parameter of an existing SQL function? I don't think this is a correctness issue as the SQL standard doesn't say that the function signature have to be `trim(srcStr, trimStr)`. I agree this was a mistake in the first place, but it has been released and I don't think we can just change it. cc @marmbrus 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] cloud-fan commented on issue #24902: [SPARK-28093][SQL] Fix TRIM/LTRIM/RTRIM function parameter order issue
cloud-fan commented on issue #24902: [SPARK-28093][SQL] Fix TRIM/LTRIM/RTRIM function parameter order issue URL: https://github.com/apache/spark/pull/24902#issuecomment-584511167 I don't see the specification of the TRIM function parameters order in the SQL standard, and I assume it's vendor-specific. I agree that most SQL systems use the new order, but it doesn't mean the old order is wrong. More importantly, the SQL standard does define the TRIM syntax as `TRIM ( [ characters FROM ] string )`, and I think this is the thing we should follow. I'd suggest we revert this commit (how can users figure out the parameter order of a function is changed? TRIM is a common function and this can break many queries silently.). We can implement `TRIM ( [ characters FROM ] string )` in 3.1. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
