-
Notifications
You must be signed in to change notification settings - Fork 761
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
refactor: try drop old udf - DON'T MERGE #17264
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @TCeason)
src/query/management/src/udf/udf_mgr.rs
line 168 at r1 (raw file):
} else { self.try_drop_old_udf(udf_name).await?; Ok(None)
To this method, Ok(None)
means nothing is removed.
If it's meant to remove an obsolete udf, the caller should call try_drop_old_udf()
directly, instead of delegate to try_drop_old_udf()
in this method.
Code quote:
self.try_drop_old_udf(udf_name).await?;
Ok(None)
Get so after we drop the old udf we should remove this logic |
} else { | ||
self.try_drop_old_udf(udf_name).await?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need a log here to know which old udf is dropped.
Docker Image for PR
|
Docker Image for PR
|
In eariler version, udf is serialize as json. In current version we can not drop/list these udfs. show user functions; 2001=>InvalidReply: source:(PbDecodeError: failed to decode Protobuf message: buffer underflow; when:(decode value of __fd_udfs/tn3ftqihs/plusp)) while list UDFs drop function IF EXISTS plusp; 2001=>InvalidReply: source:(PbDecodeError: failed to decode Protobuf message: buffer underflow; when:(decode value of __fd_udfs/tn3ftqihs/plusp)) So if drop udf return err, we directly drop the kv.
Docker Image for PR
|
Already normal. Close pr. |
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
In earlier version, udf serialize as json. In the current version, we can not drop/list these udfs.
So if drop udf return err, we directly drop the kv.
Tests
Type of change
This change is