You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "revans2 (via GitHub)" <gi...@apache.org> on 2023/03/22 15:51:01 UTC
[GitHub] [spark] revans2 opened a new pull request, #40524: [SPARK-42898][SQL] Mark that string/date casts do not need time zone id
revans2 opened a new pull request, #40524:
URL: https://github.com/apache/spark/pull/40524
### What changes were proposed in this pull request?
This removes the need for a time zone id when casting from StringType -> DateType and DateType -> StringType.
### Why are the changes needed?
It is mostly for consistency with what the code is actually doing. Marking it as needing A time zone id has no real impact to the actual execution.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
I just compiled it for now. I assume that the existing tests will cover it.
--
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
[GitHub] [spark] yaooqinn closed pull request #40524: [SPARK-42898][SQL] Mark that string/date casts do not need time zone id
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn closed pull request #40524: [SPARK-42898][SQL] Mark that string/date casts do not need time zone id
URL: https://github.com/apache/spark/pull/40524
--
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
[GitHub] [spark] pan3793 commented on pull request #40524: [SPARK-42898][SQL] Mark that string/date casts do not need time zone id
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #40524:
URL: https://github.com/apache/spark/pull/40524#issuecomment-1577853486
@revans2 IMO the test failure makes sense, we just need to change it to another timezone-aware type to save the UT coverage.
--
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
[GitHub] [spark] yaooqinn commented on pull request #40524: [SPARK-42898][SQL] Mark that string/date casts do not need time zone id
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #40524:
URL: https://github.com/apache/spark/pull/40524#issuecomment-1639664075
Hi @revans2, is there any update on this issue?
--
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
[GitHub] [spark] yaooqinn commented on pull request #40524: [SPARK-42898][SQL] Mark that string/date casts do not need time zone id
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #40524:
URL: https://github.com/apache/spark/pull/40524#issuecomment-1643292939
cc @ulysses-you who is the last one to touch this part
--
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
[GitHub] [spark] gerashegalov commented on pull request #40524: [SPARK-42898][SQL] Mark that string/date casts do not need time zone id
Posted by "gerashegalov (via GitHub)" <gi...@apache.org>.
gerashegalov commented on PR #40524:
URL: https://github.com/apache/spark/pull/40524#issuecomment-1480055444
LGTM, I would just add a unit test to CastSuite to prevent regressions
--
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
[GitHub] [spark] revans2 commented on pull request #40524: [SPARK-42898][SQL] Mark that string/date casts do not need time zone id
Posted by "revans2 (via GitHub)" <gi...@apache.org>.
revans2 commented on PR #40524:
URL: https://github.com/apache/spark/pull/40524#issuecomment-1640733683
@yaooqinn Sorry I keep meaning to get back to this, but it is not high up on my priority list. If someone else wants to take it across the finish line I am happy to let them. If not I'll see when I can find some time to do it.
--
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
[GitHub] [spark] ulysses-you commented on pull request #40524: [SPARK-42898][SQL] Mark that string/date casts do not need time zone id
Posted by "ulysses-you (via GitHub)" <gi...@apache.org>.
ulysses-you commented on PR #40524:
URL: https://github.com/apache/spark/pull/40524#issuecomment-1643307262
After offline discussion, @pan3793 will try to take over this pr. Thank you all!
--
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
[GitHub] [spark] revans2 commented on pull request #40524: [SPARK-42898][SQL] Mark that string/date casts do not need time zone id
Posted by "revans2 (via GitHub)" <gi...@apache.org>.
revans2 commented on PR #40524:
URL: https://github.com/apache/spark/pull/40524#issuecomment-1483436510
@MaxGekk This might take a little while. I don't want to delete the test, but I also cannot just switch the data types over to Timestamp because the partition string is not always stored in a format that is compatible with TimestampType. So I ma going to have to spend some more time understanding the test to see if there is a good way to update it for TimestampType.
--
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
[GitHub] [spark] pan3793 commented on pull request #40524: [SPARK-42898][SQL] Mark that string/date casts do not need time zone id
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #40524:
URL: https://github.com/apache/spark/pull/40524#issuecomment-1643403755
I opened https://github.com/apache/spark/pull/42089 to take over this PR, please take a look if you have time, thanks.
--
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