Re: [PR] [VL] Support Cast between TIMESTAMP and TIMESTAMP_NTZ [gluten]
Mariamalmesfer commented on code in PR #12654:
URL: https://github.com/apache/gluten/pull/12654#discussion_r3770127738
##
gluten-substrait/src/main/scala/org/apache/gluten/extension/columnar/validator/Validators.scala:
##
@@ -282,6 +282,8 @@ object Validators {
case Minute(child, _) => containsNTZ(child.dataType)
case Second(child, _) => containsNTZ(child.dataType)
case TimestampAdd(_, _, child, _) =>
containsNTZ(child.dataType)
+ case c: Cast if c.dataType == TimestampType =>
containsNTZ(c.child.dataType)
Review Comment:
Added a non-recursive `isNTZ` helper for the check, used in both `Cast`
guards
--
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]
Re: [PR] [VL] Support Cast between TIMESTAMP and TIMESTAMP_NTZ [gluten]
github-actions[bot] commented on PR #12654: URL: https://github.com/apache/gluten/pull/12654#issuecomment-5272466064 Run Gluten Clickhouse CI on x86 -- 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]
Re: [PR] [VL] Support Cast between TIMESTAMP and TIMESTAMP_NTZ [gluten]
rui-mo commented on code in PR #12654:
URL: https://github.com/apache/gluten/pull/12654#discussion_r3765702179
##
gluten-substrait/src/main/scala/org/apache/gluten/extension/columnar/validator/Validators.scala:
##
@@ -282,6 +282,8 @@ object Validators {
case Minute(child, _) => containsNTZ(child.dataType)
case Second(child, _) => containsNTZ(child.dataType)
case TimestampAdd(_, _, child, _) =>
containsNTZ(child.dataType)
+ case c: Cast if c.dataType == TimestampType =>
containsNTZ(c.child.dataType)
Review Comment:
The `containsNTZ` would also allow complex types which contain TimestampNTZ
type, but those cases haven't been verified.
##
gluten-substrait/src/main/scala/org/apache/gluten/extension/columnar/validator/Validators.scala:
##
@@ -282,6 +282,8 @@ object Validators {
case Minute(child, _) => containsNTZ(child.dataType)
case Second(child, _) => containsNTZ(child.dataType)
case TimestampAdd(_, _, child, _) =>
containsNTZ(child.dataType)
+ case c: Cast if c.dataType == TimestampType =>
containsNTZ(c.child.dataType)
+ case c: Cast if containsNTZ(c.dataType) => c.child.dataType
== TimestampType
Review Comment:
ditto
--
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]
Re: [PR] [VL] Support Cast between TIMESTAMP and TIMESTAMP_NTZ [gluten]
Mariamalmesfer commented on PR #12654: URL: https://github.com/apache/gluten/pull/12654#issuecomment-5256272261 @rui-mo could you take another look? Thank you! -- 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]
Re: [PR] [VL] Support Cast between TIMESTAMP and TIMESTAMP_NTZ [gluten]
github-actions[bot] commented on PR #12654: URL: https://github.com/apache/gluten/pull/12654#issuecomment-5191549939 Run Gluten Clickhouse CI on x86 -- 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]
Re: [PR] [VL] Support Cast between TIMESTAMP and TIMESTAMP_NTZ [gluten]
github-actions[bot] commented on PR #12654: URL: https://github.com/apache/gluten/pull/12654#issuecomment-5190713932 Run Gluten Clickhouse CI on x86 -- 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]
Re: [PR] [VL] Support Cast between TIMESTAMP and TIMESTAMP_NTZ [gluten]
Mariamalmesfer commented on code in PR #12654:
URL: https://github.com/apache/gluten/pull/12654#discussion_r3719654238
##
gluten-substrait/src/main/scala/org/apache/gluten/extension/columnar/validator/Validators.scala:
##
@@ -282,6 +282,8 @@ object Validators {
case Minute(child, _) => containsNTZ(child.dataType)
case Second(child, _) => containsNTZ(child.dataType)
case TimestampAdd(_, _, child, _) =>
containsNTZ(child.dataType)
+ case c: Cast if !containsNTZ(c.dataType) =>
containsNTZ(c.child.dataType)
Review Comment:
Yes, that's correct fixed required both guards to check 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: [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]
Re: [PR] [VL] Support Cast between TIMESTAMP and TIMESTAMP_NTZ [gluten]
rui-mo commented on code in PR #12654:
URL: https://github.com/apache/gluten/pull/12654#discussion_r3713347667
##
gluten-substrait/src/main/scala/org/apache/gluten/extension/columnar/validator/Validators.scala:
##
@@ -282,6 +282,8 @@ object Validators {
case Minute(child, _) => containsNTZ(child.dataType)
case Second(child, _) => containsNTZ(child.dataType)
case TimestampAdd(_, _, child, _) =>
containsNTZ(child.dataType)
+ case c: Cast if !containsNTZ(c.dataType) =>
containsNTZ(c.child.dataType)
Review Comment:
This would allow all non-timestamp_ntz types to be cast to timestamp_ntz.
While I assume the supported cases are still limited. Could you please confirm?
--
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]
Re: [PR] [VL] Support Cast between TIMESTAMP and TIMESTAMP_NTZ [gluten]
Mariamalmesfer commented on PR #12654: URL: https://github.com/apache/gluten/pull/12654#issuecomment-5176746774 Hi @rui-mo, could you take a look when you get the chance? 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: [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]
Re: [PR] [VL] Support Cast between TIMESTAMP and TIMESTAMP_NTZ [gluten]
github-actions[bot] commented on PR #12654: URL: https://github.com/apache/gluten/pull/12654#issuecomment-5165883260 Run Gluten Clickhouse CI on x86 -- 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]
Re: [PR] [VL] Support Cast between TIMESTAMP and TIMESTAMP_NTZ [gluten]
github-actions[bot] commented on PR #12654: URL: https://github.com/apache/gluten/pull/12654#issuecomment-5123764301 Run Gluten Clickhouse CI on x86 -- 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]
Re: [PR] [VL] Support Cast between TIMESTAMP and TIMESTAMP_NTZ [gluten]
github-actions[bot] commented on PR #12654: URL: https://github.com/apache/gluten/pull/12654#issuecomment-5117782192 Run Gluten Clickhouse CI on x86 -- 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]
