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

refactor: DfUdfAdapter to bridge ScalaUdf #3814

Merged
merged 6 commits into from
Apr 28, 2024
Merged

refactor: DfUdfAdapter to bridge ScalaUdf #3814

merged 6 commits into from
Apr 28, 2024

Conversation

tisonkun
Copy link
Collaborator

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

What's changed and what's your intention?

This closes #3666 and closes #3665.

Checklist

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.
  • This PR does not require documentation updates.

@tisonkun
refactor: DfUdfAdapter to bridge ScalaUdf
1010e6f 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun tisonkun requested a review from a team as a code owner April 26, 2024 14:54
@github-actions github-actions bot added the docs-not-required This change does not impact docs. label Apr 26, 2024
@tisonkun
tidy impl
5f7802f 
Signed-off-by: tison <wander4096@gmail.com>
killme2008
killme2008 approved these changes Apr 27, 2024
View reviewed changes
Copy link
Contributor

@killme2008 killme2008 left a comment

Choose a reason for hiding this comment

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

LGTM

@tisonkun tisonkun enabled auto-merge April 28, 2024 03:04
@tisonkun tisonkun disabled auto-merge April 28, 2024 03:18
waynexia
waynexia approved these changes Apr 28, 2024
View reviewed changes
Copy link
Member

@waynexia waynexia left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

src/common/query/src/logical_plan/udf.rs Outdated Show resolved Hide resolved
tisonkun added 2 commits April 28, 2024 11:46
@tisonkun
for more
87c6eae 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun
for more
879e7cf 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun
Copy link
Collaborator Author

There are still some other #[allow(deprecated)] to be updated. I'm handling ...

@codecov Codecov
Copy link

codecov bot commented Apr 28, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 70.58824% with 35 lines in your changes are missing coverage. Please review.

Project coverage is 85.48%. Comparing base (3112ced) to head (0156833).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #3814      +/-   ##
==========================================
- Coverage   85.63%   85.48%   -0.15%     
==========================================
  Files         955      952       -3     
  Lines      163100   162629     -471     
==========================================
- Hits       139669   139026     -643     
- Misses      23431    23603     +172

@tisonkun
for more
0156833 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun
Copy link
Collaborator Author

All updated.

@tisonkun tisonkun enabled auto-merge April 28, 2024 04:14
@tisonkun tisonkun added this pull request to the merge queue Apr 28, 2024
Merged via the queue into GreptimeTeam:main with commit e154dc5 Apr 28, 2024
20 checks passed
@tisonkun tisonkun deleted the reimpl-scalaudf branch April 28, 2024 04:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaelScofield MichaelScofield MichaelScofield left review comments

@waynexia waynexia waynexia approved these changes

@killme2008 killme2008 killme2008 approved these changes

Assignees
No one assigned
Labels
docs-not-required This change does not impact docs.
Projects
None yet
Milestone
No milestone
4 participants
@tisonkun @killme2008 @MichaelScofield @waynexia