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-1371][VL] Support First/Last aggregate functions #1581

Merged
merged 7 commits into from
May 17, 2023
Merged

[GLUTEN-1371][VL] Support First/Last aggregate functions #1581

merged 7 commits into from
May 17, 2023

Conversation

Yohahaha
Copy link
Contributor

@Yohahaha Yohahaha commented May 9, 2023 •
edited
Loading

What changes were proposed in this pull request?

Support first/last aggregate function and their ignore null behavior.

How was this patch tested?

added UT.

@github-actions
Copy link

github-actions bot commented May 9, 2023

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
Copy link

github-actions bot commented May 9, 2023

Run Gluten Clickhouse CI

@Yohahaha Yohahaha changed the title Support First/Last aggregate functions [GLUTEN-1371] Support First/Last aggregate functions May 9, 2023
@github-actions
Copy link

github-actions bot commented May 9, 2023

#1371

@Yohahaha Yohahaha changed the title [GLUTEN-1371] Support First/Last aggregate functions [GLUTEN-1371][VL] Support First/Last aggregate functions May 9, 2023
@Yohahaha Yohahaha mentioned this pull request May 9, 2023
Merged
@github-actions
Copy link

github-actions bot commented May 9, 2023

Run Gluten Clickhouse CI

@rui-mo
Copy link
Contributor

rui-mo commented May 10, 2023

@Yohahaha There are several failed unit tests on CI, for example:

last/first with ignoreNulls *** FAILED ***^[[0m
 172218 2023-05-09T11:22:28.5720258Z ^[[31m  Results do not match for query

Could you take a look? Logs can be downloaded for details.

@Yohahaha
Copy link
Contributor Author

Thanks @rui-mo

I'm fixing it, when CI all passed, I will request you for review again, thanks!

@github-actions
Copy link

Run Gluten Clickhouse CI

@Yohahaha
Copy link
Contributor Author

#1606

@github-actions
Copy link

Run Gluten Clickhouse CI

1 similar comment
@github-actions
Copy link

Run Gluten Clickhouse CI

@github-actions
Copy link

Run Gluten Clickhouse CI

1 similar comment
@github-actions
Copy link

Run Gluten Clickhouse CI

@github-actions
Copy link

Run Gluten Clickhouse CI

1 similar comment
@github-actions
Copy link

Run Gluten Clickhouse CI

@github-actions
Copy link

Run Gluten Clickhouse CI

@Yohahaha Yohahaha force-pushed the first_last branch 2 times, most recently from 5606ca9 to 498edf5 Compare May 15, 2023 07:03
@github-actions
Copy link

Run Gluten Clickhouse CI

1 similar comment
@github-actions
Copy link

Run Gluten Clickhouse CI

@github-actions
Copy link

Run Gluten Clickhouse CI

Yohahaha added 2 commits May 16, 2023 09:47
@Yohahaha
Support First/Last aggregate functions.
6af0a55 
fix

fix UT.

Add first/last to ch blacklist

refactor

trigger ci

fix match error.
@Yohahaha
Use tmp velox branch
0d5c61e
@github-actions
Copy link

Run Gluten Clickhouse CI

@Yohahaha
Copy link
Contributor Author

Hi @rui-mo, all CI passed, please review again, thanks!

@github-actions
Copy link

Run Gluten Clickhouse CI

@Yohahaha
Copy link
Contributor Author

CI failed due to network issue.

@github-actions
Copy link

Run Gluten Clickhouse CI

@github-actions
Copy link

Run Gluten Clickhouse CI

@FelixYBW FelixYBW merged commit ae33bb1 into apache:main May 17, 2023
@Yohahaha Yohahaha deleted the first_last branch May 17, 2023 07:41
@Yohahaha Yohahaha mentioned this pull request Jun 29, 2023
Closed
facebook-github-bot pushed a commit to facebookincubator/velox that referenced this pull request Jul 6, 2023
@Yohahaha @facebook-github-bot
Fix Spark first/last aggregate functions intermediate type from T to …
9fdf36e 
…row(T, boolean) (#4873)

Summary:
Fix first/last Spark aggregate functions' intermediate type from T to row(T, boolean) to align with Spark, the second column in signature is just a place holder.

oap-project#245
apache/incubator-gluten#1581

Pull Request resolved: #4873

Reviewed By: pedroerp

Differential Revision: D47250246

Pulled By: mbasmanova

fbshipit-source-id: dd6ccff394f820b40f59696adeee244172b27a68
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FelixYBW FelixYBW FelixYBW approved these changes

@rui-mo rui-mo rui-mo 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.
3 participants
@Yohahaha @rui-mo @FelixYBW