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] Enable cast when input type equal to dest type. #8674

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

j7nhai
Copy link
Contributor

@j7nhai j7nhai commented Feb 6, 2025

What changes were proposed in this pull request?

(Please fill in changes proposed in this fix)

(Fixes: #ISSUE-ID)

How was this patch tested?

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)

(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

@github-actions github-actions bot added the VELOX label Feb 6, 2025
Copy link

github-actions bot commented Feb 6, 2025

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:

@j7nhai j7nhai force-pushed the dev4 branch 2 times, most recently from ad41c6d to 17229b1 Compare February 6, 2025 09:56
@marin-ma
Copy link
Contributor

marin-ma commented Feb 6, 2025

This case shouldn't happen. See https://github.com/apache/incubator-gluten/blob/main/docs/velox-backend-support-progress.md#cast-functions-support-status

N/A: not applicable case, e.g., from type is as same as to type, where cast will not actually happen.

Could you provide a sample to demonstrate when this code path will be called?

@j7nhai
Copy link
Contributor Author

j7nhai commented Feb 7, 2025

This case shouldn't happen. See https://github.com/apache/incubator-gluten/blob/main/docs/velox-backend-support-progress.md#cast-functions-support-status

N/A: not applicable case, e.g., from type is as same as to type, where cast will not actually happen.

Could you provide a sample to demonstrate when this code path will be called?

sql("use test")
sql("drop table if exists t1")
sql("drop table if exists t2")

sql("create table t1 (a TIMESTAMP) USING ICEBERG")
sql("create table t2 (b TIMESTAMP) USING ICEBERG")

sql("INSERT OVERWRITE t2 VALUES (CAST('2025-01-21 00:00:00' AS TIMESTAMP))")
sql("INSERT OVERWRITE t1 SELECT * FROM t2")  // Not supported to map spark function name to substrait function name: cast(b#7 as timestamp), class name: AnsiCast.

Then I try to convert AnsiCast to Cast when origin type equal to dest type. And the native cast not support ts to ts.

@PHILO-HE
Copy link
Contributor

@j7nhai, does this issue only happen for iceberg? I found using parquet has no issue.

Spark has an optimizer that removes the same type cast. Not sure why it is not applied in your case.
https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala#L1091

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants