[ 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