You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by "David Smiley (JIRA)" <ji...@apache.org> on 2016/03/24 04:56:25 UTC
[jira] [Commented] (LUCENE-7094) spatial-extras BBoxStrategy and
(confusingly!) PointVectorStrategy use legacy numeric encoding
[ 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org