rui-mo commented on PR #12967: URL: https://github.com/apache/gluten/pull/12967#issuecomment-5616749539
@felipepessoto Yes, I added the `TIMESTAMP_UTC` type in Velox. It was designed to share the same physical representation as `TimestampType` while having different logical semantics. The motivation was to represent the timezone-adjustment semantics correctly while still allowing us to reuse the existing implementations where possible. My concern is that, since we haven’t yet verified that aggregate functions work correctly with `TIMESTAMP_UTC`, it may be better not to treat them as supported implicitly. Instead, we could explicitly validate the support in Velox by adding unit tests and updating the coverage documentation, even if no implementation changes turn out to be necessary. Would you be interested in helping add this support for `min`/`max` in Velox first? 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]
