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

Convert first, last aggregate function to UDAF #18

Closed
wants to merge 86 commits into from
Closed

Convert first, last aggregate function to UDAF #18

wants to merge 86 commits into from

Conversation

mustafasrepo
Copy link
Collaborator

Which issue does this PR close?

Closes #.

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

jayzhan211 added 30 commits April 10, 2024 14:55
@jayzhan211
move out the ordering ruel
e506662 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
introduce rule
2812886 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
revert test result
d20dde3 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
pass mulit order test
c671033 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
cleanup
3f43dc1 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
with new childes
c8b46a4 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
revert slt
fcb297b 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
revert back
ab10168 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
rm rewrite in new child
fffb54a 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
backup
fec3eaf 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
only move conversion to optimizer
5b68b48 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
find test that do reverse
2ab74c9 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
add test for first and last
03abf19 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
pass all test
83c721a 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
upd test
34b8f01 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
upd test
b17e8f7 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
cleanup
a313af0 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
cleanup
a37fb3e 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
cleanup
8de501c 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
add aggregate test
107f4c8 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
cleanup
345d4c6 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
final draft
030d7b9 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
cleanup again
cf3f33e 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
pull out finer ordering code and reuse
6d9ee9f 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
clippy
abd40c6 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
remove finer in optimize rule
3e2c186 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
add comments and clenaup
d7c2590 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
rename fun
fed356a 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
rename fun
0d3d461 
Signed-off-by: jayzhan211 <[email protected]>
@jayzhan211
fmt
d3daa4e 
Signed-off-by: jayzhan211 <[email protected]>
@mustafasrepo mustafasrepo changed the title Feature/10061 subsequent pr Convert first, last aggregate function to UDAF May 23, 2024
mustafasrepo
mustafasrepo commented May 23, 2024
View reviewed changes
datafusion/functions/Cargo.toml Outdated
@@ -80,7 +80,7 @@ itertools = { workspace = true }
log = { workspace = true }
md-5 = { version = "^0.10.0", optional = true }
rand = { workspace = true }
regex = { worksapce = true, optional = true }
regex = { workspace = true, optional = true }
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this was typo, fixed it on the way.

@mustafasrepo mustafasrepo requested a review from ozankabak May 23, 2024 12:56
@mustafasrepo mustafasrepo added the final_review Ozan can review this pr label May 23, 2024
@mustafasrepo
Copy link
Collaborator Author

@ozankabak, this PR is ready for further review.

ozankabak
ozankabak approved these changes May 24, 2024
View reviewed changes
Copy link
Collaborator

@ozankabak ozankabak left a comment

Choose a reason for hiding this comment

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

LGTM, two minor issues to investigate/discuss before we send it upstream.

datafusion/expr/src/udaf.rs
pub fn reverse_udf(&self) -> Option<AggregateUDF> {
match self.inner.reverse_expr() {
ReversedUDAF::NotSupported => None,
ReversedUDAF::Identical => Some(self.clone()),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we get rid of this clone?

@ozankabak
Copy link
Collaborator

Merged upstream.

@ozankabak ozankabak closed this May 28, 2024
ozankabak pushed a commit that referenced this pull request Jul 24, 2024
@sgrebnov
Improve unparser MySQL compatibility (apache#11589)
65dd364 
* Configurable date field extraction style for unparsing (#21)

* Add support for IntervalStyle::MySQL (#18)

* Support alternate format for Int64 unparsing (SIGNED for MySQL) (#22)

* Alternate format support for Timestamp casting (DATETIME for MySQL) (#23)

* Improve

* Fix clippy and docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ozankabak ozankabak ozankabak approved these changes

Assignees
No one assigned
Labels
core final_review Ozan can review this pr logical-expr optimizer physical-expr
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mustafasrepo @ozankabak @jayzhan211 @berkaysynnada