Skip to content

Decouple geo facet extraction from rest of document #5592

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

Merged

Conversation

nnethercott
Copy link
Contributor

@nnethercott nnethercott commented May 26, 2025

Pull Request

Related issue

Fixes #5440

What does this PR do?

  • introduces new function extract_geo_document for _geo facet extraction & removes that logic from extract_document_facets
  • adds a new macro_rules! macro to generate add and del facet functions, limiting code duplication

Note: Since each variant of the macro uses a &mut ref, we can't just define top level helpers to use throughout the match block. Ideally a factory function would be used in place of this macro but since closures have anonymous types we run into issues defining the signature of the factory, unless we wanna toss in Box<dyn ...>. I think the macro is a nice trade-off here

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

Copy link
Member

@ManyTheFish ManyTheFish left a comment

Choose a reason for hiding this comment

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

hey @nnethercott,

Nice PR, I wrote some change requests ,

thank you for contributing to Meilisearch!

@nnethercott nnethercott requested a review from ManyTheFish May 28, 2025 13:46
@ManyTheFish ManyTheFish added this to the v1.16.0 milestone May 28, 2025
@ManyTheFish ManyTheFish added the no db change The database didn't change label May 28, 2025
Copy link
Member

@ManyTheFish ManyTheFish left a comment

Choose a reason for hiding this comment

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

LGTM,

thank you for the contribution

@ManyTheFish ManyTheFish added this pull request to the merge queue May 28, 2025
Merged via the queue into meilisearch:main with commit d416b3b May 28, 2025
9 of 11 checks passed
@nnethercott nnethercott deleted the extract-geo-facets-seperately branch May 28, 2025 18:42
@meili-bot meili-bot added the v1.16.0 PRs/issues solved in v1.16.0 released on 2025-08-04 label Aug 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no db change The database didn't change v1.16.0 PRs/issues solved in v1.16.0 released on 2025-08-04
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Separate Geo facet extraction from the standard one
3 participants