You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ho...@apache.org on 2021/01/06 17:07:47 UTC
[lucene-solr] branch master updated: SOLR-15047: Fix collapse
parser behavior when collapsing on numeric fields to differentiate '0'
group from null group
This is an automated email from the ASF dual-hosted git repository.
hossman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/master by this push:
new 07071ca SOLR-15047: Fix collapse parser behavior when collapsing on numeric fields to differentiate '0' group from null group
07071ca is described below
commit 07071ca8e107d184f9dfc2a2271b5dcaaceda650
Author: Chris Hostetter <ho...@apache.org>
AuthorDate: Wed Jan 6 10:07:32 2021 -0700
SOLR-15047: Fix collapse parser behavior when collapsing on numeric fields to differentiate '0' group from null group
---
solr/CHANGES.txt | 2 +
.../solr/search/CollapsingQParserPlugin.java | 447 ++++++++++-----------
.../solr/search/TestCollapseQParserPlugin.java | 28 +-
3 files changed, 225 insertions(+), 252 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 687ebf7..eb2e73d 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -291,6 +291,8 @@ Bug Fixes
* SOLR-15058: Enforce node_name contains colon and port and find first underscore after colon to parse context
when converting a node_name to a base URL. (Timothy Potter, Su Sasa)
+* SOLR-15047: Fix collapse parser behavior when collapsing on numeric fields to differentiate '0' group from null group (hossman)
+
Other Changes
---------------------
diff --git a/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
index 256ec45..a9ccaed 100644
--- a/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
+++ b/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
@@ -764,13 +764,11 @@ public class CollapsingQParserPlugin extends QParserPlugin {
private int nullDoc = -1;
private FloatArrayList nullScores;
private String field;
- private int nullValue;
private final BoostedDocsCollector boostedDocsCollector;
public IntScoreCollector(int maxDoc,
int segments,
- int nullValue,
int nullPolicy,
int size,
String field,
@@ -784,7 +782,6 @@ public class CollapsingQParserPlugin extends QParserPlugin {
}
this.collapsedSet = new FixedBitSet(maxDoc);
- this.nullValue = nullValue;
this.nullPolicy = nullPolicy;
if(nullPolicy == NullPolicy.EXPAND.getCode()) {
nullScores = new FloatArrayList();
@@ -806,23 +803,12 @@ public class CollapsingQParserPlugin extends QParserPlugin {
@Override
public void collect(int contextDoc) throws IOException {
- int collapseValue;
+ final int globalDoc = docBase+contextDoc;
if (collapseValues.advanceExact(contextDoc)) {
- collapseValue = (int) collapseValues.longValue();
- } else {
- collapseValue = 0;
- }
- int globalDoc = docBase+contextDoc;
-
- // Check to see of we have documents boosted by the QueryElevationComponent
- if (collapseValue != nullValue) {
+ final int collapseValue = (int) collapseValues.longValue();
+ // Check to see if we have documents boosted by the QueryElevationComponent (skip normal strategy based collection)
if (boostedDocsCollector.collectIfBoosted(collapseValue, globalDoc)) return;
- } else {
- if (boostedDocsCollector.collectInNullGroupIfBoosted(globalDoc)) return;
- }
-
- if(collapseValue != nullValue) {
float score = scorer.score();
final int idx;
if((idx = cmap.indexOf(collapseValue)) >= 0) {
@@ -838,16 +824,24 @@ public class CollapsingQParserPlugin extends QParserPlugin {
long scoreDoc = (((long)Float.floatToRawIntBits(score))<<32)+globalDoc;
cmap.indexInsert(idx, collapseValue, scoreDoc);
}
- } else if(nullPolicy == NullPolicy.COLLAPSE.getCode()) {
- float score = scorer.score();
- if(score > this.nullScore) {
- this.nullScore = score;
- this.nullDoc = globalDoc;
+
+ } else { // Null Group...
+
+ // Check to see if we have documents boosted by the QueryElevationComponent (skip normal strategy based collection)
+ if (boostedDocsCollector.collectInNullGroupIfBoosted(globalDoc)) return;
+
+ if(nullPolicy == NullPolicy.COLLAPSE.getCode()) {
+ float score = scorer.score();
+ if(score > this.nullScore) {
+ this.nullScore = score;
+ this.nullDoc = globalDoc;
+ }
+ } else if(nullPolicy == NullPolicy.EXPAND.getCode()) {
+ collapsedSet.set(globalDoc);
+ nullScores.add(scorer.score());
}
- } else if(nullPolicy == NullPolicy.EXPAND.getCode()) {
- collapsedSet.set(globalDoc);
- nullScores.add(scorer.score());
}
+
}
@Override
@@ -895,24 +889,22 @@ public class CollapsingQParserPlugin extends QParserPlugin {
collapseValues = DocValues.getNumeric(contexts[currentContext].reader(), this.field);
}
- int contextDoc = globalDoc-currentDocBase;
- int collapseValue;
+ final int contextDoc = globalDoc-currentDocBase;
if (collapseValues.advanceExact(contextDoc)) {
- collapseValue = (int) collapseValues.longValue();
- } else {
- collapseValue = 0;
- }
-
- if(collapseValue != nullValue) {
- long scoreDoc = cmap.get(collapseValue);
+ final int collapseValue = (int) collapseValues.longValue();
+ final long scoreDoc = cmap.get(collapseValue);
dummy.score = Float.intBitsToFloat((int)(scoreDoc>>32));
- } else if(mergeBoost.boost(globalDoc)) {
- //It's an elevated doc so no score is needed (and should not have been populated)
- dummy.score = 0F;
- } else if (nullPolicy == NullPolicy.COLLAPSE.getCode()) {
- dummy.score = nullScore;
- } else if(nullPolicy == NullPolicy.EXPAND.getCode()) {
- dummy.score = nullScores.get(nullScoreIndex++);
+
+ } else { // Null Group...
+
+ if(mergeBoost.boost(globalDoc)) {
+ //It's an elevated doc so no score is needed (and should not have been populated)
+ dummy.score = 0F;
+ } else if (nullPolicy == NullPolicy.COLLAPSE.getCode()) {
+ dummy.score = nullScore;
+ } else if(nullPolicy == NullPolicy.EXPAND.getCode()) {
+ dummy.score = nullScores.get(nullScoreIndex++);
+ }
}
dummy.docId = contextDoc;
@@ -1143,7 +1135,6 @@ public class CollapsingQParserPlugin extends QParserPlugin {
private LeafReaderContext[] contexts;
private NumericDocValues collapseValues;
private int maxDoc;
- private int nullValue;
private int nullPolicy;
private IntFieldValueStrategy collapseStrategy;
@@ -1156,7 +1147,6 @@ public class CollapsingQParserPlugin extends QParserPlugin {
public IntFieldValueCollector(int maxDoc,
int size,
int segments,
- int nullValue,
int nullPolicy,
String collapseField,
GroupHeadSelector groupHeadSelector,
@@ -1177,7 +1167,6 @@ public class CollapsingQParserPlugin extends QParserPlugin {
contexts[i] = con.get(i);
}
this.collapseField = collapseField;
- this.nullValue = nullValue;
this.nullPolicy = nullPolicy;
this.needsScores4Collapsing = needsScores4Collapsing;
this.needsScores = needsScores;
@@ -1185,19 +1174,19 @@ public class CollapsingQParserPlugin extends QParserPlugin {
this.boostedDocsCollector = BoostedDocsCollector.build(boostDocsMap);
if (null != sortSpec) {
- this.collapseStrategy = new IntSortSpecStrategy(maxDoc, size, collapseField, nullValue, nullPolicy, groupHeadSelector, this.needsScores4Collapsing, this.needsScores, boostedDocsCollector, sortSpec, searcher);
+ this.collapseStrategy = new IntSortSpecStrategy(maxDoc, size, collapseField, nullPolicy, groupHeadSelector, this.needsScores4Collapsing, this.needsScores, boostedDocsCollector, sortSpec, searcher);
} else if (funcQuery != null) {
- this.collapseStrategy = new IntValueSourceStrategy(maxDoc, size, collapseField, nullValue, nullPolicy, groupHeadSelector, this.needsScores4Collapsing, this.needsScores, boostedDocsCollector, funcQuery, searcher);
+ this.collapseStrategy = new IntValueSourceStrategy(maxDoc, size, collapseField, nullPolicy, groupHeadSelector, this.needsScores4Collapsing, this.needsScores, boostedDocsCollector, funcQuery, searcher);
} else {
NumberType numType = fieldType.getNumberType();
assert null != numType; // shouldn't make it here for non-numeric types
switch (numType) {
case INTEGER: {
- this.collapseStrategy = new IntIntStrategy(maxDoc, size, collapseField, nullValue, nullPolicy, groupHeadSelector, this.needsScores, boostedDocsCollector);
+ this.collapseStrategy = new IntIntStrategy(maxDoc, size, collapseField, nullPolicy, groupHeadSelector, this.needsScores, boostedDocsCollector);
break;
}
case FLOAT: {
- this.collapseStrategy = new IntFloatStrategy(maxDoc, size, collapseField, nullValue, nullPolicy, groupHeadSelector, this.needsScores, boostedDocsCollector);
+ this.collapseStrategy = new IntFloatStrategy(maxDoc, size, collapseField, nullPolicy, groupHeadSelector, this.needsScores, boostedDocsCollector);
break;
}
default: {
@@ -1223,23 +1212,23 @@ public class CollapsingQParserPlugin extends QParserPlugin {
}
public void collect(int contextDoc) throws IOException {
- int collapseKey;
+ final int globalDoc = contextDoc+this.docBase;
if (collapseValues.advanceExact(contextDoc)) {
- collapseKey = (int) collapseValues.longValue();
- } else {
- collapseKey = 0;
- }
-
- int globalDoc = contextDoc+this.docBase;
-
- // Check to see if we have documents boosted by the QueryElevationComponent (skip normal strategy based collection)
- if (collapseKey == nullValue) {
- if (boostedDocsCollector.collectInNullGroupIfBoosted(globalDoc)) return;
- } else {
+ final int collapseKey = (int) collapseValues.longValue();
+ // Check to see if we have documents boosted by the QueryElevationComponent (skip normal strategy based collection)
if (boostedDocsCollector.collectIfBoosted(collapseKey, globalDoc)) return;
+ collapseStrategy.collapse(collapseKey, contextDoc, globalDoc);
+
+ } else { // Null Group...
+
+ // Check to see if we have documents boosted by the QueryElevationComponent (skip normal strategy based collection)
+ if (boostedDocsCollector.collectInNullGroupIfBoosted(globalDoc)) return;
+
+ if (NullPolicy.IGNORE.getCode() != nullPolicy) {
+ collapseStrategy.collapseNullGroup(contextDoc, globalDoc);
+ }
}
-
- collapseStrategy.collapse(collapseKey, contextDoc, globalDoc);
+
}
public void finish() throws IOException {
@@ -1274,26 +1263,25 @@ public class CollapsingQParserPlugin extends QParserPlugin {
this.collapseValues = DocValues.getNumeric(contexts[currentContext].reader(), this.collapseField);
}
- int contextDoc = globalDoc-currentDocBase;
+ final int contextDoc = globalDoc-currentDocBase;
if(this.needsScores){
- int collapseValue;
if (collapseValues.advanceExact(contextDoc)) {
- collapseValue = (int) collapseValues.longValue();
- } else {
- collapseValue = 0;
- }
-
- if(collapseValue != nullValue) {
- int pointer = cmap.get(collapseValue);
+ final int collapseValue = (int) collapseValues.longValue();
+
+ final int pointer = cmap.get(collapseValue);
dummy.score = scores.get(pointer);
- } else if (mergeBoost.boost(globalDoc)) {
- //Its an elevated doc so no score is needed
- dummy.score = 0F;
- } else if (nullPolicy == NullPolicy.COLLAPSE.getCode()) {
- dummy.score = nullScore;
- } else if(nullPolicy == NullPolicy.EXPAND.getCode()) {
- dummy.score = nullScores.get(nullScoreIndex++);
+
+ } else { // Null Group...
+
+ if (mergeBoost.boost(globalDoc)) {
+ //It's an elevated doc so no score is needed (and should not have been populated)
+ dummy.score = 0F;
+ } else if (nullPolicy == NullPolicy.COLLAPSE.getCode()) {
+ dummy.score = nullScore;
+ } else if(nullPolicy == NullPolicy.EXPAND.getCode()) {
+ dummy.score = nullScores.get(nullScoreIndex++);
+ }
}
}
@@ -1330,7 +1318,6 @@ public class CollapsingQParserPlugin extends QParserPlugin {
FunctionQuery funcQuery = null;
FieldType collapseFieldType = searcher.getSchema().getField(collapseField).getType();
- String defaultValue = searcher.getSchema().getField(collapseField).getDefaultValue();
if(collapseFieldType instanceof StrField) {
if(HINT_TOP_FC.equals(hint)) {
@@ -1384,23 +1371,8 @@ public class CollapsingQParserPlugin extends QParserPlugin {
return new OrdScoreCollector(maxDoc, leafCount, docValuesProducer, nullPolicy, boostDocs, searcher);
} else if (isNumericCollapsible(collapseFieldType)) {
-
- int nullValue = 0;
-
- // must be non-null at this point
- if (collapseFieldType.getNumberType().equals(NumberType.FLOAT)) {
- if (defaultValue != null) {
- nullValue = Float.floatToIntBits(Float.parseFloat(defaultValue));
- } else {
- nullValue = Float.floatToIntBits(0.0f);
- }
- } else {
- if (defaultValue != null) {
- nullValue = Integer.parseInt(defaultValue);
- }
- }
-
- return new IntScoreCollector(maxDoc, leafCount, nullValue, nullPolicy, size, collapseField, boostDocs, searcher);
+
+ return new IntScoreCollector(maxDoc, leafCount, nullPolicy, size, collapseField, boostDocs, searcher);
} else {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
@@ -1426,25 +1398,9 @@ public class CollapsingQParserPlugin extends QParserPlugin {
} else if (isNumericCollapsible(collapseFieldType)) {
- int nullValue = 0;
-
- // must be non-null at this point
- if (collapseFieldType.getNumberType().equals(NumberType.FLOAT)) {
- if (defaultValue != null) {
- nullValue = Float.floatToIntBits(Float.parseFloat(defaultValue));
- } else {
- nullValue = Float.floatToIntBits(0.0f);
- }
- } else {
- if (defaultValue != null) {
- nullValue = Integer.parseInt(defaultValue);
- }
- }
-
return new IntFieldValueCollector(maxDoc,
size,
leafCount,
- nullValue,
nullPolicy,
collapseField,
groupHeadSelector,
@@ -2015,22 +1971,20 @@ public class CollapsingQParserPlugin extends QParserPlugin {
protected boolean needsScores;
protected String collapseField;
protected IntIntDynamicMap docs;
- protected int nullValue;
private final BoostedDocsCollector boostedDocsCollector;
+ public abstract void collapseNullGroup(int contextDoc, int globalDoc) throws IOException;
public abstract void collapse(int collapseKey, int contextDoc, int globalDoc) throws IOException;
public abstract void setNextReader(LeafReaderContext context) throws IOException;
public IntFieldValueStrategy(int maxDoc,
int size,
String collapseField,
- int nullValue,
int nullPolicy,
boolean needsScores,
BoostedDocsCollector boostedDocsCollector) {
this.collapseField = collapseField;
- this.nullValue = nullValue;
this.nullPolicy = nullPolicy;
this.needsScores = needsScores;
this.collapsedSet = new FixedBitSet(maxDoc);
@@ -2109,13 +2063,12 @@ public class CollapsingQParserPlugin extends QParserPlugin {
public IntIntStrategy(int maxDoc,
int size,
String collapseField,
- int nullValue,
int nullPolicy,
GroupHeadSelector groupHeadSelector,
boolean needsScores,
BoostedDocsCollector boostedDocsCollector) throws IOException {
- super(maxDoc, size, collapseField, nullValue, nullPolicy, needsScores, boostedDocsCollector);
+ super(maxDoc, size, collapseField, nullPolicy, needsScores, boostedDocsCollector);
this.field = groupHeadSelector.selectorText;
this.testValues = new IntIntDynamicMap(size, 0);
@@ -2133,37 +2086,40 @@ public class CollapsingQParserPlugin extends QParserPlugin {
public void setNextReader(LeafReaderContext context) throws IOException {
this.minMaxVals = DocValues.getNumeric(context.reader(), this.field);
}
-
- public void collapse(int collapseKey, int contextDoc, int globalDoc) throws IOException {
-
- int currentVal;
+ private int advanceAndGetCurrentVal(int contextDoc) throws IOException {
if (minMaxVals.advanceExact(contextDoc)) {
- currentVal = (int) minMaxVals.longValue();
- } else {
- currentVal = 0;
- }
-
- if(collapseKey != nullValue) {
- final int idx;
- if((idx = cmap.indexOf(collapseKey)) >= 0) {
- int pointer = cmap.indexGet(idx);
- if(comp.test(currentVal, testValues.get(pointer))) {
- testValues.put(pointer, currentVal);
- docs.put(pointer, globalDoc);
- if(needsScores) {
- scores.put(pointer, scorer.score());
- }
- }
- } else {
- ++index;
- cmap.put(collapseKey, index);
- testValues.put(index, currentVal);
- docs.put(index, globalDoc);
+ return (int) minMaxVals.longValue();
+ } // else...
+ return 0;
+ }
+ public void collapse(int collapseKey, int contextDoc, int globalDoc) throws IOException {
+ final int currentVal = advanceAndGetCurrentVal(contextDoc);
+
+ final int idx;
+ if((idx = cmap.indexOf(collapseKey)) >= 0) {
+ int pointer = cmap.indexGet(idx);
+ if(comp.test(currentVal, testValues.get(pointer))) {
+ testValues.put(pointer, currentVal);
+ docs.put(pointer, globalDoc);
if(needsScores) {
- scores.put(index, scorer.score());
+ scores.put(pointer, scorer.score());
}
}
- } else if(this.nullPolicy == NullPolicy.COLLAPSE.getCode()) {
+ } else {
+ ++index;
+ cmap.put(collapseKey, index);
+ testValues.put(index, currentVal);
+ docs.put(index, globalDoc);
+ if(needsScores) {
+ scores.put(index, scorer.score());
+ }
+ }
+ }
+ public void collapseNullGroup(int contextDoc, int globalDoc) throws IOException {
+ assert NullPolicy.IGNORE.getCode() != this.nullPolicy;
+
+ final int currentVal = advanceAndGetCurrentVal(contextDoc);
+ if (this.nullPolicy == NullPolicy.COLLAPSE.getCode()) {
if(comp.test(currentVal, nullCompVal)) {
nullCompVal = currentVal;
nullDoc = globalDoc;
@@ -2193,13 +2149,12 @@ public class CollapsingQParserPlugin extends QParserPlugin {
public IntFloatStrategy(int maxDoc,
int size,
String collapseField,
- int nullValue,
int nullPolicy,
GroupHeadSelector groupHeadSelector,
boolean needsScores,
BoostedDocsCollector boostedDocsCollector) throws IOException {
- super(maxDoc, size, collapseField, nullValue, nullPolicy, needsScores, boostedDocsCollector);
+ super(maxDoc, size, collapseField, nullPolicy, needsScores, boostedDocsCollector);
this.field = groupHeadSelector.selectorText;
this.testValues = new IntFloatDynamicMap(size, 0.0f);
@@ -2217,39 +2172,40 @@ public class CollapsingQParserPlugin extends QParserPlugin {
public void setNextReader(LeafReaderContext context) throws IOException {
this.minMaxVals = DocValues.getNumeric(context.reader(), this.field);
}
-
- public void collapse(int collapseKey, int contextDoc, int globalDoc) throws IOException {
-
- int minMaxVal;
+ private float advanceAndGetCurrentVal(int contextDoc) throws IOException {
if (minMaxVals.advanceExact(contextDoc)) {
- minMaxVal = (int) minMaxVals.longValue();
- } else {
- minMaxVal = 0;
- }
-
- float currentVal = Float.intBitsToFloat(minMaxVal);
+ return Float.intBitsToFloat((int) minMaxVals.longValue());
+ } // else...
+ return Float.intBitsToFloat(0);
+ }
- if(collapseKey != nullValue) {
- final int idx;
- if((idx = cmap.indexOf(collapseKey)) >= 0) {
- int pointer = cmap.indexGet(idx);
- if(comp.test(currentVal, testValues.get(pointer))) {
- testValues.put(pointer, currentVal);
- docs.put(pointer, globalDoc);
- if(needsScores) {
- scores.put(pointer, scorer.score());
- }
- }
- } else {
- ++index;
- cmap.put(collapseKey, index);
- testValues.put(index, currentVal);
- docs.put(index, globalDoc);
+ public void collapse(int collapseKey, int contextDoc, int globalDoc) throws IOException {
+ final float currentVal = advanceAndGetCurrentVal(contextDoc);
+
+ final int idx;
+ if((idx = cmap.indexOf(collapseKey)) >= 0) {
+ int pointer = cmap.indexGet(idx);
+ if(comp.test(currentVal, testValues.get(pointer))) {
+ testValues.put(pointer, currentVal);
+ docs.put(pointer, globalDoc);
if(needsScores) {
- scores.put(index, scorer.score());
+ scores.put(pointer, scorer.score());
}
}
- } else if(this.nullPolicy == NullPolicy.COLLAPSE.getCode()) {
+ } else {
+ ++index;
+ cmap.put(collapseKey, index);
+ testValues.put(index, currentVal);
+ docs.put(index, globalDoc);
+ if(needsScores) {
+ scores.put(index, scorer.score());
+ }
+ }
+ }
+ public void collapseNullGroup(int contextDoc, int globalDoc) throws IOException {
+ assert NullPolicy.IGNORE.getCode() != this.nullPolicy;
+ final float currentVal = advanceAndGetCurrentVal(contextDoc);
+ if(this.nullPolicy == NullPolicy.COLLAPSE.getCode()) {
if(comp.test(currentVal, nullCompVal)) {
nullCompVal = currentVal;
nullDoc = globalDoc;
@@ -2265,7 +2221,7 @@ public class CollapsingQParserPlugin extends QParserPlugin {
}
}
}
-
+
/*
* Strategy for collapsing on a 32 bit numeric field and selecting the group head based
* on the min/max value of a Value Source Function.
@@ -2287,7 +2243,6 @@ public class CollapsingQParserPlugin extends QParserPlugin {
public IntValueSourceStrategy(int maxDoc,
int size,
String collapseField,
- int nullValue,
int nullPolicy,
GroupHeadSelector groupHeadSelector,
boolean needsScores4Collapsing,
@@ -2296,7 +2251,7 @@ public class CollapsingQParserPlugin extends QParserPlugin {
FunctionQuery funcQuery,
IndexSearcher searcher) throws IOException {
- super(maxDoc, size, collapseField, nullValue, nullPolicy, needsScores, boostedDocsCollector);
+ super(maxDoc, size, collapseField, nullPolicy, needsScores, boostedDocsCollector);
this.needsScores4Collapsing = needsScores4Collapsing;
this.testValues = new IntFloatDynamicMap(size, 0.0f);
@@ -2321,44 +2276,51 @@ public class CollapsingQParserPlugin extends QParserPlugin {
public void setNextReader(LeafReaderContext context) throws IOException {
functionValues = this.valueSource.getValues(rcontext, context);
}
-
- public void collapse(int collapseKey, int contextDoc, int globalDoc) throws IOException {
- float score = 0;
-
+ private float computeScoreIfNeeded4Collapse() throws IOException {
if (needsScores4Collapsing) {
- score = scorer.score();
- this.collapseScore.score = score;
- }
-
- float currentVal = functionValues.floatVal(contextDoc);
-
- if(collapseKey != nullValue) {
- final int idx;
- if((idx = cmap.indexOf(collapseKey)) >= 0) {
- int pointer = cmap.indexGet(idx);
- if(comp.test(currentVal, testValues.get(pointer))) {
- testValues.put(pointer, currentVal);
- docs.put(pointer, globalDoc);
- if(needsScores){
- if (!needsScores4Collapsing) {
- score = scorer.score();
- }
- scores.put(pointer, score);
- }
- }
- } else {
- ++index;
- cmap.put(collapseKey, index);
- docs.put(index, globalDoc);
- testValues.put(index, currentVal);
- if(needsScores) {
+ this.collapseScore.score = scorer.score();
+ return this.collapseScore.score;
+ } // else...
+ return 0F;
+ }
+ public void collapse(int collapseKey, int contextDoc, int globalDoc) throws IOException {
+
+ float score = computeScoreIfNeeded4Collapse();
+ final float currentVal = functionValues.floatVal(contextDoc);
+
+ final int idx;
+ if((idx = cmap.indexOf(collapseKey)) >= 0) {
+ int pointer = cmap.indexGet(idx);
+ if(comp.test(currentVal, testValues.get(pointer))) {
+ testValues.put(pointer, currentVal);
+ docs.put(pointer, globalDoc);
+ if(needsScores){
if (!needsScores4Collapsing) {
score = scorer.score();
}
- scores.put(index, score);
+ scores.put(pointer, score);
}
}
- } else if(this.nullPolicy == NullPolicy.COLLAPSE.getCode()) {
+ } else {
+ ++index;
+ cmap.put(collapseKey, index);
+ docs.put(index, globalDoc);
+ testValues.put(index, currentVal);
+ if(needsScores) {
+ if (!needsScores4Collapsing) {
+ score = scorer.score();
+ }
+ scores.put(index, score);
+ }
+ }
+ }
+ public void collapseNullGroup(int contextDoc, int globalDoc) throws IOException {
+ assert NullPolicy.IGNORE.getCode() != this.nullPolicy;
+
+ float score = computeScoreIfNeeded4Collapse();
+ final float currentVal = functionValues.floatVal(contextDoc);
+
+ if(this.nullPolicy == NullPolicy.COLLAPSE.getCode()) {
if(comp.test(currentVal, nullCompVal)) {
nullCompVal = currentVal;
nullDoc = globalDoc;
@@ -2398,7 +2360,6 @@ public class CollapsingQParserPlugin extends QParserPlugin {
public IntSortSpecStrategy(int maxDoc,
int size,
String collapseField,
- int nullValue,
int nullPolicy,
GroupHeadSelector groupHeadSelector,
boolean needsScores4Collapsing,
@@ -2407,7 +2368,7 @@ public class CollapsingQParserPlugin extends QParserPlugin {
SortSpec sortSpec,
IndexSearcher searcher) throws IOException {
- super(maxDoc, size, collapseField, nullValue, nullPolicy, needsScores, boostedDocsCollector);
+ super(maxDoc, size, collapseField, nullPolicy, needsScores, boostedDocsCollector);
this.needsScores4Collapsing = needsScores4Collapsing;
assert GroupHeadSelectorType.SORT.equals(groupHeadSelector.type);
@@ -2427,42 +2388,48 @@ public class CollapsingQParserPlugin extends QParserPlugin {
super.setScorer(s);
this.compareState.setScorer(s);
}
+
+ private float computeScoreIfNeeded4Collapse() throws IOException {
+ return needsScores4Collapsing ? scorer.score() : 0F;
+ }
public void collapse(int collapseKey, int contextDoc, int globalDoc) throws IOException {
- float score = 0;
-
- if (needsScores4Collapsing) {
- score = scorer.score();
- }
-
- if (collapseKey != nullValue) {
- final int idx;
- if ((idx = cmap.indexOf(collapseKey)) >= 0) {
- // we've seen this collapseKey before, test to see if it's a new group leader
- int pointer = cmap.indexGet(idx);
- if (compareState.testAndSetGroupValues(pointer, contextDoc)) {
- docs.put(pointer, globalDoc);
- if (needsScores) {
- if (!needsScores4Collapsing) {
- score = scorer.score();
- }
- scores.put(pointer, score);
- }
- }
- } else {
- // we've never seen this collapseKey before, treat it as group head for now
- ++index;
- cmap.put(collapseKey, index);
- docs.put(index, globalDoc);
- compareState.setGroupValues(index, contextDoc);
- if(needsScores) {
+ float score = computeScoreIfNeeded4Collapse();
+
+ final int idx;
+ if ((idx = cmap.indexOf(collapseKey)) >= 0) {
+ // we've seen this collapseKey before, test to see if it's a new group leader
+ int pointer = cmap.indexGet(idx);
+ if (compareState.testAndSetGroupValues(pointer, contextDoc)) {
+ docs.put(pointer, globalDoc);
+ if (needsScores) {
if (!needsScores4Collapsing) {
score = scorer.score();
}
- scores.put(index, score);
+ scores.put(pointer, score);
}
}
- } else if(this.nullPolicy == NullPolicy.COLLAPSE.getCode()) {
+ } else {
+ // we've never seen this collapseKey before, treat it as group head for now
+ ++index;
+ cmap.put(collapseKey, index);
+ docs.put(index, globalDoc);
+ compareState.setGroupValues(index, contextDoc);
+ if(needsScores) {
+ if (!needsScores4Collapsing) {
+ score = scorer.score();
+ }
+ scores.put(index, score);
+ }
+ }
+ }
+
+ public void collapseNullGroup(int contextDoc, int globalDoc) throws IOException {
+ assert NullPolicy.IGNORE.getCode() != this.nullPolicy;
+
+ float score = computeScoreIfNeeded4Collapse();
+
+ if(this.nullPolicy == NullPolicy.COLLAPSE.getCode()) {
if (-1 == nullDoc) {
// we've never seen a doc with null collapse key yet, treat it as the null group head for now
compareState.setNullGroupValues(contextDoc);
diff --git a/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java b/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java
index 56a088f..3d2e89d 100644
--- a/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java
+++ b/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java
@@ -1060,24 +1060,28 @@ public class TestCollapseQParserPlugin extends SolrTestCaseJ4 {
assertU(commit());
}
- for (String collapseField : new String[] {collapseFieldInt, collapseFieldFloat, collapseFieldString}) {
- assertQ(req(
- "q", "{!cache=false}field_s:1",
- "rows", "1",
- "minExactCount", "1",
- // this collapse will end up matching all docs
- "fq", "{!collapse field=" + collapseField + " nullPolicy=expand}"// nullPolicy needed due to a bug when val=0
- ),"//*[@numFoundExact='true']"
- ,"//*[@numFound='" + (numDocs/2) + "']"
- );
+ for (String collapseField : Arrays.asList(collapseFieldInt, collapseFieldFloat, collapseFieldString)) {
+ // all of our docs have a value in the collapse field(s) so the policy shouldn't matter...
+ for (String policy : Arrays.asList("", " nullPolicy=ignore", " nullPolicy=expand", " nullPolicy=collapse")) {
+ assertQ(req("q", "{!cache=false}field_s:1",
+ "rows", "1",
+ "minExactCount", "1", // collapse should force this to be ignored
+ // this collapse will end up creating a group for each matched doc
+ "fq", "{!collapse field=" + collapseField + policy + "}"
+ )
+ , "//*[@numFoundExact='true']"
+ , "//*[@numFound='" + (numDocs/2) + "']"
+ );
+ }
}
}
public void testNullGroupNumericVsStringCollapse() throws Exception {
// NOTE: group_i and group_s will contain identical content so these need to be "numbers"...
- // The specific numbers shouldn't matter, but until SOLR-15047 is fixed, we can't use "0"...
+ // The specific numbers shouldn't matter (and we explicitly test '0' to confirm legacy bug/behavior
+ // of treating 0 as null is no longer a problem) ...
final String A = "-1";
- final String B = "42"; // TODO: switch to "0" once SOLR-15047 is fixed
+ final String B = "0";
final String C = "1";
// Stub out our documents. From now on assume highest "id" of each group should be group head...