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

discussion for set fulltext options method #5116

Closed
lyang24 opened this issue Dec 7, 2024 · 1 comment
Closed

discussion for set fulltext options method #5116

lyang24 opened this issue Dec 7, 2024 · 1 comment
Labels
C-enhancement Category Enhancements

Comments

@lyang24
Copy link
Collaborator

lyang24 commented Dec 7, 2024

What type of enhancement is this?

Refactor

What does the enhancement do?

https://github.com/GreptimeTeam/greptimedb/blob/19373d806d7bedbb8f701d169706d23a3d1c2d8f/src/datatypes/src/schema/column_schema.rs#L286C1-L300C6

seems like duplicate to me, can we replace set_fulltext_options with with_fulltext_options ?

Implementation challenges

No response

@lyang24 lyang24 added the C-enhancement Category Enhancements label Dec 7, 2024
@CookiePieWw
Copy link
Collaborator

CookiePieWw commented Dec 9, 2024

Thanks for pointing out. I initially tried to use with... but finally add a set... in #4952

They are different in parameters and return type actually. It seems interchangeable as you said but replacement requires lots of manual work as I tried, so I'd like to keep them all, one for initialization and one as setter.

@lyang24 lyang24 closed this as completed Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category Enhancements
Projects
None yet
Development

No branches or pull requests

2 participants