Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[VL] Support casting timestamp type to varchar type #8338

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

PHILO-HE
Copy link
Contributor

@PHILO-HE PHILO-HE commented Dec 25, 2024

What changes were proposed in this pull request?

To align with Spark, we need consider session timezone in this cast operation. See facebookincubator/velox#11958

How was this patch tested?

Existing imported Spark tests.

@github-actions github-actions bot added the VELOX label Dec 25, 2024
Copy link

Thanks for opening a pull request!

Could you open an issue for this pull request on Github Issues?

https://github.com/apache/incubator-gluten/issues

Then could you also rename commit message and pull request title in the following format?

[GLUTEN-${ISSUES_ID}][COMPONENT]feat/fix: ${detailed message}

See also:

@PHILO-HE PHILO-HE force-pushed the fix-timestamp-involved-cast branch from 2a0e02f to d15c889 Compare December 25, 2024 09:43
@github-actions github-actions bot added the BUILD label Dec 25, 2024
@@ -301,7 +301,7 @@ bool SubstraitToVeloxPlanValidator::validateCast(
return false;
case TypeKind::TIMESTAMP:
// Only support cast timestamp to date
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment need change.
I remember that cast timestamp to varchar will get incorrect results. Can the velox changes here fix it? @PHILO-HE

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zml1206, thanks for your comment! Yes, Spark UTs have uncovered some wrong result issue. I am still trying to fix all issues.

@PHILO-HE PHILO-HE force-pushed the fix-timestamp-involved-cast branch from d15c889 to 99be2a0 Compare December 25, 2024 13:45
@github-actions github-actions bot added the CORE works for Gluten Core label Dec 26, 2024
Copy link

Run Gluten Clickhouse CI on x86

Copy link

Run Gluten Clickhouse CI on x86

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BUILD CORE works for Gluten Core VELOX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants