[
https://issues.apache.org/jira/browse/LUCENE-7094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15209694#comment-15209694
]
David Smiley commented on LUCENE-7094:
--------------------------------------
Thanks for working on this Nick! It's nice to see spatial-extras join the new
PointValues world.
I see you added the constant SpatialStrategy.SUFFIX_DV, and that it’s used by
BBoxStrategy & PointVectorStrategy. (BTW statics should be ordered above
non-statics) I see it’s used to put DocValue data in separate field names from
the field name given to the strategy. Why this change? It would thwart
someone using this code against an index created with 5.x. In case you didn’t
know, the terms index, docValues, stored values, pointValues etc. are
independent and so a field name might have data in all of them. There’s no
need for a field name suffix to differentiate the type.
I think BBoxStrategy.ComboField (private) shouldn’t be deprecated; it’s not
quite tied to legacy numerics as the comment you added claims, although it can
support it if someone provides a field type with the index enabled. It
supports DV data, stored data, and terms index data, mostly all via the
FieldType passed in. But I don’t think it can hold the PointValue data, so
that would still be a reason to have the Field[] array have a variable size.
If you don't get my drift, I'd be happy to take over and give it a shot. Maybe
i'm overlooking something; maybe not.
The SpatialStrategy.get/setFieldType option you added is only used by
BBoxStrategy. That seems trappy to an unwitting user who doesn't know which
does. And the "needsDocValues" field is set from this, yet only for numeric
DV, not binary DV. I wonder if this field should be called
needsNumericDocValues then? And why define it here instead of pertinent
subclasses?
I see some {{false &&}} conditions added to RandomSpatialOpStrategyTestCase I
don't think you meant to have in the patch; right?
PortedSolr3Test.testIntersections: maybe move the enabling of docValues on
PointVectorStrategy out to the parameters() static method? Or perhaps...
arguably PVS should enable this by default since without it, one can't even do
a circle query.
> spatial-extras BBoxStrategy and (confusingly!) PointVectorStrategy use legacy
> numeric encoding
> ----------------------------------------------------------------------------------------------
>
> Key: LUCENE-7094
> URL: https://issues.apache.org/jira/browse/LUCENE-7094
> Project: Lucene - Core
> Issue Type: Bug
> Reporter: Robert Muir
> Assignee: Nicholas Knize
> Attachments: LUCENE-7094.patch
>
>
> We need to deprecate these since they work on the old encoding and provide
> points based alternatives.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
