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.

Sign up for GitHub

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

Jump to bottom

[GLUTEN-8639][VL] Support casting from double/float to timestamp #8640

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

[GLUTEN-8639][VL] Support casting from double/float to timestamp #8640

wants to merge 6 commits into from

Conversation

ArnavBalyan
Copy link
Contributor

@ArnavBalyan ArnavBalyan commented Jan 30, 2025 •
edited
Loading

What changes were proposed in this pull request?

How was this patch tested?

  • Unit tests added

@ArnavBalyan ArnavBalyan marked this pull request as draft January 30, 2025 02:41
@github-actions github-actions bot added the VELOX label Jan 30, 2025
@github-actions GitHub Actions
Copy link

#8639

@ArnavBalyan ArnavBalyan force-pushed the arnavb/double-timestamp branch 2 times, most recently from 7ab5b33 to 97ec93e Compare January 31, 2025 14:28
@github-actions github-actions bot added the CORE works for Gluten Core label Jan 31, 2025
@github-actions GitHub Actions
Copy link

Run Gluten ClickHouse CI on ARM

@ArnavBalyan ArnavBalyan force-pushed the arnavb/double-timestamp branch from 97ec93e to d237252 Compare January 31, 2025 15:19
@github-actions github-actions bot added the BUILD label Jan 31, 2025
@github-actions GitHub Actions
Copy link

Run Gluten ClickHouse CI on ARM

@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 2, 2025

Run Gluten ClickHouse CI on ARM

1 similar comment
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 2, 2025

Run Gluten ClickHouse CI on ARM

@ArnavBalyan ArnavBalyan force-pushed the arnavb/double-timestamp branch from 0d4449d to bb5671d Compare February 3, 2025 14:58
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 3, 2025

Run Gluten ClickHouse CI on ARM

1 similar comment
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 3, 2025

Run Gluten ClickHouse CI on ARM

@ArnavBalyan ArnavBalyan mentioned this pull request Feb 15, 2025
Open
@ArnavBalyan ArnavBalyan force-pushed the arnavb/double-timestamp branch from c070564 to 90a63fc Compare February 28, 2025 07:27
@github-actions GitHub Actions
Copy link

Run Gluten ClickHouse CI on ARM

@github-actions github-actions bot removed the BUILD label Feb 28, 2025
@github-actions GitHub Actions
Copy link

Run Gluten ClickHouse CI on ARM

jinchengchenghh
jinchengchenghh reviewed Mar 2, 2025
View reviewed changes
cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc Outdated
// Only support cast from date to timestamp
if (toType->kind() == TypeKind::TIMESTAMP && !input->type()->isDate()) {
// Only support cast of certain types to timestamp (date and double)
if (toType->kind() == TypeKind::TIMESTAMP && !input->type()->isDate() && input->type()->kind() != TypeKind::DOUBLE) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This one facebookincubator/velox#12041 has been merged, please update to also support float, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated thanks

@ArnavBalyan ArnavBalyan force-pushed the arnavb/double-timestamp branch from 096d3bc to 62eafc8 Compare March 3, 2025 07:22
@ArnavBalyan ArnavBalyan marked this pull request as ready for review March 3, 2025 07:23
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 3, 2025

Run Gluten ClickHouse CI on ARM

@ArnavBalyan ArnavBalyan force-pushed the arnavb/double-timestamp branch from 62eafc8 to 04c9b9f Compare March 3, 2025 16:49
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 3, 2025

Run Gluten ClickHouse CI on ARM

@ArnavBalyan ArnavBalyan force-pushed the arnavb/double-timestamp branch from 04c9b9f to 64af4ec Compare March 3, 2025 16:58
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 3, 2025

Run Gluten ClickHouse CI on ARM

@jinchengchenghh
Copy link
Contributor

Please update the PR description

@ArnavBalyan ArnavBalyan changed the title [GLUTEN-8639][VL] Support casting from double to timestamp [GLUTEN-8639][VL] Support casting from double/float to timestamp Mar 3, 2025
@ArnavBalyan
Copy link
Contributor Author

Please update the PR description

done

@jinchengchenghh
Copy link
Contributor

Please refer to this one facebookincubator/velox#12041 in the PR description, but it is not merged in oap-project/velox, please wait for #8867

@ArnavBalyan
Copy link
Contributor Author

Please refer to this one facebookincubator/velox#12041 in the PR description, but it is not merged in oap-project/velox, please wait for #8867

Got it, updated the ref, will wait for the Velox to update

PHILO-HE
PHILO-HE reviewed Mar 4, 2025
View reviewed changes
...en-ut/spark33/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenCastSuite.scala Outdated
@@ -152,4 +153,33 @@ class GlutenCastSuite extends CastSuite with GlutenTestsTrait {

checkEvaluation(cast(Literal.create(null, IntegerType), ShortType), null)
}

test("cast from double to timestamp format") {
Copy link
Contributor

Choose a reason for hiding this comment

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

If this test is not a variant of vanilla spark test, we prefer moving it to some common places, not bound to one Spark version. E.g., ScalarFunctoinsValidateSuite.scala or one new dedicate test class under backends-velox.

@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 6, 2025

Run Gluten ClickHouse CI on ARM

@ArnavBalyan ArnavBalyan force-pushed the arnavb/double-timestamp branch from aef40b1 to e7fd3dd Compare March 6, 2025 11:48
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 6, 2025

Run Gluten ClickHouse CI on ARM

@ArnavBalyan ArnavBalyan force-pushed the arnavb/double-timestamp branch from e7fd3dd to c3aa1f7 Compare March 9, 2025 13:42
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 9, 2025

Run Gluten ClickHouse CI on ARM

1 similar comment
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 9, 2025

Run Gluten ClickHouse CI on ARM

@ArnavBalyan ArnavBalyan force-pushed the arnavb/double-timestamp branch from 1fe01c4 to c3aa1f7 Compare March 10, 2025 09:09
@github-actions GitHub Actions
Copy link

Run Gluten ClickHouse CI on ARM

1 similar comment
@github-actions GitHub Actions
Copy link

Run Gluten ClickHouse CI on ARM

@ArnavBalyan ArnavBalyan force-pushed the arnavb/double-timestamp branch from 2b29adc to 65d3939 Compare March 10, 2025 14:03
@github-actions GitHub Actions
Copy link

Run Gluten ClickHouse CI on ARM

@ArnavBalyan ArnavBalyan force-pushed the arnavb/double-timestamp branch from 65d3939 to 63b356d Compare March 10, 2025 15:36
@github-actions GitHub Actions
Copy link

Run Gluten ClickHouse CI on ARM

@ArnavBalyan ArnavBalyan force-pushed the arnavb/double-timestamp branch from 63b356d to e299557 Compare March 11, 2025 04:17
@github-actions GitHub Actions
Copy link

Run Gluten ClickHouse CI on ARM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PHILO-HE PHILO-HE PHILO-HE left review comments

@jinchengchenghh jinchengchenghh jinchengchenghh left review comments

Assignees
No one assigned
Labels
CORE works for Gluten Core VELOX
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ArnavBalyan @jinchengchenghh @PHILO-HE