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

LUCENE-10539: Return a stream of completions from FSTCompletion. #844

Merged
merged 2 commits into from
Apr 29, 2022

Conversation

dweiss
Copy link
Contributor

@dweiss dweiss commented Apr 27, 2022

No description provided.

@@ -130,8 +153,17 @@ public void testAlphabeticWithWeights() throws Exception {
}

public void testFullMatchList() throws Exception {
// one/0.0 is returned first because it's an exact match.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems to be a bug in the current Lucene implementation - one/0.0 is ordered last, even though exact match flag is true. I corrected the test.


Stream<Completion> stream =
IntStream.range(0, rootArcs.length)
.boxed()
Copy link
Contributor

Choose a reason for hiding this comment

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

why boxed? I haven't used streams API much, but ... is an IntStream not a Stream or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IntStream.flatMap returns an IntStream - there is no method that converts an IntStream to a flattened stream of Object (T). These are small integers, they're all cached instances - shouldn't play a big role. I don't know what the deep impact of replacing a method with a stream here will be but I think it'll be small compared to the overall cost of fst traversal and other unrelated things happening outside of the lookup code.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah yeah the bifurcation of primitives and boxed values continues to haunt us. This all LGTM

@dweiss
Copy link
Contributor Author

dweiss commented Apr 29, 2022

I plan to commit this in if there are no objections.

@dweiss dweiss merged commit 05de908 into apache:main Apr 29, 2022
@dweiss
Copy link
Contributor Author

dweiss commented Apr 29, 2022

Thanks @msokolov !

wjp719 added a commit to wjp719/lucene that referenced this pull request May 10, 2022
* main: (24 commits)
  LUCENE-10532: remove @Slow annotation (apache#832)
  LUCENE-10312: Add PersianStemmer (apache#540)
  LUCENE-10558: Implement URL ctor to support classpath/module usage in Kuromoji and Nori dictionaries (main branch) (apache#871)
  LUCENE-10436: Reinstate public getdocValuesdocIdSetIterator method on DocValues (apache#869)
  Disable liftbot, we have our own tools
  LUCENE-10553: Fix WANDScorer's handling of 0 and +Infty. (apache#860)
  Make CONTRIBUTING.md a bit more succinct (apache#866)
  LUCENE-10504: KnnGraphTester to use KnnVectorQuery (apache#796)
  Add change line for LUCENE-9848
  LUCENE-9848 Sort HNSW graph neighbors for construction (apache#862)
  LUCENE-10524 Add benchmark suite details to CONTRIBUTING.md (apache#853)
  LUCENE-10552: KnnVectorQuery has incorrect equals/ hashCode (apache#859)
  LUCENE-10534: MinFloatFunction / MaxFloatFunction calls exists twice (apache#837)
  LUCENE-10188: Give SortedSetDocValues a docValueCount() (apache#663)
  Allow to link to github PR from changes (apache#854)
  LUCENE-10551: improve testing of LowercaseAsciiCompression (apache#858)
  LUCENE-10542: FieldSource exists implementations can avoid value retrieval (apache#847)
  LUCENE-10539: Return a stream of completions from FSTCompletion. (apache#844)
  gradle 7.3.3 quick upgrade (apache#856)
  LUCENE-10530: Avoid floating point precision bug in TestTaxonomyFacetAssociations (apache#848)
  ...
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