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

feat: added trgm operators #486

Merged
merged 1 commit into from
Nov 2, 2022

Conversation

baszalmstra
Copy link
Contributor

PR Info

Adds pg_trgm binary operators which are required to allow proper use of indexes for Postgres.

Adds

  • Binary operators from the Postgres pg_trgm extension

@ikrivosheev
Copy link
Member

@baszalmstra thank you for PR! Can you run rustfmt?

Copy link
Member

@ikrivosheev ikrivosheev left a comment

Choose a reason for hiding this comment

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

@baszalmstra thank you! LGTM!

I think we can also add: BinOpen::Custom for this case

@baszalmstra
Copy link
Contributor Author

I think we can also add: BinOpen::Custom for this case

I tried adding BinOper::Custom(String) but the BinOper is Copy and String obviously isn't. Removing the Copy derive would change the API significantly.

I see a few options:

  • Use &'static str, but that would limit the use
  • Remove the Copy trait from BinOp
  • Implement the Copy trait and use Rc, Arc, or DynIden

WDYT?

@tyt2y3
Copy link
Member

tyt2y3 commented Oct 23, 2022

May be we can cater custom BinOp later.

@ikrivosheev ikrivosheev merged commit 7953371 into SeaQL:master Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants