Skip to content

[nocommit] LUCENE-9948: Automatically detect multi- vs. single-valued cases in LongValueFacetCounts#122

Merged
rmuir merged 4 commits intoapache:mainfrom
gsmiller:LUCENE-9948-pr
May 3, 2021
Merged

[nocommit] LUCENE-9948: Automatically detect multi- vs. single-valued cases in LongValueFacetCounts#122
rmuir merged 4 commits intoapache:mainfrom
gsmiller:LUCENE-9948-pr

Conversation

@gsmiller
Copy link
Copy Markdown
Contributor

@gsmiller gsmiller commented May 2, 2021

Description

Automatically detect single- vs. multi-value cases instead of asking the user to specify.

Solution

Use DocValues helper methods to automatically determine the underlying field type.

Tests

No new tests but modified existing to no longer require the multiValued ctor parameter, and ensured they cover both the single- and multi-valued cases.

Checklist

Please review the following and check all that apply:

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Lucene maintainers access to contribute to my PR branch. (optional but recommended)
  • I have developed this patch against the main branch.
  • I have run ./gradlew check.
  • I have added tests for my changes.

Copy link
Copy Markdown
Member

@rmuir rmuir left a comment

Choose a reason for hiding this comment

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

looks good. thanks for cleaning up the api here.

@rmuir
Copy link
Copy Markdown
Member

rmuir commented May 3, 2021

one TODO i want to try for fun, is to look at output from gradlew -p lucene/facet coverage and see if all the e.g. optimized cases here are really tested (#119). Will look into it later!

@rmuir
Copy link
Copy Markdown
Member

rmuir commented May 3, 2021

Looks good:
Screen_Shot_2021-05-03_at_11 14 55

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.

2 participants