nknize commented on code in PR #1017:
URL: https://github.com/apache/lucene/pull/1017#discussion_r924571326


##########
lucene/core/src/java/org/apache/lucene/document/BaseShapeDocValuesBoundingBoxQuery.java:
##########
@@ -0,0 +1,188 @@
+/*
+ * 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 java.io.IOException;
+import org.apache.lucene.document.ShapeField.QueryRelation;
+import org.apache.lucene.index.BinaryDocValues;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.index.FieldInfo;
+import org.apache.lucene.index.LeafReader;
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.index.PointValues;
+import org.apache.lucene.search.ConstantScoreScorer;
+import org.apache.lucene.search.ConstantScoreWeight;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.QueryVisitor;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.search.Scorer;
+import org.apache.lucene.search.TwoPhaseIterator;
+import org.apache.lucene.search.Weight;
+
+/**
+ * Base doc values bounding box query for {@link 
LatLonShapeDocValuesBoundingBoxQuery} and {@link
+ * XYShapeDocValuesBoundingBoxQuery}
+ */
+abstract class BaseShapeDocValuesBoundingBoxQuery extends Query {
+
+  protected final String field;
+  protected final QueryRelation queryRelation;
+  protected final int minX;
+  protected final int maxX;
+  protected final int minY;
+  protected final int maxY;
+
+  BaseShapeDocValuesBoundingBoxQuery(
+      String field, QueryRelation queryRelation, int minX, int maxX, int minY, 
int maxY) {
+    if (field == null) {
+      throw new IllegalArgumentException("field must not be null");
+    }
+    this.field = field;
+    this.queryRelation = validateRelation(queryRelation);
+    this.minX = minX;
+    this.maxX = maxX;
+    this.minY = minY;
+    this.maxY = maxY;
+  }
+
+  private static QueryRelation validateRelation(QueryRelation queryRelation) {
+    if (queryRelation == QueryRelation.CONTAINS) {
+      throw new IllegalArgumentException(
+          "ShapeDocValuesBoundingBoxQuery does not yet support CONTAINS 
queries");
+    }
+    return queryRelation;
+  }
+
+  @Override
+  public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, 
float boost)
+      throws IOException {
+    return new ConstantScoreWeight(this, boost) {
+      @Override
+      public Scorer scorer(LeafReaderContext context) throws IOException {
+        final LeafReader reader = context.reader();
+        final BinaryDocValues values = 
context.reader().getBinaryDocValues(field);
+        if (values == null) {
+          return null;
+        }
+        final FieldInfo fieldInfo = reader.getFieldInfos().fieldInfo(field);
+        if (fieldInfo == null) {
+          // No docs in this segment indexed this field at all
+          return null;
+        }
+
+        final TwoPhaseIterator iterator =
+            new TwoPhaseIterator(values) {
+              @Override
+              public boolean matches() throws IOException {
+                ShapeDocValuesField shapeDVField =
+                    new ShapeDocValuesField(field, values.binaryValue());
+                return match(shapeDVField);
+              }
+
+              @Override
+              public float matchCost() {
+                return BaseShapeDocValuesBoundingBoxQuery.this.matchCost();
+              }
+            };
+        return new ConstantScoreScorer(this, boost, scoreMode, iterator);
+      }
+
+      @Override
+      public boolean isCacheable(LeafReaderContext ctx) {
+        return DocValues.isCacheable(ctx, field);
+      }
+    };
+  }
+
+  public boolean match(ShapeDocValuesField shapeDocValues) throws IOException {
+    boolean result = matchesBox(shapeDocValues, queryRelation, minX, maxX, 
minY, maxY);
+    if (queryRelation == QueryRelation.DISJOINT) {
+      return result == false;
+    }
+    return result;
+  }
+
+  public float matchCost() {
+    return 60 * 100;

Review Comment:
   +1



##########
lucene/core/src/java/org/apache/lucene/document/BaseShapeDocValuesBoundingBoxQuery.java:
##########
@@ -0,0 +1,188 @@
+/*
+ * 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 java.io.IOException;
+import org.apache.lucene.document.ShapeField.QueryRelation;
+import org.apache.lucene.index.BinaryDocValues;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.index.FieldInfo;
+import org.apache.lucene.index.LeafReader;
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.index.PointValues;
+import org.apache.lucene.search.ConstantScoreScorer;
+import org.apache.lucene.search.ConstantScoreWeight;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.QueryVisitor;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.search.Scorer;
+import org.apache.lucene.search.TwoPhaseIterator;
+import org.apache.lucene.search.Weight;
+
+/**
+ * Base doc values bounding box query for {@link 
LatLonShapeDocValuesBoundingBoxQuery} and {@link
+ * XYShapeDocValuesBoundingBoxQuery}
+ */
+abstract class BaseShapeDocValuesBoundingBoxQuery extends Query {
+
+  protected final String field;
+  protected final QueryRelation queryRelation;
+  protected final int minX;
+  protected final int maxX;
+  protected final int minY;
+  protected final int maxY;
+
+  BaseShapeDocValuesBoundingBoxQuery(
+      String field, QueryRelation queryRelation, int minX, int maxX, int minY, 
int maxY) {
+    if (field == null) {
+      throw new IllegalArgumentException("field must not be null");
+    }
+    this.field = field;
+    this.queryRelation = validateRelation(queryRelation);
+    this.minX = minX;
+    this.maxX = maxX;
+    this.minY = minY;
+    this.maxY = maxY;
+  }
+
+  private static QueryRelation validateRelation(QueryRelation queryRelation) {
+    if (queryRelation == QueryRelation.CONTAINS) {
+      throw new IllegalArgumentException(
+          "ShapeDocValuesBoundingBoxQuery does not yet support CONTAINS 
queries");
+    }
+    return queryRelation;
+  }
+
+  @Override
+  public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, 
float boost)
+      throws IOException {
+    return new ConstantScoreWeight(this, boost) {
+      @Override
+      public Scorer scorer(LeafReaderContext context) throws IOException {
+        final LeafReader reader = context.reader();
+        final BinaryDocValues values = 
context.reader().getBinaryDocValues(field);
+        if (values == null) {
+          return null;
+        }
+        final FieldInfo fieldInfo = reader.getFieldInfos().fieldInfo(field);
+        if (fieldInfo == null) {
+          // No docs in this segment indexed this field at all
+          return null;
+        }
+
+        final TwoPhaseIterator iterator =
+            new TwoPhaseIterator(values) {
+              @Override
+              public boolean matches() throws IOException {
+                ShapeDocValuesField shapeDVField =
+                    new ShapeDocValuesField(field, values.binaryValue());
+                return match(shapeDVField);
+              }
+
+              @Override
+              public float matchCost() {
+                return BaseShapeDocValuesBoundingBoxQuery.this.matchCost();
+              }
+            };
+        return new ConstantScoreScorer(this, boost, scoreMode, iterator);
+      }
+
+      @Override
+      public boolean isCacheable(LeafReaderContext ctx) {
+        return DocValues.isCacheable(ctx, field);
+      }
+    };
+  }
+
+  public boolean match(ShapeDocValuesField shapeDocValues) throws IOException {

Review Comment:
   yes.. and this method is being refactored away in a follow up PR when 
general geometries are supported.



##########
lucene/core/src/java/org/apache/lucene/document/ShapeDocValuesField.java:
##########
@@ -0,0 +1,896 @@
+/*
+ * 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 java.io.IOException;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.List;
+import org.apache.lucene.analysis.Analyzer;
+import org.apache.lucene.analysis.TokenStream;
+import org.apache.lucene.document.ShapeField.DecodedTriangle.TYPE;
+import org.apache.lucene.document.ShapeField.QueryRelation;
+import org.apache.lucene.document.SpatialQuery.EncodedRectangle;
+import org.apache.lucene.index.DocValuesType;
+import org.apache.lucene.index.IndexableFieldType;
+import org.apache.lucene.index.PointValues.Relation;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.store.ByteArrayDataInput;
+import org.apache.lucene.store.ByteBuffersDataOutput;
+import org.apache.lucene.store.DataInput;
+import org.apache.lucene.util.ArrayUtil;
+import org.apache.lucene.util.BytesRef;
+
+/** A doc values field representation for {@link LatLonShape} and {@link 
XYShape} */
+public final class ShapeDocValuesField extends Field {

Review Comment:
   I opted to go this route because the doc value `Field` is just a serialized 
binary array and passing the field data to a separate Utility class that 
reads/interprets the bytes at search and aggregation time felt clumsy (to me). 
IMHO it made more "object oriented" sense to keep the reader/writer self 
contained to the field class that uses it and add the (centroid / bounding box 
/ etc) interpretive "getters" directly to that field. 
   
   If it's cleaner to other users I can go back to my original design like 
`LatLonShape` and refactor the reader/writer to a `ShapeDocValues` utility 
class and leave `ShapeDocValuesField` in it's own class to just provide the 
index time data accessors (e.g., `byteValue`)?  



-- 
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

Reply via email to