nknize commented on code in PR #12162: URL: https://github.com/apache/lucene/pull/12162#discussion_r1114644266
########## lucene/core/src/java/org/apache/lucene/document/LatLonField.java: ########## @@ -0,0 +1,318 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.lucene.document; + +import static org.apache.lucene.geo.GeoEncodingUtils.decodeLatitude; +import static org.apache.lucene.geo.GeoEncodingUtils.decodeLongitude; +import static org.apache.lucene.geo.GeoEncodingUtils.encodeLatitude; +import static org.apache.lucene.geo.GeoEncodingUtils.encodeLongitude; + +import java.io.IOException; +import org.apache.lucene.geo.LatLonGeometry; +import org.apache.lucene.geo.Point; +import org.apache.lucene.geo.Polygon; +import org.apache.lucene.index.DocValuesType; +import org.apache.lucene.search.BooleanClause.Occur; +import org.apache.lucene.search.BooleanQuery; +import org.apache.lucene.search.FieldDoc; +import org.apache.lucene.search.IndexOrDocValuesQuery; +import org.apache.lucene.search.IndexSearcher; +import org.apache.lucene.search.MatchAllDocsQuery; +import org.apache.lucene.search.MatchNoDocsQuery; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.SortField; +import org.apache.lucene.search.TopFieldDocs; +import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.NumericUtils; + +/** + * An indexed location field for querying and sorting. If you need more fine-grained control you can + * use {@link LatLonPoint} and {@link LatLonDocValuesField}. + * + * <p>Finding all documents within a range at search time is efficient. Multiple values for the same + * field in one document is allowed. + * + * <p>This field defines static factory methods for common operations: + * + * <ul> + * <li>{@link #newBoxQuery newBoxQuery()} for matching points within a bounding box. + * <li>{@link #newDistanceQuery newDistanceQuery()} for matching points within a specified + * distance. + * <li>{@link #newPolygonQuery newPolygonQuery()} for matching points within an arbitrary polygon. + * <li>{@link #newGeometryQuery newGeometryQuery()} for matching points complying with a spatial + * relationship with an arbitrary geometry. + * <li>{@link #newDistanceFeatureQuery newDistanceFeatureQuery()} for returning points scored by + * distance to a specified location. + * <li>{@link #nearest nearest()} for returning the nearest points from a specified location. + * <li>{@link #newDistanceSort newDistanceSort()} for ordering documents by distance from a + * specified location. + * </ul> + * + * <p>If you also need to store the value, you should add a separate {@link StoredField} instance. + * + * <p><b>WARNING</b>: Values are indexed with some loss of precision from the original {@code + * double} values (4.190951585769653E-8 for the latitude component and 8.381903171539307E-8 for + * longitude). + * + * @see LatLonPoint + * @see LatLonDocValuesField + */ +public class LatLonField extends Field { Review Comment: These class names are getting super inconsistent. Since this targets points can we refactor this to `LatLonPointField` so as to not confuse general `LatLonField` w/ `ShapeField`? I think this also makes it more consistent w/ `LatLonPointSortField`. ########## lucene/core/src/java/org/apache/lucene/geo/LatLonGeometry.java: ########## @@ -17,7 +17,23 @@ package org.apache.lucene.geo; -/** Lat/Lon Geometry object. */ +/** + * Represents a unified view of the supported geometries on the earth's surface. They can determine + * their spatial relationship against data indexed with {@link + * org.apache.lucene.document.LatLonField}, {@link org.apache.lucene.document.LatLonPoint} and + * {@link org.apache.lucene.document.LatLonDocValuesField}. + * + * <p>The supported geometries are: + * + * <ol> + * <li>{@link Point} + * <li>{@link Line} + * <li>{@link Polygon} + * <li>{@link Circle} + * </ol> + * + * @lucene.experimental + */ public abstract class LatLonGeometry extends Geometry { Review Comment: In the spirit of internal vs public facing APIs, is there a need to keep this class public? This is a generic contract to enable `Circle`, `Line`, `Point`, `Polygon`, `Rectangle` geometries to implement `Component2D` comparisons. I don't think we need/want to expose this to downstream projects implementing their own geometries? ########## lucene/core/src/java/org/apache/lucene/document/LatLonField.java: ########## @@ -0,0 +1,318 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.lucene.document; + +import static org.apache.lucene.geo.GeoEncodingUtils.decodeLatitude; +import static org.apache.lucene.geo.GeoEncodingUtils.decodeLongitude; +import static org.apache.lucene.geo.GeoEncodingUtils.encodeLatitude; +import static org.apache.lucene.geo.GeoEncodingUtils.encodeLongitude; + +import java.io.IOException; +import org.apache.lucene.geo.LatLonGeometry; +import org.apache.lucene.geo.Point; +import org.apache.lucene.geo.Polygon; +import org.apache.lucene.index.DocValuesType; +import org.apache.lucene.search.BooleanClause.Occur; +import org.apache.lucene.search.BooleanQuery; +import org.apache.lucene.search.FieldDoc; +import org.apache.lucene.search.IndexOrDocValuesQuery; +import org.apache.lucene.search.IndexSearcher; +import org.apache.lucene.search.MatchAllDocsQuery; +import org.apache.lucene.search.MatchNoDocsQuery; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.SortField; +import org.apache.lucene.search.TopFieldDocs; +import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.NumericUtils; + +/** + * An indexed location field for querying and sorting. If you need more fine-grained control you can + * use {@link LatLonPoint} and {@link LatLonDocValuesField}. + * + * <p>Finding all documents within a range at search time is efficient. Multiple values for the same + * field in one document is allowed. + * + * <p>This field defines static factory methods for common operations: + * + * <ul> + * <li>{@link #newBoxQuery newBoxQuery()} for matching points within a bounding box. + * <li>{@link #newDistanceQuery newDistanceQuery()} for matching points within a specified + * distance. + * <li>{@link #newPolygonQuery newPolygonQuery()} for matching points within an arbitrary polygon. + * <li>{@link #newGeometryQuery newGeometryQuery()} for matching points complying with a spatial + * relationship with an arbitrary geometry. + * <li>{@link #newDistanceFeatureQuery newDistanceFeatureQuery()} for returning points scored by + * distance to a specified location. + * <li>{@link #nearest nearest()} for returning the nearest points from a specified location. + * <li>{@link #newDistanceSort newDistanceSort()} for ordering documents by distance from a + * specified location. + * </ul> + * + * <p>If you also need to store the value, you should add a separate {@link StoredField} instance. + * + * <p><b>WARNING</b>: Values are indexed with some loss of precision from the original {@code + * double} values (4.190951585769653E-8 for the latitude component and 8.381903171539307E-8 for + * longitude). + * + * @see LatLonPoint + * @see LatLonDocValuesField + */ +public class LatLonField extends Field { + /** LatLonPoint is encoded as integer values so number of bytes is 4 */ + public static final int BYTES = Integer.BYTES; + /** + * Type for an indexed LatLonPoint + * + * <p>Each point stores two dimensions with 4 bytes per dimension. + */ + public static final FieldType TYPE = new FieldType(); + + static { + TYPE.setDimensions(2, Integer.BYTES); + TYPE.setDocValuesType(DocValuesType.SORTED_NUMERIC); + TYPE.freeze(); + } + + // holds the doc value value. + private long docValue; + + /** + * Change the values of this field + * + * @param latitude latitude value: must be within standard +/-90 coordinate bounds. + * @param longitude longitude value: must be within standard +/-180 coordinate bounds. + * @throws IllegalArgumentException if latitude or longitude are out of bounds + */ + public void setLocationValue(double latitude, double longitude) { + final byte[] bytes; + + if (fieldsData == null) { + bytes = new byte[8]; + fieldsData = new BytesRef(bytes); + } else { + bytes = ((BytesRef) fieldsData).bytes; + } + + int latitudeEncoded = encodeLatitude(latitude); + int longitudeEncoded = encodeLongitude(longitude); + NumericUtils.intToSortableBytes(latitudeEncoded, bytes, 0); + NumericUtils.intToSortableBytes(longitudeEncoded, bytes, Integer.BYTES); + docValue = Long.valueOf((((long) latitudeEncoded) << 32) | (longitudeEncoded & 0xFFFFFFFFL)); + } + + @Override + public Number numericValue() { + return docValue; + } + + /** + * Creates a new LatLonPoint with the specified latitude and longitude + * + * @param name field name + * @param latitude latitude value: must be within standard +/-90 coordinate bounds. + * @param longitude longitude value: must be within standard +/-180 coordinate bounds. + * @throws IllegalArgumentException if the field name is null or latitude or longitude are out of + * bounds + */ + public LatLonField(String name, double latitude, double longitude) { + super(name, TYPE); + setLocationValue(latitude, longitude); + } + + @Override + public String toString() { Review Comment: This just occurred to me, should we also implement `equals` and `hashcode` contracts? Looks like the companion PR didn't do this, and we don't historically implement these contracts, I'm just wondering if we should do our diligence to ensure deep comparisons? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
