Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Compound Wildcard Indexes #4790

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

Open
marcingrzejszczak wants to merge 5 commits into main
base: main
Choose a base branch
Loading
from issue/4471
Open

Compound Wildcard Indexes #4790

marcingrzejszczak wants to merge 5 commits into main from issue/4471

Conversation

Copy link
Contributor

@marcingrzejszczak marcingrzejszczak commented Sep 16, 2024

No description provided.

Copy link
Contributor Author

Question about the current implementation

A ccording to the tests and the implementation the following combinations are NOT allowed (a MappingException is being thrown)

  • wildcardFieldName NOT empty and wildcardProjection NOT empty
  • wildcardFieldName EMPTY and wildecardProjection EMPTY

While the first condition is correct I have doubts about the second one.

If wildcardFieldName is empty, that implies the default $** value. That in effect means that all fields are scanned and wildcardProjection is optional. Why is an exception being thrown?

cc @christophstrobl

Copy link
Member

Yeah, you can create the index using an all fields term without narrowing down the scope via projections.
Would it make sense to switch the default value for wildcardFieldName to $** and replace the empty checks with something like an isAllFieldsIndexTerm(...) so we can get the code path there more expressive?

Another thing that crossed my mind is that you can specify more than one wildcard index on a collection - I guess same is true for compound wildcard ones, which would require the annotation to be repeatable.

marcingrzejszczak reacted with thumbs up emoji

Copy link
Contributor Author

I looked into the code and re:

Would it make sense to switch the default value for wildcardFieldName to $** and replace the empty checks with something like an isAllFieldsIndexTerm(...) so we can get the code path there more expressive?

If we look at @CompoundIndex it has the def method

String def() default "";

and the Javadoc states

  • If left empty on nested document, the whole document will be indexed.

Which means that if we go with the approach that the wildcardFieldName would be equal to $** then I think we would be inconsistent. WDYT?

Copy link
Member

I do not agree - we've got the fields attribute that translates to the def of the compound index. So if that is blank and the default for wildcardFieldName is $** the entire doc would be indexed, right?

Copy link
Contributor Author

OK I understand. For future reference we're talking about @CompoundWildcardIndexed that has fields method that delegates to @CompoundIndex's def method. Now if @CWI is empty which is the default that would be that @CI def is empty which means that it translates to $** which indexes the whole doc.

@marcingrzejszczak marcingrzejszczak marked this pull request as ready for review September 25, 2024 09:40
Copy link
Member

Does it make sense to provide a CompoundWildcardIndexes container to allow repeatable usage? Would also be good to have a test that is using value expressions for the index name to see if resolving those work as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Labels
status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /