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.

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

fix(query): Need return error udf_name when derialize from pb failed. #14964

Merged
merged 2 commits into from
Mar 16, 2024

Conversation

TCeason
Copy link
Collaborator

@TCeason TCeason commented Mar 15, 2024

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

https://github.com/datafuselabs/databend/pull/12729/files#diff-9c992028e59caebc313d761b8488b17f142618fb89db64c51c1655689d68c41b

image

I think we can not upgrade the old json to pb. The best way is return error udf when select * from system.user_functions and then user need to re-create the udf.

  • Fixes #[Link the issue here]

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - rollback deserialize udf code. If the test can be passed the code is ok.

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

This change is Reviewable

@github-actions github-actions bot added the pr-bugfix this PR patches a bug in codebase label Mar 15, 2024
@TCeason TCeason force-pushed the fix_get_udf branch 3 times, most recently from a408790 to 223c1e0 Compare March 15, 2024 08:19
@TCeason TCeason requested review from drmingdrmer and sundy-li March 15, 2024 08:20
Copy link
Member

@drmingdrmer drmingdrmer left a comment

Choose a reason for hiding this comment

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

What does this PR do?

It reverted to using prefix_list_kv() to retrieve from meta-service, that's all?

Reviewed 2 of 4 files at r1, all commit messages.
Reviewable status: 2 of 4 files reviewed, all discussions resolved (waiting on @sundy-li)

@TCeason
Copy link
Collaborator Author

TCeason commented Mar 15, 2024

What does this PR do?

It reverted to using prefix_list_kv() to retrieve from meta-service, that's all?

Reviewed 2 of 4 files at r1, all commit messages.
Reviewable status: 2 of 4 files reviewed, all discussions resolved (waiting on @sundy-li)

just return a error msg with udf_name when deserialize from pb error.

Then user needs to re-create the udf.

@TCeason TCeason changed the title fix(query): udf compat old json format fix(query): Need return error udf_name when derialize from pb failed. Mar 15, 2024
@TCeason
Copy link
Collaborator Author

TCeason commented Mar 15, 2024

What does this PR do?
It reverted to using prefix_list_kv() to retrieve from meta-service, that's all?

Reviewed 2 of 4 files at r1, all commit messages.
Reviewable status: 2 of 4 files reviewed, all discussions resolved (waiting on @sundy-li)

just return a error msg with udf_name when deserialize from pb error.

Then user needs to re-create the udf.

cc @BohuTANG How about this case?

@drmingdrmer drmingdrmer requested a review from sundy-li March 15, 2024 12:35
Copy link
Member

@drmingdrmer drmingdrmer left a 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 r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @sundy-li)

@sundy-li sundy-li added this pull request to the merge queue Mar 16, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 16, 2024
@BohuTANG BohuTANG merged commit e3bade2 into databendlabs:main Mar 16, 2024
80 of 82 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-bugfix this PR patches a bug in codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants