You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "mihailom-db (via GitHub)" <gi...@apache.org> on 2024/03/25 09:59:38 UTC
[PR] [SPARK-47504][SQL][COLLATION] Resolve AbstractDataType simpleStrings for StringTypeCollated [spark]
mihailom-db opened a new pull request, #45694:
URL: https://github.com/apache/spark/pull/45694
### What changes were proposed in this pull request?
Renaming simpleString in StringTypeAnyCollation.
### Why are the changes needed?
[SPARK-47296](https://issues.apache.org/jira/browse/SPARK-47296) introduced a change to fail all unsupported functions. Because of this change expected inputTypes in ExpectsInputTypes had to be changed. This change introduced a change on user side which will print "STRING_ANY_COLLATION" in places where before we printed "STRING" when an error occurred. Concretely if we get an input of Int where StringTypeAnyCollation was expected, we will throw this faulty message for users.
### Does this PR introduce _any_ user-facing change?
Yes
### How was this patch tested?
Existing tests were changed back to "STRING" notation.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47504][SQL] Resolve AbstractDataType simpleStrings for StringTypeCollated [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #45694:
URL: https://github.com/apache/spark/pull/45694#issuecomment-2043136877
thanks, merging to master!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47504][SQL][COLLATION] Resolve AbstractDataType simpleStrings for StringTypeCollated [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #45694:
URL: https://github.com/apache/spark/pull/45694#discussion_r1538490960
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/CollationTypeConstraints.scala:
##########
@@ -53,6 +53,7 @@ abstract class StringTypeCollated extends AbstractDataType {
* Use StringTypeBinary for expressions supporting only binary collation.
*/
case object StringTypeBinary extends StringTypeCollated {
+ // TODO: When this AbstractDataType is first used, think of a proper simpleString name
Review Comment:
Mind filing a JIRA, and make this like:
```
// TODO(SPARK-XXXXX): When this AbstractDataType is first used, think of a proper simpleString name
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47504][SQL] Resolve AbstractDataType simpleStrings for StringTypeCollated [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #45694:
URL: https://github.com/apache/spark/pull/45694#discussion_r1555285066
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/StringTypeCollated.scala:
##########
@@ -30,6 +30,7 @@ abstract class StringTypeCollated extends AbstractDataType {
* Use StringTypeBinary for expressions supporting only binary collation.
*/
case object StringTypeBinary extends StringTypeCollated {
+ // TODO (SPARK-47504): Rename this AbstractDataType's simpleString when it is first used
Review Comment:
can we fix it now as we know it should be `"string"`?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47504][SQL] Resolve AbstractDataType simpleStrings for StringTypeCollated [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan closed pull request #45694: [SPARK-47504][SQL] Resolve AbstractDataType simpleStrings for StringTypeCollated
URL: https://github.com/apache/spark/pull/45694
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47504][SQL] Resolve AbstractDataType simpleStrings for StringTypeCollated [spark]
Posted by "mihailom-db (via GitHub)" <gi...@apache.org>.
mihailom-db commented on PR #45694:
URL: https://github.com/apache/spark/pull/45694#issuecomment-2042815237
@cloud-fan Could we merge this in? So some new PR does not conflict with it again.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org