Apache
/
lucene
mirror of https://github.com/apache/lucene.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

SOLR-9288: Fix [docid] transformer to return -1 when used in RTG with uncommitted doc 

(cherry picked from commit 08019f4288)
This commit is contained in:
Chris Hostetter 2016-07-19 10:50:02 -07:00
parent dfa3f61ecf
commit 9feeee4e27
4 changed files with 90 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
solr/CHANGES.txt
View File

@ -123,6 +123,8 @@ Bug Fixes
* SOLR-9285: Fixed AIOOBE when using ValueSourceAugmenter in single node RTG (hossman)
* SOLR-9288: Fix [docid] transformer to return -1 when used in RTG with uncommitted doc (hossman)
Optimizations
----------------------

10
solr/core/src/java/org/apache/solr/response/transform/DocIdAugmenterFactory.java
View File

@ -21,7 +21,10 @@ import org.apache.solr.common.params.SolrParams;
import org.apache.solr.request.SolrQueryRequest;
/**
*
* Augments the document with a <code>[docid]</code> integer containing it's current
* (internal) id in the lucene index. May be <code>-1</code> if this document did not come from the
* index (ie: a RealTimeGet from the transaction log)
*
* @since solr 4.0
*/
public class DocIdAugmenterFactory extends TransformerFactory
@ -49,9 +52,8 @@ class DocIdAugmenter extends DocTransformer
@Override
public void transform(SolrDocument doc, int docid, float score) {
if( docid >= 0 ) {
doc.setField( name, docid );
}
assert -1 <= docid;
doc.setField( name, docid );
}
}

61
solr/core/src/test/org/apache/solr/cloud/TestRandomFlRTGCloud.java
View File

@ -47,6 +47,7 @@ import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.response.transform.DocTransformer; // jdocs
import org.apache.solr.util.RandomizeSSL;
import org.apache.lucene.util.TestUtil;
@ -90,8 +91,6 @@ public class TestRandomFlRTGCloud extends SolrCloudTestCase {
(Arrays.<FlValidator>asList(
// TODO: SOLR-9314: add more of these for other various transformers
//
// TODO: add a [docid] validator (blocked by SOLR-9288 & SOLR-9289)
//
new GlobValidator("*"),
new GlobValidator("*_i"),
new GlobValidator("*_s"),
@ -119,6 +118,9 @@ public class TestRandomFlRTGCloud extends SolrCloudTestCase {
new RenameFieldValueValidator("id", "my_id_alias"),
new RenameFieldValueValidator("bbb_i", "my_int_field_alias"),
new RenameFieldValueValidator("ddd_s", "my_str_field_alias")));
// SOLR-9289...
FL_VALIDATORS.add(new DocIdValidator());
FL_VALIDATORS.add(new DocIdValidator("my_docid_alias"));
} else {
// No-Op
// No known transformers that only work in distrib cloud but fail in singleCoreMode
@ -428,7 +430,7 @@ public class TestRandomFlRTGCloud extends SolrCloudTestCase {
}
/**
* abstraction for diff types of things that can be added to an 'fl' param that can validate
* Abstraction for diff types of things that can be added to an 'fl' param that can validate
* the results are correct compared to an expected SolrInputDocument
*/
private interface FlValidator {
@ -441,6 +443,21 @@ public class TestRandomFlRTGCloud extends SolrCloudTestCase {
}
params.add(buildCommaSepParams(random(), "fl", fls));
}
/**
* Indicates if this validator is for a transformer that returns true from
* {@link DocTransformer#needsSolrIndexSearcher}. Other validators for transformers that
* do <em>not</em> require a re-opened searcher (but may have slightly diff behavior depending
* on wether a doc comesfrom the index or from the update log) may use this information to
* decide wether they wish to enforce stricter assertions on the resulting document.
*
* The default implementation always returns <code>false</code>
*
* @see DocIdValidator
*/
public default boolean requiresRealtimeSearcherReOpen() {
return false;
}
/**
* Must return a non null String that can be used in an fl param -- either by itself,
@ -496,6 +513,42 @@ public class TestRandomFlRTGCloud extends SolrCloudTestCase {
public String getFlParam() { return actualFieldName + ":" + expectedFieldName; }
}
/**
* enforces that a valid <code>[docid]</code> is present in the response, possibly using a
* resultKey alias. By default the only validation of docId values is that they are an integer
* greater than or equal to <code>-1</code> -- but if any other validator in use returns true
* from {@link #requiresRealtimeSearcherReOpen} then the constraint is tightened and values must
* be greater than or equal to <code>0</code>
*/
private static class DocIdValidator implements FlValidator {
private final String resultKey;
public DocIdValidator(final String resultKey) {
this.resultKey = resultKey;
}
public DocIdValidator() {
this("[docid]");
}
public String getFlParam() { return "[docid]".equals(resultKey) ? resultKey : resultKey+":[docid]"; }
public Collection<String> assertRTGResults(final Collection<FlValidator> validators,
final SolrInputDocument expected,
final SolrDocument actual) {
final Object value = actual.getFirstValue(resultKey);
assertNotNull(getFlParam() + " => no value in actual doc", value);
assertTrue("[docid] must be an Integer: " + value, value instanceof Integer);
int minValidDocId = -1; // if it comes from update log
for (FlValidator other : validators) {
if (other.requiresRealtimeSearcherReOpen()) {
minValidDocId = 0;
break;
}
}
assertTrue("[docid] must be >= " + minValidDocId + ": " + value,
minValidDocId <= ((Integer)value).intValue());
return Collections.<String>singleton(resultKey);
}
}
/** Trivial validator of a ValueSourceAugmenter */
private static class FunctionValidator implements FlValidator {
private static String func(String fieldName) {
@ -515,6 +568,8 @@ public class TestRandomFlRTGCloud extends SolrCloudTestCase {
this.resultKey = resultKey;
this.fieldName = fieldName;
}
/** always returns true */
public boolean requiresRealtimeSearcherReOpen() { return true; }
public String getFlParam() { return fl; }
public Collection<String> assertRTGResults(final Collection<FlValidator> validators,
final SolrInputDocument expected,

60
solr/core/src/test/org/apache/solr/search/TestPseudoReturnFields.java
View File

@ -531,21 +531,13 @@ public class TestPseudoReturnFields extends SolrTestCaseJ4 {
}
}
@AwaitsFix(bugUrl="https://issues.apache.org/jira/browse/SOLR-9288")
public void testDocIdAugmenterRTG() throws Exception {
// NOTE: once this test is fixed to pass, testAugmentersRTG should also be updated to test [docid]
// TODO: behavior of fl=[docid] should be consistent regardless of wether doc is committed
// what should behavior be?
// right now, for an uncommited doc, [docid] is silently ignored and no value included in result
// perhaps it should be "null" or "-1" ?
// behavior shouldn't matter if we are committed or uncommitted
// for an uncommitted doc, we should get -1
for (String id : Arrays.asList("42","99")) {
assertQ(id + ": fl=[docid]",
req("qt","/get","id",id, "wt","xml", "fl","[docid]")
,"count(//doc)=1"
,"//doc/int[@name='[docid]']"
,"//doc/int[@name='[docid]'][.>=-1]"
,"//doc[count(*)=1]"
);
}
@ -554,22 +546,21 @@ public class TestPseudoReturnFields extends SolrTestCaseJ4 {
public void testAugmentersRTG() throws Exception {
// behavior shouldn't matter if we are committed or uncommitted
for (String id : Arrays.asList("42","99")) {
// NOTE: once testDocIdAugmenterRTG can pass, [docid] should be tested here as well.
for (SolrParams p : Arrays.asList
(params("fl","[shard],[explain],x_alias:[value v=10 t=int],abs(val_i)"),
params("fl","[shard],abs(val_i)","fl","[explain],x_alias:[value v=10 t=int]"),
params("fl","[shard]","fl","[explain],x_alias:[value v=10 t=int]","fl","abs(val_i)"),
params("fl","[shard]","fl","[explain]","fl","x_alias:[value v=10 t=int]","fl","abs(val_i)"))) {
(params("fl","[docid],[shard],[explain],x_alias:[value v=10 t=int],abs(val_i)"),
params("fl","[docid],[shard],abs(val_i)","fl","[explain],x_alias:[value v=10 t=int]"),
params("fl","[docid],[shard]","fl","[explain],x_alias:[value v=10 t=int]","fl","abs(val_i)"),
params("fl","[docid]","fl","[shard]","fl","[explain]","fl","x_alias:[value v=10 t=int]","fl","abs(val_i)"))) {
assertQ(id + ": " + p,
req(p, "qt","/get","id",id, "wt","xml")
,"count(//doc)=1"
// ,"//doc/int[@name='[docid]']" // TODO
,"//doc/int[@name='[docid]'][.>=-1]"
,"//doc/float[@name='abs(val_i)'][.='1.0']"
,"//doc/str[@name='[shard]'][.='[not a shard request]']"
// RTG: [explain] should be missing (ignored)
,"//doc/int[@name='x_alias'][.=10]"
,"//doc[count(*)=3]"
,"//doc[count(*)=4]"
);
}
}
@ -595,21 +586,20 @@ public class TestPseudoReturnFields extends SolrTestCaseJ4 {
public void testAugmentersAndExplicitRTG() throws Exception {
// behavior shouldn't matter if we are committed or uncommitted
for (String id : Arrays.asList("42","99")) {
// NOTE: once testDocIdAugmenterRTG can pass, [docid] should be tested here as well.
for (SolrParams p : Arrays.asList
(params("fl","id,[explain],x_alias:[value v=10 t=int],abs(val_i)"),
params("fl","id,abs(val_i)","fl","[explain],x_alias:[value v=10 t=int]"),
params("fl","id","fl","[explain]","fl","x_alias:[value v=10 t=int]","fl","abs(val_i)"))) {
(params("fl","id,[docid],[explain],x_alias:[value v=10 t=int],abs(val_i)"),
params("fl","id,[docid],abs(val_i)","fl","[explain],x_alias:[value v=10 t=int]"),
params("fl","id","fl","[docid]","fl","[explain]","fl","x_alias:[value v=10 t=int]","fl","abs(val_i)"))) {
assertQ(id + ": " + p,
req(p, "qt","/get","id",id, "wt","xml")
,"count(//doc)=1"
,"//doc/str[@name='id']"
// ,"//doc/int[@name='[docid]']" // TODO
,"//doc/int[@name='[docid]'][.>=-1]"
,"//doc/float[@name='abs(val_i)'][.='1.0']"
// RTG: [explain] should be missing (ignored)
,"//doc/int[@name='x_alias'][.=10]"
,"//doc[count(*)=3]"
,"//doc[count(*)=4]"
);
}
}
@ -646,29 +636,28 @@ public class TestPseudoReturnFields extends SolrTestCaseJ4 {
public void testAugmentersAndScoreRTG() throws Exception {
// if we use RTG (committed or otherwise) score should be ignored
for (String id : Arrays.asList("42","99")) {
// NOTE: once testDocIdAugmenterRTG can pass, [docid] should be tested here as well.
assertQ(id,
req("qt","/get","id",id, "wt","xml",
"fl","x_alias:[value v=10 t=int],score,abs(val_i)")
// ,"//doc/int[@name='[docid]']" // TODO
"fl","x_alias:[value v=10 t=int],score,abs(val_i),[docid]")
,"//doc/int[@name='[docid]'][.>=-1]"
,"//doc/float[@name='abs(val_i)'][.='1.0']"
,"//doc/int[@name='x_alias'][.=10]"
,"//doc[count(*)=2]"
,"//doc[count(*)=3]"
);
for (SolrParams p : Arrays.asList(params("fl","x_alias:[value v=10 t=int],[explain],score,abs(val_i)"),
params("fl","x_alias:[value v=10 t=int],[explain]","fl","score,abs(val_i)"),
params("fl","x_alias:[value v=10 t=int]","fl","[explain]","fl","score","fl","abs(val_i)"))) {
for (SolrParams p : Arrays.asList(params("fl","[docid],x_alias:[value v=10 t=int],[explain],score,abs(val_i)"),
params("fl","x_alias:[value v=10 t=int],[explain]","fl","[docid],score,abs(val_i)"),
params("fl","[docid]","fl","x_alias:[value v=10 t=int]","fl","[explain]","fl","score","fl","abs(val_i)"))) {
assertQ(p.toString(),
req(p, "qt","/get","id",id, "wt","xml")
// ,"//doc/int[@name='[docid]']" // TODO
,"//doc/int[@name='[docid]']" // TODO
,"//doc/float[@name='abs(val_i)'][.='1.0']"
,"//doc/int[@name='x_alias'][.=10]"
// RTG: [explain] and score should be missing (ignored)
,"//doc[count(*)=2]"
,"//doc[count(*)=3]"
);
}
}
@ -713,8 +702,7 @@ public class TestPseudoReturnFields extends SolrTestCaseJ4 {
// NOTE: 'ssto' is the missing one
final List<String> fl = Arrays.asList
// NOTE: once testDocIdAugmenterRTG can pass, [docid] should be tested here as well.
("id","[explain]","score","val_*","subj*","abs(val_i)");
("id","[explain]","score","val_*","subj*","abs(val_i)","[docid]");
final int iters = atLeast(random, 10);
for (int i = 0; i< iters; i++) {
@ -734,12 +722,12 @@ public class TestPseudoReturnFields extends SolrTestCaseJ4 {
req(p, "qt","/get","id",id, "wt","xml")
,"count(//doc)=1"
,"//doc/str[@name='id']"
// ,"//doc/int[@name='[docid]']" // TODO
,"//doc/int[@name='[docid]'][.>=-1]"
,"//doc/float[@name='abs(val_i)'][.='1.0']"
// RTG: [explain] and score should be missing (ignored)
,"//doc/int[@name='val_i'][.=1]"
,"//doc/str[@name='subject']"
,"//doc[count(*)=4]"
,"//doc[count(*)=5]"
);
}
}