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

[VL] Support YearMonthIntervalType and enable make_ym_interval #4798

Merged
merged 2 commits into from
Mar 28, 2024
Merged

[VL] Support YearMonthIntervalType and enable make_ym_interval #4798

merged 2 commits into from
Mar 28, 2024

Conversation

marin-ma
Copy link
Contributor

@marin-ma marin-ma commented Feb 28, 2024 •
edited
Loading

Limitation: Cast from/to INTERVAL_YEAR_MONTH is not supported in Velox.

@github-actions GitHub Actions
Copy link

Thanks for opening a pull request!

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

https://github.com/oap-project/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:

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

1 similar comment
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@marin-ma marin-ma force-pushed the test-make-ym-interval branch from 6d4b53b to e37f600 Compare February 28, 2024 15:53
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

1 similar comment
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@marin-ma marin-ma force-pushed the test-make-ym-interval branch from ef746c5 to 2cc7fde Compare March 21, 2024 05:28
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@marin-ma marin-ma marked this pull request as ready for review March 21, 2024 05:57
@marin-ma
Copy link
Contributor Author

@PHILO-HE Could you help to review first? I will revert get_velox.sh after tomorrow rebase. Thanks!

@marin-ma marin-ma force-pushed the test-make-ym-interval branch from 2cc7fde to 35e1296 Compare March 21, 2024 15:47
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

PHILO-HE
PHILO-HE reviewed Mar 22, 2024
View reviewed changes
Copy link
Contributor

@PHILO-HE PHILO-HE left a comment

Choose a reason for hiding this comment

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

Thanks! One small comment.

backends-velox/src/test/scala/io/glutenproject/execution/VeloxFunctionsValidateSuite.scala Outdated
checkOperatorMatch[ProjectExecTransformer]
}

// select make_ym_interval() from make_ym_interval_tbl will be optimized with fallback.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe, need to add some handling in FallbackEmptySchemaRelation to allow offloading this.

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@PHILO-HE
Copy link
Contributor

@taiyang-li, CH CI is red, could you give us some clues to fix? Thanks!

@marin-ma marin-ma force-pushed the test-make-ym-interval branch from 49df8c5 to 4660f58 Compare March 25, 2024 06:59
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

1 similar comment
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@taiyang-li
Copy link
Contributor

@taiyang-li, CH CI is red, could you give us some clues to fix? Thanks!

image

Seems like the new added type is not supported in CH backend.

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@taiyang-li
Copy link
Contributor

@taiyang-li, CH CI is red, could you give us some clues to fix? Thanks!

image

Seems like the new added type is not supported in CH backend.

@zzcclp do we have any mechanism to disable some types while transforming ?

@marin-ma marin-ma force-pushed the test-make-ym-interval branch from ce6f149 to 9c1e68a Compare March 26, 2024 04:59
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

1 similar comment
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@zzcclp
Copy link
Contributor

zzcclp commented Mar 26, 2024

It seems there are some SPARK-36830: Support reading and writing ANSI intervals which are not disable for the spark 3.3

@marin-ma marin-ma force-pushed the test-make-ym-interval branch from 724418c to 604fc7d Compare March 26, 2024 16:13
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@marin-ma
Copy link
Contributor Author

@zzcclp CH CI passed. Could you help to review? Thanks!

@marin-ma
Copy link
Contributor Author

@taiyang-li @zzcclp @PHILO-HE Could we merge this patch if no further comments?

PHILO-HE
PHILO-HE approved these changes Mar 28, 2024
View reviewed changes
Copy link
Contributor

@PHILO-HE PHILO-HE left a comment

Choose a reason for hiding this comment

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

Thanks!

@marin-ma marin-ma merged commit 8f5ad48 into apache:main Mar 28, 2024
30 checks passed
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 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@marin-ma @PHILO-HE @taiyang-li @zzcclp