You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2022/11/07 04:10:44 UTC
[GitHub] [lucene] mdmarshmallow opened a new pull request, #11901: Github#11869: Add RangeOnRangeFacetCounts
mdmarshmallow opened a new pull request, #11901:
URL: https://github.com/apache/lucene/pull/11901
### Description
<!--
If this is your first contribution to Lucene, please make sure you have reviewed the contribution guide.
https://github.com/apache/lucene/blob/main/CONTRIBUTING.md
-->
Issue: https://github.com/apache/lucene/issues/11869
Adds `RangeOnRangeFacetCounts` which supports double ranges and long ranges to mirror `RangeFacetCounts`. Currently, this does not have support for multivalues fields and uses basic linear scanning to count the facets.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mdmarshmallow commented on a diff in pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
mdmarshmallow commented on code in PR #11901:
URL: https://github.com/apache/lucene/pull/11901#discussion_r1044919888
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/Range.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.util.Arrays;
+
+/**
+ * Base class for a single labeled range.
+ *
+ * @lucene.experimental
+ */
+public abstract class Range {
+
+ /** Label that identifies this range. */
+ public final String label;
+
+ /** Number of dims per range */
+ public final int dims;
+
+ /** Sole constructor. */
+ protected Range(String label, int dims) {
+ if (label == null) {
+ throw new NullPointerException("label must not be null");
+ }
+ this.label = label;
+ this.dims = dims;
+ }
+
+ /** Invoke this for a useless range. */
+ protected void failNoMatch() {
+ throw new IllegalArgumentException("range \"" + label + "\" matches nothing");
+ }
+
+ /** Gets a corresponding array of labels given a range */
+ public static String[] getLabelsFromRanges(Range[] ranges) {
Review Comment:
Changed in next rev
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mdmarshmallow commented on pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
mdmarshmallow commented on PR #11901:
URL: https://github.com/apache/lucene/pull/11901#issuecomment-1320700985
Ah yeah good point on the FacetSets... so I actually already use `LongRangeDocValueFields` here: public class `LongRangeDocValuesFacetField extends LongRangeDocValuesField`. The difference is that the `LongRangeDocValuesFacetField` constructor enforces a single dimension. I'd imagine if we want to extend it, it shouldn't be that hard to just add another constructor here.
With regards to this though:
>As for how to handle faceting of multi-dim ranges, I think the logic would be the same as the "slow" query you reference, and would depend on the RangeFieldQuery#QueryType specified.
I think I will need to change this PR a bit then. Right now what happens is that the query can have multiple ranges, which all need to "match" the stored range, which matches the "cross product" idea I talked about earlier which we won't be doing. So the way to go from here I think would be to either enforce only one range in the query, or just go ahead and do the multidimensional implementation. I think the latter makes more sense since I will need to change code either way.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mdmarshmallow commented on a diff in pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
mdmarshmallow commented on code in PR #11901:
URL: https://github.com/apache/lucene/pull/11901#discussion_r1044928215
##########
lucene/core/src/java/org/apache/lucene/document/LongRangeDocValuesField.java:
##########
@@ -86,4 +87,28 @@ private static void checkArgs(final long[] min, final long[] max) {
}
}
}
+
+ /**
+ * Sets the value of this field to a new value
+ *
+ * @param min the new min
+ * @param max the new max
+ */
+ public void setLongRangeValue(long min, long max) {
Review Comment:
I just used `setBytesValue` directly in the unit test which works. Removed the setter for the next revision.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gsmiller commented on pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
gsmiller commented on PR #11901:
URL: https://github.com/apache/lucene/pull/11901#issuecomment-1317848376
@mdmarshmallow started to dig into this. Will get you some feedback this week. Just letting you know it's on my radar. Thanks for all the hard work on this!
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mdmarshmallow commented on a diff in pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
mdmarshmallow commented on code in PR #11901:
URL: https://github.com/apache/lucene/pull/11901#discussion_r1046128095
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,209 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.ArrayUtil;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final byte[][] encodedRanges;
+ private final String[] labels;
+ private final int numEncodedValueBytes;
+ private final int dims;
+
+ /** Counts, initialized in by subclass. */
+ protected final int[] counts;
+
+ /** Our field name. */
+ protected final String field;
+
+ /** Total number of hits. */
+ protected int totCount;
+
+ private final ArrayUtil.ByteArrayComparator comparator;
+
+ /** Type of "range overlap" we want to count. */
+ RangeFieldQuery.QueryType queryType;
+
+ protected RangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ Query fastMatchQuery,
+ int numEncodedValueBytes,
+ byte[][] encodedRanges,
+ String[] labels)
+ throws IOException {
+ super(fastMatchQuery);
+
+ assert encodedRanges.length == labels.length;
+ assert encodedRanges[0].length % (2 * numEncodedValueBytes) == 0;
+
+ this.encodedRanges = encodedRanges;
+ this.field = field;
+ this.labels = labels;
+ this.numEncodedValueBytes = numEncodedValueBytes;
+ this.dims = encodedRanges[0].length / (2 * this.numEncodedValueBytes);
+ this.queryType = queryType;
+ this.comparator = ArrayUtil.getUnsignedComparator(this.numEncodedValueBytes);
Review Comment:
Changed this in the next revision
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mdmarshmallow commented on a diff in pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
mdmarshmallow commented on code in PR #11901:
URL: https://github.com/apache/lucene/pull/11901#discussion_r1046130284
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/LongRange.java:
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.util.Arrays;
+import java.util.Objects;
+
+/** Represents a long range for RangeOnRange faceting */
+public class LongRange extends Range {
+ /** Minimum (inclusive). */
+ public final long[] min;
+
+ /** Maximum (inclusive). */
+ public final long[] max;
+
+ /**
+ * Represents a single dimensional long range for RangeOnRange faceting
+ *
+ * @param label the name of the range
+ * @param minIn the minimum
+ * @param minInclusive if the minimum is inclusive
+ * @param maxIn the maximum
+ * @param maxInclusive if the maximum is inclusive
+ */
+ public LongRange(
+ String label, long minIn, boolean minInclusive, long maxIn, boolean maxInclusive) {
+ super(label, 1);
+
+ if (minInclusive == false) {
+ if (minIn != Long.MAX_VALUE) {
+ minIn++;
+ } else {
+ failNoMatch();
+ }
+ }
+
+ if (maxInclusive == false) {
+ if (maxIn != Long.MIN_VALUE) {
+ maxIn--;
+ } else {
+ failNoMatch();
+ }
+ }
+
+ if (minIn > maxIn) {
+ failNoMatch();
+ }
+
+ this.min = new long[] {minIn};
+ this.max = new long[] {maxIn};
+ }
+
+ /**
+ * Represents a multidimensional long range for RangeOnRange faceting
+ *
+ * @param label the name of the range
+ * @param min the minimum, inclusive
+ * @param max the maximum, inclusive
+ */
+ public LongRange(String label, long[] min, long[] max) {
+ super(label, min.length);
+ checkArgs(min, max);
+ this.min = min;
+ this.max = max;
+ }
+
+ @Override
+ public String toString() {
+ return "LongRange(label: "
+ + label
+ + ", min: "
+ + Arrays.toString(min)
+ + ", max: "
+ + Arrays.toString(max)
+ + ")";
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ LongRange longRange = (LongRange) o;
+ return Arrays.equals(min, longRange.min) && Arrays.equals(max, longRange.max);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(label, Arrays.hashCode(min), Arrays.hashCode(max));
+ }
+
+ private static void checkArgs(final long[] min, final long[] max) {
+ if (min == null || max == null || min.length == 0 || max.length == 0) {
+ throw new IllegalArgumentException("min/max range values cannot be null or empty");
+ }
+ if (min.length != max.length) {
+ throw new IllegalArgumentException("min/max ranges must agree");
+ }
+
+ for (int i = 0; i < min.length; i++) {
+ if (min[i] > max[i]) {
+ throw new IllegalArgumentException("min should be less than max");
Review Comment:
Yeah I think that makes sense, changing in next revision.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gsmiller commented on a diff in pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
gsmiller commented on code in PR #11901:
URL: https://github.com/apache/lucene/pull/11901#discussion_r1044547958
##########
lucene/core/src/java/org/apache/lucene/document/LongRange.java:
##########
@@ -61,6 +61,17 @@ public LongRange(String name, final long[] min, final long[] max) {
setRangeValues(min, max);
}
+ /**
+ * Create a new LongRange type with one dimension
+ *
+ * @param name field name. must not be null.
+ * @param min min value
+ * @param max max value
+ */
+ public LongRange(String name, final long min, final long max) {
Review Comment:
I like the idea of adding these (along with adding single-valued ctors to the Field classes as mentioned elsewhere), but let's do it in a separate PR since it's not strictly required for what you're building here. I think that would help keep the change tight and allow future "digital archeologists" to understand the work better.
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,207 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final Range[] ranges;
+ private final String[] labels;
Review Comment:
Is there a need to separately store the labels here? Could we just reference the `Range[] range` array instead to get the labels as needed?
##########
lucene/core/src/java/org/apache/lucene/document/LongRangeDocValuesFacetField.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * 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 org.apache.lucene.util.BytesRef;
+
+/**
+ * A subclass of LongRangeDocValuesField that only allows single dimension range representation.
+ * Meant to be used with RangeOnRange faceting
+ */
+public class LongRangeDocValuesFacetField extends LongRangeDocValuesField {
Review Comment:
I'm not sure we need to create new Field classes for faceting. Can we simplify things a bit here and implement range faceting for the existing range fields? For example, if a user wants to facet on long-based ranges, can they just index a `LongRangeDocValuesField` directly and we facet over that?
Looking at this subclass definition, it seems like the added value is just the single-dim ctor. Is that right? I think that's nice syntactic sugar, but maybe we add it directly to `LongRangeDocValuesField` instead? (If we want to do that, I'd suggest doing that in a separate PR just to separate things a bit).
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,207 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final Range[] ranges;
+ private final String[] labels;
+ private final int numEncodedValueBytes;
+ private final int dims;
+ private final int numRanges;
+
+ /** Counts, initialized in by subclass. */
+ protected final int[] counts;
+
+ /** Our field name. */
+ protected final String field;
+
+ /** Total number of hits. */
+ protected int totCount;
+
+ /** Type of "range overlap" we want to count. */
+ RangeFieldQuery.QueryType queryType;
+
+ protected RangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ Query fastMatchQuery,
+ Range... ranges)
+ throws IOException {
+ super(fastMatchQuery);
+ this.field = field;
+ this.ranges = ranges;
+ this.numEncodedValueBytes = ranges[0].getEncodedValueBytes();
+ this.dims = ranges[0].dims;
+ this.labels = getLabels(ranges);
+ this.numRanges = ranges.length;
+ this.queryType = queryType;
+ counts = new int[numRanges];
+ count(field, hits.getMatchingDocs());
+ }
+
+ /** Counts from the provided field. */
+ protected void count(String field, List<FacetsCollector.MatchingDocs> matchingDocs)
+ throws IOException {
+
+ int missingCount = 0;
+
+ for (int i = 0; i < matchingDocs.size(); i++) {
+
+ FacetsCollector.MatchingDocs hits = matchingDocs.get(i);
+ BinaryRangeDocValues binaryRangeDocValues =
+ new BinaryRangeDocValues(
+ DocValues.getBinary(hits.context.reader(), field), dims, numEncodedValueBytes);
+
+ final DocIdSetIterator it = createIterator(hits);
+ if (it == null) {
+ continue;
+ }
+
+ totCount += hits.totalHits;
+ for (int doc = it.nextDoc(); doc != DocIdSetIterator.NO_MORE_DOCS; ) {
+ if (binaryRangeDocValues.advanceExact(doc)) {
+ boolean hasValidRange = false;
+ for (int range = 0; range < numRanges; range++) {
+ byte[] encodedRange = getEncodedRange(ranges[range]);
+ byte[] packedRange = binaryRangeDocValues.getPackedValue();
+ assert encodedRange.length == packedRange.length;
+ boolean matchesRange = true;
+ for (int dim = 0; dim < dims; dim++) {
+ if (queryType.matches(encodedRange, packedRange, dims, numEncodedValueBytes, dim)
+ == false) {
+ matchesRange = false;
+ break;
+ }
+ }
+ if (matchesRange) {
+ counts[range]++;
+ hasValidRange = true;
+ }
+ }
+ if (hasValidRange == false) {
+ missingCount++;
+ }
+ } else {
+ missingCount++;
+ }
+ doc = it.nextDoc();
+ }
+ }
+ totCount -= missingCount;
+ }
+
+ @Override
+ public FacetResult getAllChildren(String dim, String... path) throws IOException {
Review Comment:
Can you add some javadoc explaining that we will guarantee that children will be returned in the order the ranges are provided? See the javadoc in `LongRangeFacetCounts` for an example. In general, this method doesn't guarantee any ordering, but it's useful to have the guaranteed ordering property for these cases where users provide ranges in a specific order.
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,207 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final Range[] ranges;
+ private final String[] labels;
+ private final int numEncodedValueBytes;
+ private final int dims;
+ private final int numRanges;
+
+ /** Counts, initialized in by subclass. */
+ protected final int[] counts;
+
+ /** Our field name. */
+ protected final String field;
+
+ /** Total number of hits. */
+ protected int totCount;
+
+ /** Type of "range overlap" we want to count. */
+ RangeFieldQuery.QueryType queryType;
+
+ protected RangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ Query fastMatchQuery,
+ Range... ranges)
+ throws IOException {
+ super(fastMatchQuery);
+ this.field = field;
+ this.ranges = ranges;
+ this.numEncodedValueBytes = ranges[0].getEncodedValueBytes();
+ this.dims = ranges[0].dims;
+ this.labels = getLabels(ranges);
+ this.numRanges = ranges.length;
+ this.queryType = queryType;
+ counts = new int[numRanges];
+ count(field, hits.getMatchingDocs());
+ }
+
+ /** Counts from the provided field. */
+ protected void count(String field, List<FacetsCollector.MatchingDocs> matchingDocs)
+ throws IOException {
+
+ int missingCount = 0;
+
+ for (int i = 0; i < matchingDocs.size(); i++) {
+
+ FacetsCollector.MatchingDocs hits = matchingDocs.get(i);
+ BinaryRangeDocValues binaryRangeDocValues =
+ new BinaryRangeDocValues(
+ DocValues.getBinary(hits.context.reader(), field), dims, numEncodedValueBytes);
+
+ final DocIdSetIterator it = createIterator(hits);
+ if (it == null) {
+ continue;
+ }
+
+ totCount += hits.totalHits;
+ for (int doc = it.nextDoc(); doc != DocIdSetIterator.NO_MORE_DOCS; ) {
+ if (binaryRangeDocValues.advanceExact(doc)) {
+ boolean hasValidRange = false;
+ for (int range = 0; range < numRanges; range++) {
+ byte[] encodedRange = getEncodedRange(ranges[range]);
Review Comment:
Let's pull the encoding out of the loop. What if we encoded all the ranges up-front before entering the loop? One way we could do this is have the ctors actually require the encoded ranges to be provided, and then the concrete subclasses could encode up-front before calling `super` and there wouldn't be a need to expose this `getEncodedRange` method at all.
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/LongRangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * 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.facet.rangeonrange;
+
+import static org.apache.lucene.document.LongRange.verifyAndEncode;
+
+import java.io.IOException;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.search.Query;
+
+/** Represents counts for long range on range faceting */
+public class LongRangeOnRangeFacetCounts extends RangeOnRangeFacetCounts {
+
+ /**
+ * Represents counts for long range on range faceting (inclusive)
+ *
+ * @param field document's field
+ * @param hits hits we want the counts of
+ * @param queryType type of intersection we want to count
+ * @param ranges ranges we want the counts of
+ * @throws IOException low level exception
+ */
+ public LongRangeOnRangeFacetCounts(
+ String field, FacetsCollector hits, RangeFieldQuery.QueryType queryType, LongRange... ranges)
+ throws IOException {
+ super(field, hits, queryType, null, ranges);
+ }
+
+ /**
+ * Represents counts for long range on range faceting
+ *
+ * @param field document's field
+ * @param hits hits we want the counts of
+ * @param queryType type of intersection we want to count
+ * @param fastMatchQuery query to quickly discard hits
+ * @param ranges ranges we want the counts of
+ * @throws IOException low level exception
+ */
+ public LongRangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ Query fastMatchQuery,
+ LongRange... ranges)
+ throws IOException {
+ super(field, hits, queryType, fastMatchQuery, ranges);
+ }
+
+ @Override
+ public byte[] getEncodedRange(Range range) {
Review Comment:
What about changing the ctor of `RangeOnRangeFacetCounts` to accept the already-encoded ranges instead of exposing this callback method? Then the ctors in `LongRangeOnRangeFacetCounts` could just do the encoding "privately" and pass the encoded ranges to super()?
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,207 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final Range[] ranges;
+ private final String[] labels;
+ private final int numEncodedValueBytes;
+ private final int dims;
+ private final int numRanges;
+
+ /** Counts, initialized in by subclass. */
+ protected final int[] counts;
+
+ /** Our field name. */
+ protected final String field;
+
+ /** Total number of hits. */
+ protected int totCount;
+
+ /** Type of "range overlap" we want to count. */
+ RangeFieldQuery.QueryType queryType;
+
+ protected RangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ Query fastMatchQuery,
+ Range... ranges)
+ throws IOException {
+ super(fastMatchQuery);
+ this.field = field;
+ this.ranges = ranges;
+ this.numEncodedValueBytes = ranges[0].getEncodedValueBytes();
+ this.dims = ranges[0].dims;
+ this.labels = getLabels(ranges);
+ this.numRanges = ranges.length;
+ this.queryType = queryType;
+ counts = new int[numRanges];
+ count(field, hits.getMatchingDocs());
+ }
+
+ /** Counts from the provided field. */
+ protected void count(String field, List<FacetsCollector.MatchingDocs> matchingDocs)
Review Comment:
Maybe add a TODO somewhere describing the idea that we may be able to do the counting more efficiently than exhaustively checking all ranges for each doc? Seems like we could probably leverage some sort of space-partitioning data structure here to identify "matching" ranges to count for each doc instead of just looping over all of them linearly?
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,207 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final Range[] ranges;
+ private final String[] labels;
+ private final int numEncodedValueBytes;
+ private final int dims;
+ private final int numRanges;
+
+ /** Counts, initialized in by subclass. */
+ protected final int[] counts;
+
+ /** Our field name. */
+ protected final String field;
+
+ /** Total number of hits. */
+ protected int totCount;
+
+ /** Type of "range overlap" we want to count. */
+ RangeFieldQuery.QueryType queryType;
+
+ protected RangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ Query fastMatchQuery,
+ Range... ranges)
+ throws IOException {
+ super(fastMatchQuery);
+ this.field = field;
+ this.ranges = ranges;
+ this.numEncodedValueBytes = ranges[0].getEncodedValueBytes();
+ this.dims = ranges[0].dims;
+ this.labels = getLabels(ranges);
+ this.numRanges = ranges.length;
+ this.queryType = queryType;
+ counts = new int[numRanges];
+ count(field, hits.getMatchingDocs());
+ }
+
+ /** Counts from the provided field. */
+ protected void count(String field, List<FacetsCollector.MatchingDocs> matchingDocs)
+ throws IOException {
+
+ int missingCount = 0;
+
+ for (int i = 0; i < matchingDocs.size(); i++) {
+
+ FacetsCollector.MatchingDocs hits = matchingDocs.get(i);
+ BinaryRangeDocValues binaryRangeDocValues =
+ new BinaryRangeDocValues(
+ DocValues.getBinary(hits.context.reader(), field), dims, numEncodedValueBytes);
+
+ final DocIdSetIterator it = createIterator(hits);
+ if (it == null) {
+ continue;
+ }
+
+ totCount += hits.totalHits;
+ for (int doc = it.nextDoc(); doc != DocIdSetIterator.NO_MORE_DOCS; ) {
+ if (binaryRangeDocValues.advanceExact(doc)) {
+ boolean hasValidRange = false;
+ for (int range = 0; range < numRanges; range++) {
+ byte[] encodedRange = getEncodedRange(ranges[range]);
+ byte[] packedRange = binaryRangeDocValues.getPackedValue();
+ assert encodedRange.length == packedRange.length;
+ boolean matchesRange = true;
+ for (int dim = 0; dim < dims; dim++) {
+ if (queryType.matches(encodedRange, packedRange, dims, numEncodedValueBytes, dim)
Review Comment:
There's a version of `QueryType#matches` that checks all dims instead of just a single dim. Let's use that so you don't need to re-implement the loop over all the dims.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gsmiller commented on a diff in pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
gsmiller commented on code in PR #11901:
URL: https://github.com/apache/lucene/pull/11901#discussion_r1044897144
##########
lucene/core/src/java/org/apache/lucene/document/LongRangeDocValuesField.java:
##########
@@ -86,4 +87,28 @@ private static void checkArgs(final long[] min, final long[] max) {
}
}
}
+
+ /**
+ * Sets the value of this field to a new value
+ *
+ * @param min the new min
+ * @param max the new max
+ */
+ public void setLongRangeValue(long min, long max) {
Review Comment:
Right, but they're optional for fields (and not required for users or tests). Have a look at the javadoc for `Field#setStringValue`. I think it will make it a bit more clear. For your unit tests specifically, you can just create new field instances right?
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gsmiller commented on a diff in pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
gsmiller commented on code in PR #11901:
URL: https://github.com/apache/lucene/pull/11901#discussion_r1044827355
##########
lucene/core/src/java/org/apache/lucene/document/LongRangeDocValuesField.java:
##########
@@ -86,4 +87,28 @@ private static void checkArgs(final long[] min, final long[] max) {
}
}
}
+
+ /**
+ * Sets the value of this field to a new value
+ *
+ * @param min the new min
+ * @param max the new max
+ */
+ public void setLongRangeValue(long min, long max) {
Review Comment:
Do we actually need these setters as part of this change? I have no problem adding them, but I wonder if we should do it as a separate change since it's kind of unrelated to your work here right?
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,210 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.ArrayUtil;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final Range[] ranges;
+ private final byte[][] encodedRanges;
+ private final int numEncodedValueBytes;
+ private final int dims;
+
+ /** Counts, initialized in by subclass. */
+ protected final int[] counts;
+
+ /** Our field name. */
+ protected final String field;
+
+ /** Total number of hits. */
+ protected int totCount;
+
+ private final ArrayUtil.ByteArrayComparator comparator;
+
+ /** Type of "range overlap" we want to count. */
+ RangeFieldQuery.QueryType queryType;
+
+ protected RangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ Query fastMatchQuery,
+ byte[][] encodedRanges,
+ Range... ranges)
+ throws IOException {
+ super(fastMatchQuery);
+ this.field = field;
+ this.ranges = ranges;
+ this.numEncodedValueBytes = ranges[0].getEncodedValueNumBytes();
+ this.dims = ranges[0].dims;
+ this.queryType = queryType;
+ this.comparator = ArrayUtil.getUnsignedComparator(this.numEncodedValueBytes);
+ this.encodedRanges = verifyEncodedRange(encodedRanges);
Review Comment:
Maybe just inline the logic of `verifyEncodedRange` directly in the ctor? Not sure it's worth breaking out. If you want to break it out though, let's make the method static and pass everything in. It's tricky that calling `verify...` requires `this.ranges` to have already been set.
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,210 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.ArrayUtil;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final Range[] ranges;
Review Comment:
So this suggestion kind of seems contradictory to my previous suggestion of not storing the string labels in a separate array. But, now that we're separately storing the encoded ranges, I wonder if we should bring back the string label array and _not_ store the `Range` array at all? So the sub-classes would pass two parallel arrays to the super ctor: 1) string labels, 2) encoded ranges. This is pretty nit-picky, but might be just a tad tighter?
##########
lucene/core/src/java/org/apache/lucene/document/RangeFieldQuery.java:
##########
@@ -251,7 +251,7 @@ Relation compare(
}
@Override
- boolean matches(
+ public boolean matches(
Review Comment:
We don't need this public vis anymore right?
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/LongRange.java:
##########
@@ -0,0 +1,124 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.util.Arrays;
+import java.util.Objects;
+
+/** Represents a long range for RangeOnRange faceting */
+public class LongRange extends Range {
+ /** Minimum (inclusive). */
+ public final long[] min;
+
+ /** Maximum (inclusive). */
+ public final long[] max;
+
+ /**
+ * Represents a single dimensional long range for RangeOnRange faceting
+ *
+ * @param label the name of the range
+ * @param minIn the minimum
+ * @param minInclusive if the minimum is inclusive
+ * @param maxIn the maximum
+ * @param maxInclusive if the maximum is inclusive
+ */
+ public LongRange(
+ String label, long minIn, boolean minInclusive, long maxIn, boolean maxInclusive) {
+ super(label, 1);
+
+ if (!minInclusive) {
Review Comment:
Should we unify the arg validation between the single-dim and multi-dim ctors? It feels like we should be able to just delegate to the multi-dim ctor here and have all validation be common (same for the double version).
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/LongRange.java:
##########
@@ -0,0 +1,124 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.util.Arrays;
+import java.util.Objects;
+
+/** Represents a long range for RangeOnRange faceting */
+public class LongRange extends Range {
+ /** Minimum (inclusive). */
+ public final long[] min;
+
+ /** Maximum (inclusive). */
+ public final long[] max;
+
+ /**
+ * Represents a single dimensional long range for RangeOnRange faceting
+ *
+ * @param label the name of the range
+ * @param minIn the minimum
+ * @param minInclusive if the minimum is inclusive
+ * @param maxIn the maximum
+ * @param maxInclusive if the maximum is inclusive
+ */
+ public LongRange(
+ String label, long minIn, boolean minInclusive, long maxIn, boolean maxInclusive) {
+ super(label, 1);
+
+ if (!minInclusive) {
+ if (minIn != Long.MAX_VALUE) {
+ minIn++;
+ } else {
+ failNoMatch();
+ }
+ }
+
+ if (!maxInclusive) {
+ if (maxIn != Long.MIN_VALUE) {
+ maxIn--;
+ } else {
+ failNoMatch();
+ }
+ }
+
+ if (minIn > maxIn) {
+ failNoMatch();
+ }
+
+ this.min = new long[] {minIn};
+ this.max = new long[] {maxIn};
+ }
+
+ /**
+ * Represents a multidimensional long range for RangeOnRange faceting
+ *
+ * @param label the name of the range
+ * @param min the minimum
+ * @param max the maximum
+ */
+ public LongRange(String label, long[] min, long[] max) {
Review Comment:
Should the javadoc mention that the ranges are all treated as "inclusive" in all dims?
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,210 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.ArrayUtil;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final Range[] ranges;
+ private final byte[][] encodedRanges;
+ private final int numEncodedValueBytes;
+ private final int dims;
+
+ /** Counts, initialized in by subclass. */
+ protected final int[] counts;
+
+ /** Our field name. */
+ protected final String field;
+
+ /** Total number of hits. */
+ protected int totCount;
+
+ private final ArrayUtil.ByteArrayComparator comparator;
+
+ /** Type of "range overlap" we want to count. */
+ RangeFieldQuery.QueryType queryType;
+
+ protected RangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ Query fastMatchQuery,
+ byte[][] encodedRanges,
+ Range... ranges)
+ throws IOException {
+ super(fastMatchQuery);
+ this.field = field;
+ this.ranges = ranges;
+ this.numEncodedValueBytes = ranges[0].getEncodedValueNumBytes();
Review Comment:
One more place where I wonder if we really need a callback? Could the sub-classes pass this information in the ctor and get rid of this method completely?
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mdmarshmallow commented on pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
mdmarshmallow commented on PR #11901:
URL: https://github.com/apache/lucene/pull/11901#issuecomment-1319432286
Thanks for looking at this @gsmiller, I know it's a large PR. I based a lot of this off of `LongRangeSlowRangeQuery`, which does handle multidimensional fields. However, if I understand it correctly, it does so in a way that is similar to `FacetSets` where it treats all the dimensions as defining a hyper rectangle. By this I mean that input ranges are only compared with the stored range of the same dimension. I figured we shouldn't do that for `RangeOnRangeFacetCounts` as `FacetSets` already does this. So I guess I wasn't super sure on how to handle it? Like if we allow for multidimensional ranges, then should we make sure that every input range matches every stored range regardless of dimension (take a cross product)? I can add this if you think it makes sense.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mdmarshmallow commented on a diff in pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
mdmarshmallow commented on code in PR #11901:
URL: https://github.com/apache/lucene/pull/11901#discussion_r1046211359
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/LongRangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.facet.rangeonrange;
+
+import static org.apache.lucene.document.LongRange.verifyAndEncode;
+
+import java.io.IOException;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.search.Query;
+
+/** Represents counts for long range on range faceting */
Review Comment:
Added some more descriptive java docs in the next revision, let me know if those are better!
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gsmiller commented on a diff in pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
gsmiller commented on code in PR #11901:
URL: https://github.com/apache/lucene/pull/11901#discussion_r1046413930
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/DoubleRangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.facet.rangeonrange;
+
+import static org.apache.lucene.document.DoubleRange.verifyAndEncode;
+
+import java.io.IOException;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.search.Query;
+
+/**
+ * Represents counts for double range on range faceting. To be more specific, this means that given
+ * a range (or list of ranges), this class will count all the documents in the index (or that match
+ * a fast match query) that contain ranges that "match" the provided ranges. These ranges are
+ * specified by the field parameter and expected to be of type {@link
+ * org.apache.lucene.document.DoubleRangeDocValuesField}. Matching is defined by the queryType
+ * param, you can see the type of matching supported by looking at {@link
+ * org.apache.lucene.document.RangeFieldQuery.QueryType}. In addition, this class supports
+ * multidimensional ranges. A multidimensional range will be counted as a match if every dimension
+ * matches the corresponding indexed range's dimension.
+ */
+public class DoubleRangeOnRangeFacetCounts extends RangeOnRangeFacetCounts {
+
+ /**
+ * Constructor without the fast match query, see other constructor description for more details.
+ */
+ public DoubleRangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ DoubleRange... ranges)
+ throws IOException {
+ super(
+ field,
+ hits,
+ queryType,
+ null,
+ Double.BYTES,
+ getEncodedRanges(ranges),
+ Range.getLabelsFromRanges(ranges));
+ }
+
+ /**
+ * Represents counts for long range on range faceting. See class javadoc for more details.
+ *
+ * @param field specifies a {@link org.apache.lucene.document.LongRangeDocValuesField} that will
Review Comment:
Should be DoubleRangeDocValuesField right?
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/DoubleRangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.facet.rangeonrange;
+
+import static org.apache.lucene.document.DoubleRange.verifyAndEncode;
+
+import java.io.IOException;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.search.Query;
+
+/**
+ * Represents counts for double range on range faceting. To be more specific, this means that given
+ * a range (or list of ranges), this class will count all the documents in the index (or that match
+ * a fast match query) that contain ranges that "match" the provided ranges. These ranges are
+ * specified by the field parameter and expected to be of type {@link
+ * org.apache.lucene.document.DoubleRangeDocValuesField}. Matching is defined by the queryType
+ * param, you can see the type of matching supported by looking at {@link
+ * org.apache.lucene.document.RangeFieldQuery.QueryType}. In addition, this class supports
+ * multidimensional ranges. A multidimensional range will be counted as a match if every dimension
+ * matches the corresponding indexed range's dimension.
+ */
+public class DoubleRangeOnRangeFacetCounts extends RangeOnRangeFacetCounts {
+
+ /**
+ * Constructor without the fast match query, see other constructor description for more details.
+ */
+ public DoubleRangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ DoubleRange... ranges)
+ throws IOException {
+ super(
+ field,
+ hits,
+ queryType,
+ null,
+ Double.BYTES,
+ getEncodedRanges(ranges),
+ Range.getLabelsFromRanges(ranges));
+ }
+
+ /**
+ * Represents counts for long range on range faceting. See class javadoc for more details.
Review Comment:
I think you mean to say "double" here instead of "long"
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gsmiller commented on a diff in pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
gsmiller commented on code in PR #11901:
URL: https://github.com/apache/lucene/pull/11901#discussion_r1044897335
##########
lucene/core/src/java/org/apache/lucene/document/RangeFieldQuery.java:
##########
@@ -251,7 +251,7 @@ Relation compare(
}
@Override
- boolean matches(
+ public boolean matches(
Review Comment:
Ah sorry, got them flipped.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gsmiller commented on a diff in pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
gsmiller commented on code in PR #11901:
URL: https://github.com/apache/lucene/pull/11901#discussion_r1044901068
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/Range.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.util.Arrays;
+
+/**
+ * Base class for a single labeled range.
+ *
+ * @lucene.experimental
+ */
+public abstract class Range {
+
+ /** Label that identifies this range. */
+ public final String label;
+
+ /** Number of dims per range */
+ public final int dims;
+
+ /** Sole constructor. */
+ protected Range(String label, int dims) {
+ if (label == null) {
+ throw new NullPointerException("label must not be null");
+ }
+ this.label = label;
+ this.dims = dims;
+ }
+
+ /** Invoke this for a useless range. */
+ protected void failNoMatch() {
+ throw new IllegalArgumentException("range \"" + label + "\" matches nothing");
+ }
+
+ /** Gets a corresponding array of labels given a range */
+ public static String[] getLabelsFromRanges(Range[] ranges) {
Review Comment:
Can this be pkg-private?
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mdmarshmallow commented on a diff in pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
mdmarshmallow commented on code in PR #11901:
URL: https://github.com/apache/lucene/pull/11901#discussion_r1046133836
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/LongRange.java:
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.util.Arrays;
+import java.util.Objects;
+
+/** Represents a long range for RangeOnRange faceting */
+public class LongRange extends Range {
+ /** Minimum (inclusive). */
+ public final long[] min;
+
+ /** Maximum (inclusive). */
+ public final long[] max;
+
+ /**
+ * Represents a single dimensional long range for RangeOnRange faceting
+ *
+ * @param label the name of the range
+ * @param minIn the minimum
+ * @param minInclusive if the minimum is inclusive
+ * @param maxIn the maximum
+ * @param maxInclusive if the maximum is inclusive
+ */
+ public LongRange(
+ String label, long minIn, boolean minInclusive, long maxIn, boolean maxInclusive) {
+ super(label, 1);
+
+ if (minInclusive == false) {
+ if (minIn != Long.MAX_VALUE) {
+ minIn++;
+ } else {
+ failNoMatch();
+ }
+ }
+
+ if (maxInclusive == false) {
+ if (maxIn != Long.MIN_VALUE) {
+ maxIn--;
+ } else {
+ failNoMatch();
+ }
+ }
+
+ if (minIn > maxIn) {
+ failNoMatch();
+ }
+
+ this.min = new long[] {minIn};
+ this.max = new long[] {maxIn};
+ }
+
+ /**
+ * Represents a multidimensional long range for RangeOnRange faceting
+ *
+ * @param label the name of the range
+ * @param min the minimum, inclusive
+ * @param max the maximum, inclusive
+ */
+ public LongRange(String label, long[] min, long[] max) {
+ super(label, min.length);
+ checkArgs(min, max);
+ this.min = min;
+ this.max = max;
+ }
+
+ @Override
+ public String toString() {
+ return "LongRange(label: "
+ + label
+ + ", min: "
+ + Arrays.toString(min)
+ + ", max: "
+ + Arrays.toString(max)
+ + ")";
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ LongRange longRange = (LongRange) o;
+ return Arrays.equals(min, longRange.min) && Arrays.equals(max, longRange.max);
Review Comment:
Ah yeah, thanks for catching that. Also added `dims` check as well to both.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mdmarshmallow commented on a diff in pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
mdmarshmallow commented on code in PR #11901:
URL: https://github.com/apache/lucene/pull/11901#discussion_r1046120980
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,209 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.ArrayUtil;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final byte[][] encodedRanges;
+ private final String[] labels;
+ private final int numEncodedValueBytes;
+ private final int dims;
+
+ /** Counts, initialized in by subclass. */
+ protected final int[] counts;
+
+ /** Our field name. */
+ protected final String field;
+
+ /** Total number of hits. */
+ protected int totCount;
+
+ private final ArrayUtil.ByteArrayComparator comparator;
+
+ /** Type of "range overlap" we want to count. */
+ RangeFieldQuery.QueryType queryType;
Review Comment:
No I don't think they do, changed in the next revision.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mdmarshmallow commented on a diff in pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
mdmarshmallow commented on code in PR #11901:
URL: https://github.com/apache/lucene/pull/11901#discussion_r1044918564
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/LongRange.java:
##########
@@ -0,0 +1,124 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.util.Arrays;
+import java.util.Objects;
+
+/** Represents a long range for RangeOnRange faceting */
+public class LongRange extends Range {
+ /** Minimum (inclusive). */
+ public final long[] min;
+
+ /** Maximum (inclusive). */
+ public final long[] max;
+
+ /**
+ * Represents a single dimensional long range for RangeOnRange faceting
+ *
+ * @param label the name of the range
+ * @param minIn the minimum
+ * @param minInclusive if the minimum is inclusive
+ * @param maxIn the maximum
+ * @param maxInclusive if the maximum is inclusive
+ */
+ public LongRange(
+ String label, long minIn, boolean minInclusive, long maxIn, boolean maxInclusive) {
+ super(label, 1);
+
+ if (!minInclusive) {
Review Comment:
Will change in next rev
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gsmiller commented on a diff in pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
gsmiller commented on code in PR #11901:
URL: https://github.com/apache/lucene/pull/11901#discussion_r1044821853
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,207 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final Range[] ranges;
+ private final String[] labels;
+ private final int numEncodedValueBytes;
+ private final int dims;
+ private final int numRanges;
+
+ /** Counts, initialized in by subclass. */
+ protected final int[] counts;
+
+ /** Our field name. */
+ protected final String field;
+
+ /** Total number of hits. */
+ protected int totCount;
+
+ /** Type of "range overlap" we want to count. */
+ RangeFieldQuery.QueryType queryType;
+
+ protected RangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ Query fastMatchQuery,
+ Range... ranges)
+ throws IOException {
+ super(fastMatchQuery);
+ this.field = field;
+ this.ranges = ranges;
+ this.numEncodedValueBytes = ranges[0].getEncodedValueBytes();
+ this.dims = ranges[0].dims;
+ this.labels = getLabels(ranges);
+ this.numRanges = ranges.length;
+ this.queryType = queryType;
+ counts = new int[numRanges];
+ count(field, hits.getMatchingDocs());
+ }
+
+ /** Counts from the provided field. */
+ protected void count(String field, List<FacetsCollector.MatchingDocs> matchingDocs)
+ throws IOException {
+
+ int missingCount = 0;
+
+ for (int i = 0; i < matchingDocs.size(); i++) {
+
+ FacetsCollector.MatchingDocs hits = matchingDocs.get(i);
+ BinaryRangeDocValues binaryRangeDocValues =
+ new BinaryRangeDocValues(
+ DocValues.getBinary(hits.context.reader(), field), dims, numEncodedValueBytes);
+
+ final DocIdSetIterator it = createIterator(hits);
+ if (it == null) {
+ continue;
+ }
+
+ totCount += hits.totalHits;
+ for (int doc = it.nextDoc(); doc != DocIdSetIterator.NO_MORE_DOCS; ) {
+ if (binaryRangeDocValues.advanceExact(doc)) {
+ boolean hasValidRange = false;
+ for (int range = 0; range < numRanges; range++) {
+ byte[] encodedRange = getEncodedRange(ranges[range]);
+ byte[] packedRange = binaryRangeDocValues.getPackedValue();
+ assert encodedRange.length == packedRange.length;
+ boolean matchesRange = true;
+ for (int dim = 0; dim < dims; dim++) {
+ if (queryType.matches(encodedRange, packedRange, dims, numEncodedValueBytes, dim)
+ == false) {
+ matchesRange = false;
+ break;
+ }
+ }
+ if (matchesRange) {
+ counts[range]++;
+ hasValidRange = true;
+ }
+ }
+ if (hasValidRange == false) {
+ missingCount++;
+ }
+ } else {
+ missingCount++;
+ }
+ doc = it.nextDoc();
+ }
+ }
+ totCount -= missingCount;
+ }
+
+ @Override
+ public FacetResult getAllChildren(String dim, String... path) throws IOException {
Review Comment:
Sounds right. Thanks!
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gsmiller commented on a diff in pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
gsmiller commented on code in PR #11901:
URL: https://github.com/apache/lucene/pull/11901#discussion_r1044821610
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/LongRangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * 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.facet.rangeonrange;
+
+import static org.apache.lucene.document.LongRange.verifyAndEncode;
+
+import java.io.IOException;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.search.Query;
+
+/** Represents counts for long range on range faceting */
+public class LongRangeOnRangeFacetCounts extends RangeOnRangeFacetCounts {
+
+ /**
+ * Represents counts for long range on range faceting (inclusive)
+ *
+ * @param field document's field
+ * @param hits hits we want the counts of
+ * @param queryType type of intersection we want to count
+ * @param ranges ranges we want the counts of
+ * @throws IOException low level exception
+ */
+ public LongRangeOnRangeFacetCounts(
+ String field, FacetsCollector hits, RangeFieldQuery.QueryType queryType, LongRange... ranges)
+ throws IOException {
+ super(field, hits, queryType, null, ranges);
+ }
+
+ /**
+ * Represents counts for long range on range faceting
+ *
+ * @param field document's field
+ * @param hits hits we want the counts of
+ * @param queryType type of intersection we want to count
+ * @param fastMatchQuery query to quickly discard hits
+ * @param ranges ranges we want the counts of
+ * @throws IOException low level exception
+ */
+ public LongRangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ Query fastMatchQuery,
+ LongRange... ranges)
+ throws IOException {
+ super(field, hits, queryType, fastMatchQuery, ranges);
+ }
+
+ @Override
+ public byte[] getEncodedRange(Range range) {
Review Comment:
Cool. I mean, I don't feel all that strongly about having the subclasses do the encoding and pass along to `super` vs. having a callback exposed and having the super class encode during initialization, but it seemed just a little simpler this way?
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gsmiller commented on a diff in pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
gsmiller commented on code in PR #11901:
URL: https://github.com/apache/lucene/pull/11901#discussion_r1044900429
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/LongRange.java:
##########
@@ -0,0 +1,124 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.util.Arrays;
+import java.util.Objects;
+
+/** Represents a long range for RangeOnRange faceting */
+public class LongRange extends Range {
+ /** Minimum (inclusive). */
+ public final long[] min;
+
+ /** Maximum (inclusive). */
+ public final long[] max;
+
+ /**
+ * Represents a single dimensional long range for RangeOnRange faceting
+ *
+ * @param label the name of the range
+ * @param minIn the minimum
+ * @param minInclusive if the minimum is inclusive
+ * @param maxIn the maximum
+ * @param maxInclusive if the maximum is inclusive
+ */
+ public LongRange(
+ String label, long minIn, boolean minInclusive, long maxIn, boolean maxInclusive) {
+ super(label, 1);
+
+ if (!minInclusive) {
Review Comment:
Hmm, yeah that's fair. OK, I think what you've got is fine then. I did just notice you're using `!` though instead of `== false` in those ctors. Mind changing that?
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mdmarshmallow commented on a diff in pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
mdmarshmallow commented on code in PR #11901:
URL: https://github.com/apache/lucene/pull/11901#discussion_r1044867617
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,210 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.ArrayUtil;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final Range[] ranges;
+ private final byte[][] encodedRanges;
+ private final int numEncodedValueBytes;
+ private final int dims;
+
+ /** Counts, initialized in by subclass. */
+ protected final int[] counts;
+
+ /** Our field name. */
+ protected final String field;
+
+ /** Total number of hits. */
+ protected int totCount;
+
+ private final ArrayUtil.ByteArrayComparator comparator;
+
+ /** Type of "range overlap" we want to count. */
+ RangeFieldQuery.QueryType queryType;
+
+ protected RangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ Query fastMatchQuery,
+ byte[][] encodedRanges,
+ Range... ranges)
+ throws IOException {
+ super(fastMatchQuery);
+ this.field = field;
+ this.ranges = ranges;
+ this.numEncodedValueBytes = ranges[0].getEncodedValueNumBytes();
+ this.dims = ranges[0].dims;
+ this.queryType = queryType;
+ this.comparator = ArrayUtil.getUnsignedComparator(this.numEncodedValueBytes);
+ this.encodedRanges = verifyEncodedRange(encodedRanges);
Review Comment:
Made the verification inline in the next revision
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/LongRange.java:
##########
@@ -0,0 +1,124 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.util.Arrays;
+import java.util.Objects;
+
+/** Represents a long range for RangeOnRange faceting */
+public class LongRange extends Range {
+ /** Minimum (inclusive). */
+ public final long[] min;
+
+ /** Maximum (inclusive). */
+ public final long[] max;
+
+ /**
+ * Represents a single dimensional long range for RangeOnRange faceting
+ *
+ * @param label the name of the range
+ * @param minIn the minimum
+ * @param minInclusive if the minimum is inclusive
+ * @param maxIn the maximum
+ * @param maxInclusive if the maximum is inclusive
+ */
+ public LongRange(
+ String label, long minIn, boolean minInclusive, long maxIn, boolean maxInclusive) {
+ super(label, 1);
+
+ if (!minInclusive) {
Review Comment:
This ctor allows for the `minInclusive` and `maxInclusive` params, which means there is some additional validation there, which is why I just did separate validation entirely. The only part I could remove is this:
```
if (minIn > maxIn) {
failNoMatch();
}
```
Maybe we could allow these parameters in the other ctor or remove those params from this ctor to fully unify the validation?
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/LongRange.java:
##########
@@ -0,0 +1,124 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.util.Arrays;
+import java.util.Objects;
+
+/** Represents a long range for RangeOnRange faceting */
+public class LongRange extends Range {
+ /** Minimum (inclusive). */
+ public final long[] min;
+
+ /** Maximum (inclusive). */
+ public final long[] max;
+
+ /**
+ * Represents a single dimensional long range for RangeOnRange faceting
+ *
+ * @param label the name of the range
+ * @param minIn the minimum
+ * @param minInclusive if the minimum is inclusive
+ * @param maxIn the maximum
+ * @param maxInclusive if the maximum is inclusive
+ */
+ public LongRange(
+ String label, long minIn, boolean minInclusive, long maxIn, boolean maxInclusive) {
+ super(label, 1);
+
+ if (!minInclusive) {
+ if (minIn != Long.MAX_VALUE) {
+ minIn++;
+ } else {
+ failNoMatch();
+ }
+ }
+
+ if (!maxInclusive) {
+ if (maxIn != Long.MIN_VALUE) {
+ maxIn--;
+ } else {
+ failNoMatch();
+ }
+ }
+
+ if (minIn > maxIn) {
+ failNoMatch();
+ }
+
+ this.min = new long[] {minIn};
+ this.max = new long[] {maxIn};
+ }
+
+ /**
+ * Represents a multidimensional long range for RangeOnRange faceting
+ *
+ * @param label the name of the range
+ * @param min the minimum
+ * @param max the maximum
+ */
+ public LongRange(String label, long[] min, long[] max) {
Review Comment:
Ah yeah, thanks for catching! Added to the javadoc
##########
lucene/core/src/java/org/apache/lucene/document/LongRangeDocValuesField.java:
##########
@@ -86,4 +87,28 @@ private static void checkArgs(final long[] min, final long[] max) {
}
}
}
+
+ /**
+ * Sets the value of this field to a new value
+ *
+ * @param min the new min
+ * @param max the new max
+ */
+ public void setLongRangeValue(long min, long max) {
Review Comment:
They are actually used in the tests for this change. I followed the same format as `RangeFaceting` when creating the tests and it also used this setter pattern.
##########
lucene/core/src/java/org/apache/lucene/document/RangeFieldQuery.java:
##########
@@ -251,7 +251,7 @@ Relation compare(
}
@Override
- boolean matches(
+ public boolean matches(
Review Comment:
I think this one has to be since it's the non dim-specific version, but the other one (line 213) doesn't, thanks for catching that!
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,210 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.ArrayUtil;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final Range[] ranges;
Review Comment:
Yeah I think that's fine, especially if we get rid of the `getEncodedValueNumBytes` callback as you suggested and pass in the value directly, then there will be even less of a reason to pass in the ranges. Will change in the next revision.
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,210 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.ArrayUtil;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final Range[] ranges;
+ private final byte[][] encodedRanges;
+ private final int numEncodedValueBytes;
+ private final int dims;
+
+ /** Counts, initialized in by subclass. */
+ protected final int[] counts;
+
+ /** Our field name. */
+ protected final String field;
+
+ /** Total number of hits. */
+ protected int totCount;
+
+ private final ArrayUtil.ByteArrayComparator comparator;
+
+ /** Type of "range overlap" we want to count. */
+ RangeFieldQuery.QueryType queryType;
+
+ protected RangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ Query fastMatchQuery,
+ byte[][] encodedRanges,
+ Range... ranges)
+ throws IOException {
+ super(fastMatchQuery);
+ this.field = field;
+ this.ranges = ranges;
+ this.numEncodedValueBytes = ranges[0].getEncodedValueNumBytes();
Review Comment:
Removed this method in next revision
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gsmiller merged pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
gsmiller merged PR #11901:
URL: https://github.com/apache/lucene/pull/11901
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gsmiller commented on pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
gsmiller commented on PR #11901:
URL: https://github.com/apache/lucene/pull/11901#issuecomment-1320139204
@mdmarshmallow it's similar to FacetSets but a bit different since FacetSets work over stored points, while this would work over stored ranges. I think it would make sense to eventually support multiple dimensions here, since the data (`LongRangeDocValueFields`) allow for it. I'm thinking about a user who is storing multi-dim ranges and creating queries that filter over those same ranges, but has no way to facet over them.
As for how to handle faceting of multi-dim ranges, I think the logic would be the same as the "slow" query you reference, and would depend on the `RangeFieldQuery#QueryType` specified.
But... as for concrete next steps, I see the value in simplifying the problem to a single dimension to get started. That likely covers most use-cases anyway. But I'd also like to create a path towards supporting multi-dim without having to create a completely new API. Could we re-use `LongRangeDocValueFields` for the indexing piece of this, and create a faceting implementation that functions over that data, but only make it work for single dimensions to start? Maybe the faceting API you create is simplified to the single-dimension case and it validates that the underlying field being faceted is single-dim? That way, in the future, if we add multi-dim support, users can keep using the same field type for their documents but the API gets generalized?
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gsmiller commented on pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
gsmiller commented on PR #11901:
URL: https://github.com/apache/lucene/pull/11901#issuecomment-1322333279
> Ah yeah good point on the FacetSets... so I actually already use LongRangeDocValueFields here: public class LongRangeDocValuesFacetField extends LongRangeDocValuesField. The difference is that the LongRangeDocValuesFacetField constructor enforces a single dimension. I'd imagine if we want to extend it, it shouldn't be that hard to just add another constructor here.
Right. What I'm questioning here is the need for `LongRangeDocValuesFacetField`. If the only value it adds is a ctor that "speaks" in a single-dimension, I wonder if we should keep it or just ask our users to use `LongRangeDocValuesField` directly. If we think users would benefit from a simpler, single-dim API, let's consider adding that directly to `LongRangeDocValuesField`.
> Right now what happens is that the query can have multiple ranges, which all need to "match" the stored range, which is pretty much the "cross product" idea I talked about earlier which we won't be doing.
I don't think I'm following. Are you referring to the current "slow query" implementation? It doesn't support multiple ranges right? It supports multiple dimensions? And yes- each dimension needs to satisfy the requirements specified by the provided `QueryType`. I imaging faceting doing the same thing, but separately considering each range for facet counting. Maybe we're confusing the concept of "multiple dimensions" with "multiple ranges" in our discussion here?
> or just go ahead and do the multidimensional implementation
Yeah, I think that's fine. I don't think it will be significantly more complex to implement multi-dim support. I think the only complexity comes in thinking about the problem and conveying something usable to users (who, I suspect, will be looking for a single-dim implementation most of the time).
> Edit: now that I think about it a bit more, I'm not actually doing a cross product, I think I was getting the concept of multidimensional ranges for multiple ranges mixed up in my mind. I'll need to look more into the API to see how we can make it extensible in case we want to make this multi-dimensional in the future.
Ah ok. Maybe disregard my above comment about mixing up "multiple ranges" and "multiple dimensions." Sounds like maybe that's whats going on. Let me know if I can help disambiguate. Thanks!
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mdmarshmallow commented on pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
mdmarshmallow commented on PR #11901:
URL: https://github.com/apache/lucene/pull/11901#issuecomment-1338159980
Hey @gsmiller, sorry for the delay but finally got around to your feedback. I decided to just add multidimensional support since it was fairly straightforward (if not a bit tedious to test lol). I kept the single dim API the same (there's some syntax sugar) and made the multidimensional API similar to `LongSlowRangeQuery`. Thanks for all the feedback!
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mdmarshmallow commented on a diff in pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
mdmarshmallow commented on code in PR #11901:
URL: https://github.com/apache/lucene/pull/11901#discussion_r1044690705
##########
lucene/core/src/java/org/apache/lucene/document/LongRangeDocValuesFacetField.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * 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 org.apache.lucene.util.BytesRef;
+
+/**
+ * A subclass of LongRangeDocValuesField that only allows single dimension range representation.
+ * Meant to be used with RangeOnRange faceting
+ */
+public class LongRangeDocValuesFacetField extends LongRangeDocValuesField {
Review Comment:
Sounds good, I will just modify `LongRangeDocValuesField` and `DoubleRangeDocValuesField`.
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/LongRangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * 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.facet.rangeonrange;
+
+import static org.apache.lucene.document.LongRange.verifyAndEncode;
+
+import java.io.IOException;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.search.Query;
+
+/** Represents counts for long range on range faceting */
+public class LongRangeOnRangeFacetCounts extends RangeOnRangeFacetCounts {
+
+ /**
+ * Represents counts for long range on range faceting (inclusive)
+ *
+ * @param field document's field
+ * @param hits hits we want the counts of
+ * @param queryType type of intersection we want to count
+ * @param ranges ranges we want the counts of
+ * @throws IOException low level exception
+ */
+ public LongRangeOnRangeFacetCounts(
+ String field, FacetsCollector hits, RangeFieldQuery.QueryType queryType, LongRange... ranges)
+ throws IOException {
+ super(field, hits, queryType, null, ranges);
+ }
+
+ /**
+ * Represents counts for long range on range faceting
+ *
+ * @param field document's field
+ * @param hits hits we want the counts of
+ * @param queryType type of intersection we want to count
+ * @param fastMatchQuery query to quickly discard hits
+ * @param ranges ranges we want the counts of
+ * @throws IOException low level exception
+ */
+ public LongRangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ Query fastMatchQuery,
+ LongRange... ranges)
+ throws IOException {
+ super(field, hits, queryType, fastMatchQuery, ranges);
+ }
+
+ @Override
+ public byte[] getEncodedRange(Range range) {
Review Comment:
Hmm, yeah I could do that, I think the only concern is that I didn't want to pass in `ranges` twice, but I guess there's no real reason to avoiding that. I changed it.
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,207 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final Range[] ranges;
+ private final String[] labels;
Review Comment:
Yeah we don't need this array. Removed in next revision. I'll also remove some other "useless" params.
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,207 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final Range[] ranges;
+ private final String[] labels;
+ private final int numEncodedValueBytes;
+ private final int dims;
+ private final int numRanges;
+
+ /** Counts, initialized in by subclass. */
+ protected final int[] counts;
+
+ /** Our field name. */
+ protected final String field;
+
+ /** Total number of hits. */
+ protected int totCount;
+
+ /** Type of "range overlap" we want to count. */
+ RangeFieldQuery.QueryType queryType;
+
+ protected RangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ Query fastMatchQuery,
+ Range... ranges)
+ throws IOException {
+ super(fastMatchQuery);
+ this.field = field;
+ this.ranges = ranges;
+ this.numEncodedValueBytes = ranges[0].getEncodedValueBytes();
+ this.dims = ranges[0].dims;
+ this.labels = getLabels(ranges);
+ this.numRanges = ranges.length;
+ this.queryType = queryType;
+ counts = new int[numRanges];
+ count(field, hits.getMatchingDocs());
+ }
+
+ /** Counts from the provided field. */
+ protected void count(String field, List<FacetsCollector.MatchingDocs> matchingDocs)
+ throws IOException {
+
+ int missingCount = 0;
+
+ for (int i = 0; i < matchingDocs.size(); i++) {
+
+ FacetsCollector.MatchingDocs hits = matchingDocs.get(i);
+ BinaryRangeDocValues binaryRangeDocValues =
+ new BinaryRangeDocValues(
+ DocValues.getBinary(hits.context.reader(), field), dims, numEncodedValueBytes);
+
+ final DocIdSetIterator it = createIterator(hits);
+ if (it == null) {
+ continue;
+ }
+
+ totCount += hits.totalHits;
+ for (int doc = it.nextDoc(); doc != DocIdSetIterator.NO_MORE_DOCS; ) {
+ if (binaryRangeDocValues.advanceExact(doc)) {
+ boolean hasValidRange = false;
+ for (int range = 0; range < numRanges; range++) {
+ byte[] encodedRange = getEncodedRange(ranges[range]);
+ byte[] packedRange = binaryRangeDocValues.getPackedValue();
+ assert encodedRange.length == packedRange.length;
+ boolean matchesRange = true;
+ for (int dim = 0; dim < dims; dim++) {
+ if (queryType.matches(encodedRange, packedRange, dims, numEncodedValueBytes, dim)
+ == false) {
+ matchesRange = false;
+ break;
+ }
+ }
+ if (matchesRange) {
+ counts[range]++;
+ hasValidRange = true;
+ }
+ }
+ if (hasValidRange == false) {
+ missingCount++;
+ }
+ } else {
+ missingCount++;
+ }
+ doc = it.nextDoc();
+ }
+ }
+ totCount -= missingCount;
+ }
+
+ @Override
+ public FacetResult getAllChildren(String dim, String... path) throws IOException {
Review Comment:
I just used the same java doc that `RangeFacetCounts` uses for this method. Hopefully that works.
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,207 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final Range[] ranges;
+ private final String[] labels;
+ private final int numEncodedValueBytes;
+ private final int dims;
+ private final int numRanges;
+
+ /** Counts, initialized in by subclass. */
+ protected final int[] counts;
+
+ /** Our field name. */
+ protected final String field;
+
+ /** Total number of hits. */
+ protected int totCount;
+
+ /** Type of "range overlap" we want to count. */
+ RangeFieldQuery.QueryType queryType;
+
+ protected RangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ Query fastMatchQuery,
+ Range... ranges)
+ throws IOException {
+ super(fastMatchQuery);
+ this.field = field;
+ this.ranges = ranges;
+ this.numEncodedValueBytes = ranges[0].getEncodedValueBytes();
+ this.dims = ranges[0].dims;
+ this.labels = getLabels(ranges);
+ this.numRanges = ranges.length;
+ this.queryType = queryType;
+ counts = new int[numRanges];
+ count(field, hits.getMatchingDocs());
+ }
+
+ /** Counts from the provided field. */
+ protected void count(String field, List<FacetsCollector.MatchingDocs> matchingDocs)
Review Comment:
Added in next revision.
##########
lucene/core/src/java/org/apache/lucene/document/LongRange.java:
##########
@@ -61,6 +61,17 @@ public LongRange(String name, final long[] min, final long[] max) {
setRangeValues(min, max);
}
+ /**
+ * Create a new LongRange type with one dimension
+ *
+ * @param name field name. must not be null.
+ * @param min min value
+ * @param max max value
+ */
+ public LongRange(String name, final long min, final long max) {
Review Comment:
Makes sense, I'll remove these then
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,207 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final Range[] ranges;
+ private final String[] labels;
+ private final int numEncodedValueBytes;
+ private final int dims;
+ private final int numRanges;
+
+ /** Counts, initialized in by subclass. */
+ protected final int[] counts;
+
+ /** Our field name. */
+ protected final String field;
+
+ /** Total number of hits. */
+ protected int totCount;
+
+ /** Type of "range overlap" we want to count. */
+ RangeFieldQuery.QueryType queryType;
+
+ protected RangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ Query fastMatchQuery,
+ Range... ranges)
+ throws IOException {
+ super(fastMatchQuery);
+ this.field = field;
+ this.ranges = ranges;
+ this.numEncodedValueBytes = ranges[0].getEncodedValueBytes();
+ this.dims = ranges[0].dims;
+ this.labels = getLabels(ranges);
+ this.numRanges = ranges.length;
+ this.queryType = queryType;
+ counts = new int[numRanges];
+ count(field, hits.getMatchingDocs());
+ }
+
+ /** Counts from the provided field. */
+ protected void count(String field, List<FacetsCollector.MatchingDocs> matchingDocs)
+ throws IOException {
+
+ int missingCount = 0;
+
+ for (int i = 0; i < matchingDocs.size(); i++) {
+
+ FacetsCollector.MatchingDocs hits = matchingDocs.get(i);
+ BinaryRangeDocValues binaryRangeDocValues =
+ new BinaryRangeDocValues(
+ DocValues.getBinary(hits.context.reader(), field), dims, numEncodedValueBytes);
+
+ final DocIdSetIterator it = createIterator(hits);
+ if (it == null) {
+ continue;
+ }
+
+ totCount += hits.totalHits;
+ for (int doc = it.nextDoc(); doc != DocIdSetIterator.NO_MORE_DOCS; ) {
+ if (binaryRangeDocValues.advanceExact(doc)) {
+ boolean hasValidRange = false;
+ for (int range = 0; range < numRanges; range++) {
+ byte[] encodedRange = getEncodedRange(ranges[range]);
+ byte[] packedRange = binaryRangeDocValues.getPackedValue();
+ assert encodedRange.length == packedRange.length;
+ boolean matchesRange = true;
+ for (int dim = 0; dim < dims; dim++) {
+ if (queryType.matches(encodedRange, packedRange, dims, numEncodedValueBytes, dim)
Review Comment:
Ah yeah, forgot about that one, thanks for catching that! Changed in the next revision.
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,207 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final Range[] ranges;
+ private final String[] labels;
+ private final int numEncodedValueBytes;
+ private final int dims;
+ private final int numRanges;
+
+ /** Counts, initialized in by subclass. */
+ protected final int[] counts;
+
+ /** Our field name. */
+ protected final String field;
+
+ /** Total number of hits. */
+ protected int totCount;
+
+ /** Type of "range overlap" we want to count. */
+ RangeFieldQuery.QueryType queryType;
+
+ protected RangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ Query fastMatchQuery,
+ Range... ranges)
+ throws IOException {
+ super(fastMatchQuery);
+ this.field = field;
+ this.ranges = ranges;
+ this.numEncodedValueBytes = ranges[0].getEncodedValueBytes();
+ this.dims = ranges[0].dims;
+ this.labels = getLabels(ranges);
+ this.numRanges = ranges.length;
+ this.queryType = queryType;
+ counts = new int[numRanges];
+ count(field, hits.getMatchingDocs());
+ }
+
+ /** Counts from the provided field. */
+ protected void count(String field, List<FacetsCollector.MatchingDocs> matchingDocs)
+ throws IOException {
+
+ int missingCount = 0;
+
+ for (int i = 0; i < matchingDocs.size(); i++) {
+
+ FacetsCollector.MatchingDocs hits = matchingDocs.get(i);
+ BinaryRangeDocValues binaryRangeDocValues =
+ new BinaryRangeDocValues(
+ DocValues.getBinary(hits.context.reader(), field), dims, numEncodedValueBytes);
+
+ final DocIdSetIterator it = createIterator(hits);
+ if (it == null) {
+ continue;
+ }
+
+ totCount += hits.totalHits;
+ for (int doc = it.nextDoc(); doc != DocIdSetIterator.NO_MORE_DOCS; ) {
+ if (binaryRangeDocValues.advanceExact(doc)) {
+ boolean hasValidRange = false;
+ for (int range = 0; range < numRanges; range++) {
+ byte[] encodedRange = getEncodedRange(ranges[range]);
Review Comment:
Addressed this in your other comment.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gsmiller commented on a diff in pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
Posted by GitBox <gi...@apache.org>.
gsmiller commented on code in PR #11901:
URL: https://github.com/apache/lucene/pull/11901#discussion_r1045109686
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,209 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.ArrayUtil;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final byte[][] encodedRanges;
+ private final String[] labels;
+ private final int numEncodedValueBytes;
+ private final int dims;
+
+ /** Counts, initialized in by subclass. */
+ protected final int[] counts;
+
+ /** Our field name. */
+ protected final String field;
+
+ /** Total number of hits. */
+ protected int totCount;
+
+ private final ArrayUtil.ByteArrayComparator comparator;
+
+ /** Type of "range overlap" we want to count. */
+ RangeFieldQuery.QueryType queryType;
+
+ protected RangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ Query fastMatchQuery,
+ int numEncodedValueBytes,
+ byte[][] encodedRanges,
+ String[] labels)
+ throws IOException {
+ super(fastMatchQuery);
+
+ assert encodedRanges.length == labels.length;
+ assert encodedRanges[0].length % (2 * numEncodedValueBytes) == 0;
+
+ this.encodedRanges = encodedRanges;
+ this.field = field;
+ this.labels = labels;
+ this.numEncodedValueBytes = numEncodedValueBytes;
+ this.dims = encodedRanges[0].length / (2 * this.numEncodedValueBytes);
+ this.queryType = queryType;
+ this.comparator = ArrayUtil.getUnsignedComparator(this.numEncodedValueBytes);
+ this.counts = new int[encodedRanges.length];
+ count(field, hits.getMatchingDocs());
+ }
+
+ /** Counts from the provided field. */
+ protected void count(String field, List<FacetsCollector.MatchingDocs> matchingDocs)
+ throws IOException {
+ // TODO: We currently just exhaustively check the ranges in each document with every range in
+ // the ranges array.
+ // We might be able to do something more efficient here by grouping the ranges array into a
+ // space partitioning
+ // data structure of some sort.
+
+ int missingCount = 0;
+
+ for (FacetsCollector.MatchingDocs hits : matchingDocs) {
+
+ BinaryRangeDocValues binaryRangeDocValues =
+ new BinaryRangeDocValues(
+ DocValues.getBinary(hits.context.reader(), field), dims, numEncodedValueBytes);
+
+ final DocIdSetIterator it = createIterator(hits);
Review Comment:
Forgot to mention earlier, but I love that you've added support for fast-match queries. And that it was so straight-forward to do so after that functionality was factored out into its own class. Nice!
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/LongRangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.facet.rangeonrange;
+
+import static org.apache.lucene.document.LongRange.verifyAndEncode;
+
+import java.io.IOException;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.search.Query;
+
+/** Represents counts for long range on range faceting */
Review Comment:
Could we expand the javadoc a little here to help users understand this faceting implementation and the ways they can use it? For example, it's probably worth mentioning the type of field it expects, the single- vs. multi-dim cases and that it's meant to pair with the range-on-range queries? (Same feedback for the double case). Maybe have a look at the ctor javadoc and such as well, and think about what we should convey to users that are looking at this javadoc to figure out how to use it.
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,209 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.ArrayUtil;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final byte[][] encodedRanges;
+ private final String[] labels;
+ private final int numEncodedValueBytes;
+ private final int dims;
+
+ /** Counts, initialized in by subclass. */
+ protected final int[] counts;
+
+ /** Our field name. */
+ protected final String field;
+
+ /** Total number of hits. */
+ protected int totCount;
+
+ private final ArrayUtil.ByteArrayComparator comparator;
+
+ /** Type of "range overlap" we want to count. */
+ RangeFieldQuery.QueryType queryType;
Review Comment:
Do these `protected` and pkg-private fields need this visibility? Can we make them `private` instead? It doesn't look like sub-classes need direct access to any of them. We can always increase visibility later if a sub-class needs it.
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/RangeOnRangeFacetCounts.java:
##########
@@ -0,0 +1,209 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryRangeDocValues;
+import org.apache.lucene.document.RangeFieldQuery;
+import org.apache.lucene.facet.FacetCountsWithFilterQuery;
+import org.apache.lucene.facet.FacetResult;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.LabelAndValue;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.util.ArrayUtil;
+import org.apache.lucene.util.PriorityQueue;
+
+abstract class RangeOnRangeFacetCounts extends FacetCountsWithFilterQuery {
+
+ private final byte[][] encodedRanges;
+ private final String[] labels;
+ private final int numEncodedValueBytes;
+ private final int dims;
+
+ /** Counts, initialized in by subclass. */
+ protected final int[] counts;
+
+ /** Our field name. */
+ protected final String field;
+
+ /** Total number of hits. */
+ protected int totCount;
+
+ private final ArrayUtil.ByteArrayComparator comparator;
+
+ /** Type of "range overlap" we want to count. */
+ RangeFieldQuery.QueryType queryType;
+
+ protected RangeOnRangeFacetCounts(
+ String field,
+ FacetsCollector hits,
+ RangeFieldQuery.QueryType queryType,
+ Query fastMatchQuery,
+ int numEncodedValueBytes,
+ byte[][] encodedRanges,
+ String[] labels)
+ throws IOException {
+ super(fastMatchQuery);
+
+ assert encodedRanges.length == labels.length;
+ assert encodedRanges[0].length % (2 * numEncodedValueBytes) == 0;
+
+ this.encodedRanges = encodedRanges;
+ this.field = field;
+ this.labels = labels;
+ this.numEncodedValueBytes = numEncodedValueBytes;
+ this.dims = encodedRanges[0].length / (2 * this.numEncodedValueBytes);
+ this.queryType = queryType;
+ this.comparator = ArrayUtil.getUnsignedComparator(this.numEncodedValueBytes);
Review Comment:
minor: It looks like we only need `dims`, `numEncodedValueBytes`, `encodedRanges`, `comparator` and `queryType` in the `count()` method. Could we just pass them in and not need to maintain a field reference to them in the class instance? Always nice to keep the fields tight if possible. Easier to read and dereferences them earlier for GC.
Edit: For `comparator`, you could also instantiate it in the `count` method itself.
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/LongRange.java:
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.util.Arrays;
+import java.util.Objects;
+
+/** Represents a long range for RangeOnRange faceting */
+public class LongRange extends Range {
+ /** Minimum (inclusive). */
+ public final long[] min;
+
+ /** Maximum (inclusive). */
+ public final long[] max;
+
+ /**
+ * Represents a single dimensional long range for RangeOnRange faceting
+ *
+ * @param label the name of the range
+ * @param minIn the minimum
+ * @param minInclusive if the minimum is inclusive
+ * @param maxIn the maximum
+ * @param maxInclusive if the maximum is inclusive
+ */
+ public LongRange(
+ String label, long minIn, boolean minInclusive, long maxIn, boolean maxInclusive) {
+ super(label, 1);
+
+ if (minInclusive == false) {
+ if (minIn != Long.MAX_VALUE) {
+ minIn++;
+ } else {
+ failNoMatch();
+ }
+ }
+
+ if (maxInclusive == false) {
+ if (maxIn != Long.MIN_VALUE) {
+ maxIn--;
+ } else {
+ failNoMatch();
+ }
+ }
+
+ if (minIn > maxIn) {
+ failNoMatch();
+ }
+
+ this.min = new long[] {minIn};
+ this.max = new long[] {maxIn};
+ }
+
+ /**
+ * Represents a multidimensional long range for RangeOnRange faceting
+ *
+ * @param label the name of the range
+ * @param min the minimum, inclusive
+ * @param max the maximum, inclusive
+ */
+ public LongRange(String label, long[] min, long[] max) {
+ super(label, min.length);
+ checkArgs(min, max);
+ this.min = min;
+ this.max = max;
+ }
+
+ @Override
+ public String toString() {
+ return "LongRange(label: "
+ + label
+ + ", min: "
+ + Arrays.toString(min)
+ + ", max: "
+ + Arrays.toString(max)
+ + ")";
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ LongRange longRange = (LongRange) o;
+ return Arrays.equals(min, longRange.min) && Arrays.equals(max, longRange.max);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(label, Arrays.hashCode(min), Arrays.hashCode(max));
+ }
+
+ private static void checkArgs(final long[] min, final long[] max) {
+ if (min == null || max == null || min.length == 0 || max.length == 0) {
+ throw new IllegalArgumentException("min/max range values cannot be null or empty");
+ }
+ if (min.length != max.length) {
+ throw new IllegalArgumentException("min/max ranges must agree");
+ }
+
+ for (int i = 0; i < min.length; i++) {
+ if (min[i] > max[i]) {
+ throw new IllegalArgumentException("min should be less than max");
Review Comment:
Should we delegate to `failNoMatch()` here for consistency? (Same for the double case).
##########
lucene/facet/src/java/org/apache/lucene/facet/rangeonrange/LongRange.java:
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.facet.rangeonrange;
+
+import java.util.Arrays;
+import java.util.Objects;
+
+/** Represents a long range for RangeOnRange faceting */
+public class LongRange extends Range {
+ /** Minimum (inclusive). */
+ public final long[] min;
+
+ /** Maximum (inclusive). */
+ public final long[] max;
+
+ /**
+ * Represents a single dimensional long range for RangeOnRange faceting
+ *
+ * @param label the name of the range
+ * @param minIn the minimum
+ * @param minInclusive if the minimum is inclusive
+ * @param maxIn the maximum
+ * @param maxInclusive if the maximum is inclusive
+ */
+ public LongRange(
+ String label, long minIn, boolean minInclusive, long maxIn, boolean maxInclusive) {
+ super(label, 1);
+
+ if (minInclusive == false) {
+ if (minIn != Long.MAX_VALUE) {
+ minIn++;
+ } else {
+ failNoMatch();
+ }
+ }
+
+ if (maxInclusive == false) {
+ if (maxIn != Long.MIN_VALUE) {
+ maxIn--;
+ } else {
+ failNoMatch();
+ }
+ }
+
+ if (minIn > maxIn) {
+ failNoMatch();
+ }
+
+ this.min = new long[] {minIn};
+ this.max = new long[] {maxIn};
+ }
+
+ /**
+ * Represents a multidimensional long range for RangeOnRange faceting
+ *
+ * @param label the name of the range
+ * @param min the minimum, inclusive
+ * @param max the maximum, inclusive
+ */
+ public LongRange(String label, long[] min, long[] max) {
+ super(label, min.length);
+ checkArgs(min, max);
+ this.min = min;
+ this.max = max;
+ }
+
+ @Override
+ public String toString() {
+ return "LongRange(label: "
+ + label
+ + ", min: "
+ + Arrays.toString(min)
+ + ", max: "
+ + Arrays.toString(max)
+ + ")";
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ LongRange longRange = (LongRange) o;
+ return Arrays.equals(min, longRange.min) && Arrays.equals(max, longRange.max);
Review Comment:
Should `equals` also check `label` to be consistent with `hashCode()`? (Same with the double case).
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org