[
https://issues.apache.org/jira/browse/LUCENE-6930?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15119397#comment-15119397
]
Michael McCandless commented on LUCENE-6930:
--------------------------------------------
Thanks [~nknize], this is a nice (and large!) change.
The sole "R" in this javadoc left me hanging a bit ;)
{noformat}
/**
* GeoTerms are coded as the following:
*
* R
*/
{noformat}
Can you update {{GeoPointDistanceQuery}} javadocs explaining the max
radius limit? I.e. that the circle projected on the earth's surface
cannot wrap around and touch itself again (if I understand that
right!)?
Can we move {{GeoPointTokenStream}} under {{o.a.l.document}} and make
it package private? (And make {{TermEncoding}} public elsewhere.)
Can all other ctors of {{GeoPointField}} just forward to the primary
("takes everything") ctor call, i.e. call {{this(...)}} instead of
{{super(...)}}? Also, can we break this compound ternary operator
into a static helper method?:
{noformat}
super(name, stored == Store.YES ?
termEncoding == GeoPointTokenStream.TermEncoding.PREFIX ?
PREFIX_TYPE_STORED : NUMERIC_TYPE_STORED :
termEncoding == GeoPointTokenStream.TermEncoding.PREFIX ?
PREFIX_TYPE_NOT_STORED : NUMERIC_TYPE_NOT_STORED);
{noformat}
E.g. maybe {{getFieldType}}.
Should it be an error if you pass a custom {{FieldType}} to
{{GeoPointField}} that disabled indexing? I.e. catch that up front,
where we check DV type and numeric type, and then remove this:
{noformat}
if (fieldType().indexOptions() == IndexOptions.NONE) {
// Not indexed
return null;
}
{noformat}
from the {{tokenStream}} method?
Can we deprecate the {{GeoPointField}} ctors that take
{{TermEncoding}}? (You should use/migrate to the default ctor that
uses the better {{PREFIX}} encoding).
{{GeoUtils.longToByteArray}} and {{.fromByteArray}} and
{{GeoEncodingUtils.geoTermToString}} look dead?
This comment confuses me:
{noformat}
// start shift at 61
private short shift;
{noformat}
Does it really start at 61? Seems like ({{computeMaxShift}}) it's
either 45 (for a large bbox) or 36 (for a not-large bbox)? Can we
move the comment down to where we actually do assign to shift?
> Decouple GeoPointField from NumericType
> ---------------------------------------
>
> Key: LUCENE-6930
> URL: https://issues.apache.org/jira/browse/LUCENE-6930
> Project: Lucene - Core
> Issue Type: Improvement
> Reporter: Nicholas Knize
> Attachments: LUCENE-6930.patch, LUCENE-6930.patch, LUCENE-6930.patch,
> LUCENE-6930.patch, LUCENE-6930.patch
>
>
> {{GeoPointField}} currently relies on {{NumericTokenStream}} to create prefix
> terms for a GeoPoint using the precision step defined in {{GeoPointField}}.
> At search time {{GeoPointTermsEnum}} recurses to a max precision that is
> computed by the Query parameters. This max precision is never the full
> precision, so creating and indexing the full precision terms is useless and
> wasteful (it was always a side effect of just using indexing logic from the
> Numeric type).
> Furthermore, since the numerical logic always stored high precision terms
> first, the recursion in {{GeoPointTermsEnum}} required transient memory for
> storing ranges. By moving the trie logic to its own {{GeoPointTokenStream}}
> and reversing the term order (such that lower resolution terms are first),
> the GeoPointTermsEnum can naturally traverse, enabling on-demand creation of
> PrefixTerms. This will be done in a separate issue.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
