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

Allow prefix behavior to depend on the field's type #4684

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rcrowell
Copy link
Contributor

Description

I helped add the exact match query feature last year, but the exact-match prefix = does not seem to work for PathQuery. After stepping through this code for a while tonight, I believe this because MatchQuery does not know to convert its pattern to a memoryview.

The proposed solution here tweaks the way prefixes work, allowing any prefix string to be assigned to a dictionary mapping the field's un-prefixed query type to the prefix's desired query type (use a defaultdict to provide a default query class, if desired).

I thought this made sense (vs simply checking for the field name 'path') to support plugins that may want to use this functionality for their own ends. But is there a better, more straightforward approach that I could have used?

(...)

To Do

  • Documentation. (If you've add a new command-line flag, for example, find the appropriate page under docs/ to describe it.)
  • Changelog. (Add an entry to docs/changelog.rst near the top of the document.)
  • Tests. (Encouraged but not strictly required.)

@JOJ0 JOJ0 requested review from kergoth and removed request for kergoth March 14, 2023 09:36
@stale
Copy link

stale bot commented Oct 15, 2023

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 15, 2023
@sampsyo sampsyo removed the stale label Oct 20, 2023
@sampsyo
Copy link
Member

sampsyo commented Oct 20, 2023

While I am sorry I haven't had the bandwidth to do a thorough review on this, the PR already looks very well thought out. I wouldn't object to merging it. My only concern is whether the same high-level fix could be achieved in a simpler way: make MatchQuery aware of bytes. That way, we wouldn't have to switch between different queries based on the type; we could instead just have one type-aware query. (Not clear that this doesn't create other problems.)

Copy link

stale bot commented Mar 17, 2024

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants