[
https://issues.apache.org/jira/browse/LUCENE-7737?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16061535#comment-16061535
]
David Smiley commented on LUCENE-7737:
--------------------------------------
Your move of code to fromDoubleValuesSource didn't compile; see equals(). It
was an easy fix. BTW I find IntelliJ refactoring helps with code moves/renames.
All your updates to the patch are good & explanations to my questions.
More outdated javadocs: on SerializedDVStrategy.makeShapeValueSource method and
same method in BBoxStrategy (legacy & not). You could just keep this simple
as "Provides access to a Shape per document."
Regarding the test (Geo3d) failure: In
SerializedDVStrategy.ShapeDocValueSource.ShapeValues.value when you reference
the bytesRef, you assume the offset is 0 when in fact you should reference the
offset from the bytesRef. This bug caused this ShapeValues to always read the
first shape (at offset 0) instead of the shape at the offset corresponding to
the current doc. This took me awhile to finally figure out ;-)
During my debugging, I kept looking at IntersectsRPTVerifyQuery.createWeight
where you get two TwoPhaseIterators, both with the same approximation reference
(around line ~111). I found this very confusing and thought I had found this
to be a bug but I guess it's right. My confusion is related to me not being
sure why you changed ShapeValuesPredicate to be something that returns a
TwoPhaseIterator. Perhaps you do this to express that the API is forward-only
instead of, say, returning a random-access Bits (since the underlying docValues
is also forward-only)? I guess that I can understand though I'm not sure it's
worth it over that (your call). Over here I think the logic would be a little
clearer if you declare the predFuncValues variable inside the anonymous class
of the twoPhaseIterator a few lines below. And mention in comments you're
using the same approximation for both.
> Remove spatial-extras dependency on queries
> -------------------------------------------
>
> Key: LUCENE-7737
> URL: https://issues.apache.org/jira/browse/LUCENE-7737
> Project: Lucene - Core
> Issue Type: Improvement
> Components: modules/spatial-extras
> Reporter: Alan Woodward
> Priority: Minor
> Fix For: master (7.0)
>
> Attachments: LUCENE-7737.patch, LUCENE-7737.patch, LUCENE-7737.patch
>
>
> The spatial-extras module uses ValueSources for a number of different
> purposes, requiring a dependency on the queries module. I'd like to try
> using core-only interfaces here instead, allowing us to remove the dependency
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
