From c186cbfcbc4bf838d0756892c9710b1a67883590 Mon Sep 17 00:00:00 2001 From: Shalin Shekhar Mangar Date: Sat, 13 Dec 2014 00:05:00 +0000 Subject: [PATCH] SOLR-6604: SOLR-6812: Fix NPE with distrib.singlePass=true and expand component. Increased test coverage of expand component with docValues. This closes #98. git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1645098 13f79535-47bb-0310-9956-ffa450edef68 --- solr/CHANGES.txt | 10 +-- .../handler/component/ExpandComponent.java | 26 ++++-- .../DistributedExpandComponentTest.java | 83 ++++++++++++++----- .../component/TestExpandComponent.java | 53 ++++++++---- 4 files changed, 120 insertions(+), 52 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 71e735f2047..6bcf9be3f7f 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -319,6 +319,10 @@ Bug Fixes * SOLR-6626: NPE in FieldMutatingUpdateProcessor when indexing a doc with null field value (Noble Paul) +* SOLR-6604: SOLR-6812: Fix NPE with distrib.singlePass=true and expand + component. Increased test coverage of expand component with docValues. + (Christine Poerschke, Per Steffensen, shalin) + Optimizations ---------------------- @@ -484,12 +488,6 @@ Other Changes the example directory instead of server/solr. (Alexandre Rafalovitch, Anshum Gupta, hossman, Timothy Potter) -* SOLR-6843: JMX RMI connector should be disabled by default but can be activated by - setting ENABLE_REMOTE_JMX_OPTS to true in solr.in.(sh|cmd). (Timothy Potter) - -* SOLR-6844: Rename ConfigSolr.getZkHostPort(), which actually returns the Solr port, - to .getSolrHostPort(). (Martijn Koster, Steve Rowe) - ================== 4.10.3 ================== Bug Fixes diff --git a/solr/core/src/java/org/apache/solr/handler/component/ExpandComponent.java b/solr/core/src/java/org/apache/solr/handler/component/ExpandComponent.java index 9336f54fa56..6b39a014c9b 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/ExpandComponent.java +++ b/solr/core/src/java/org/apache/solr/handler/component/ExpandComponent.java @@ -86,6 +86,7 @@ import com.carrotsearch.hppc.cursors.ObjectCursor; */ public class ExpandComponent extends SearchComponent implements PluginInfoInitialized, SolrCoreAware { public static final String COMPONENT_NAME = "expand"; + private static final int finishingStage = ResponseBuilder.STAGE_GET_FIELDS; private PluginInfo info = PluginInfo.EMPTY_INFO; @Override @@ -116,13 +117,6 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia SolrQueryRequest req = rb.req; SolrParams params = req.getParams(); - boolean isShard = params.getBool(ShardParams.IS_SHARD, false); - String ids = params.get(ShardParams.IDS); - - if (ids == null && isShard) { - return; - } - String field = params.get(ExpandParams.EXPAND_FIELD); if (field == null) { List filters = rb.getFilters(); @@ -246,9 +240,23 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia rb.rsp.add("expanded", outMap); } + @Override + public int distributedProcess(ResponseBuilder rb) throws IOException { + if (rb.doExpand && rb.stage < finishingStage) { + return finishingStage; + } + return ResponseBuilder.STAGE_DONE; + } + @Override public void modifyRequest(ResponseBuilder rb, SearchComponent who, ShardRequest sreq) { - + SolrParams params = rb.req.getParams(); + if (!params.getBool(COMPONENT_NAME, false)) return; + if (!rb.onePassDistributedQuery && (sreq.purpose & ShardRequest.PURPOSE_GET_FIELDS) == 0) { + sreq.params.set(COMPONENT_NAME, "false"); + } else { + sreq.params.set(COMPONENT_NAME, "true"); + } } @SuppressWarnings("unchecked") @@ -286,7 +294,7 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia return; } - if (rb.stage != ResponseBuilder.STAGE_GET_FIELDS) { + if (rb.stage != finishingStage) { return; } diff --git a/solr/core/src/test/org/apache/solr/handler/component/DistributedExpandComponentTest.java b/solr/core/src/test/org/apache/solr/handler/component/DistributedExpandComponentTest.java index e06a43960b4..9cd6f52c016 100644 --- a/solr/core/src/test/org/apache/solr/handler/component/DistributedExpandComponentTest.java +++ b/solr/core/src/test/org/apache/solr/handler/component/DistributedExpandComponentTest.java @@ -50,20 +50,22 @@ public class DistributedExpandComponentTest extends BaseDistributedSearchTestCas @Override public void doTest() throws Exception { + final String group = (random().nextBoolean() ? "group_s" : "group_s_dv"); + del("*:*"); - index_specific(0,"id","1", "term_s", "YYYY", "group_s", "group1", "test_ti", "5", "test_tl", "10", "test_tf", "2000"); - index_specific(0,"id","2", "term_s", "YYYY", "group_s", "group1", "test_ti", "50", "test_tl", "100", "test_tf", "200"); - index_specific(1,"id","5", "term_s", "YYYY", "group_s", "group2", "test_ti", "4", "test_tl", "10", "test_tf", "2000"); - index_specific(1,"id","6", "term_s", "YYYY", "group_s", "group2", "test_ti", "10", "test_tl", "100", "test_tf", "200"); - index_specific(0,"id","7", "term_s", "YYYY", "group_s", "group1", "test_ti", "1", "test_tl", "100000", "test_tf", "2000"); - index_specific(1,"id","8", "term_s", "YYYY", "group_s", "group2", "test_ti", "2", "test_tl", "100000", "test_tf", "200"); - index_specific(2,"id","9", "term_s", "YYYY", "group_s", "group3", "test_ti", "1000", "test_tl", "1005", "test_tf", "3000"); - index_specific(2, "id", "10", "term_s", "YYYY", "group_s", "group3", "test_ti", "1500", "test_tl", "1001", "test_tf", "3200"); - index_specific(2,"id", "11", "term_s", "YYYY", "group_s", "group3", "test_ti", "1300", "test_tl", "1002", "test_tf", "3300"); - index_specific(1,"id","12", "term_s", "YYYY", "group_s", "group4", "test_ti", "15", "test_tl", "10", "test_tf", "2000"); - index_specific(1,"id","13", "term_s", "YYYY", "group_s", "group4", "test_ti", "16", "test_tl", "9", "test_tf", "2000"); - index_specific(1,"id","14", "term_s", "YYYY", "group_s", "group4", "test_ti", "1", "test_tl", "20", "test_tf", "2000"); + index_specific(0,"id","1", "term_s", "YYYY", group, "group1", "test_ti", "5", "test_tl", "10", "test_tf", "2000"); + index_specific(0,"id","2", "term_s", "YYYY", group, "group1", "test_ti", "50", "test_tl", "100", "test_tf", "200"); + index_specific(1,"id","5", "term_s", "YYYY", group, "group2", "test_ti", "4", "test_tl", "10", "test_tf", "2000"); + index_specific(1,"id","6", "term_s", "YYYY", group, "group2", "test_ti", "10", "test_tl", "100", "test_tf", "200"); + index_specific(0,"id","7", "term_s", "YYYY", group, "group1", "test_ti", "1", "test_tl", "100000", "test_tf", "2000"); + index_specific(1,"id","8", "term_s", "YYYY", group, "group2", "test_ti", "2", "test_tl", "100000", "test_tf", "200"); + index_specific(2,"id","9", "term_s", "YYYY", group, "group3", "test_ti", "1000", "test_tl", "1005", "test_tf", "3000"); + index_specific(2, "id", "10", "term_s", "YYYY", group, "group3", "test_ti", "1500", "test_tl", "1001", "test_tf", "3200"); + index_specific(2,"id", "11", "term_s", "YYYY", group, "group3", "test_ti", "1300", "test_tl", "1002", "test_tf", "3300"); + index_specific(1,"id","12", "term_s", "YYYY", group, "group4", "test_ti", "15", "test_tl", "10", "test_tf", "2000"); + index_specific(1,"id","13", "term_s", "YYYY", group, "group4", "test_ti", "16", "test_tl", "9", "test_tf", "2000"); + index_specific(1,"id","14", "term_s", "YYYY", group, "group4", "test_ti", "1", "test_tl", "20", "test_tf", "2000"); commit(); @@ -80,21 +82,21 @@ public class DistributedExpandComponentTest extends BaseDistributedSearchTestCas handle.put("maxScore", SKIPVAL); handle.put("_version_", SKIP); - query("q", "*:*", "fq", "{!collapse field=group_s}", "defType", "edismax", "bf", "field(test_ti)", "expand", "true", "fl","*,score"); - query("q", "*:*", "fq", "{!collapse field=group_s}", "defType", "edismax", "bf", "field(test_ti)", "expand", "true", "expand.sort", "test_tl desc", "fl","*,score"); - query("q", "*:*", "fq", "{!collapse field=group_s}", "defType", "edismax", "bf", "field(test_ti)", "expand", "true", "expand.sort", "test_tl desc", "expand.rows", "1", "fl","*,score"); + query("q", "*:*", "fq", "{!collapse field="+group+"}", "defType", "edismax", "bf", "field(test_ti)", "expand", "true", "fl","*,score"); + query("q", "*:*", "fq", "{!collapse field="+group+"}", "defType", "edismax", "bf", "field(test_ti)", "expand", "true", "expand.sort", "test_tl desc", "fl","*,score"); + query("q", "*:*", "fq", "{!collapse field="+group+"}", "defType", "edismax", "bf", "field(test_ti)", "expand", "true", "expand.sort", "test_tl desc", "expand.rows", "1", "fl","*,score"); //Test no expand results - query("q", "test_ti:5", "fq", "{!collapse field=group_s}", "defType", "edismax", "bf", "field(test_ti)", "expand", "true", "expand.sort", "test_tl desc", "expand.rows", "1", "fl","*,score"); + query("q", "test_ti:5", "fq", "{!collapse field="+group+"}", "defType", "edismax", "bf", "field(test_ti)", "expand", "true", "expand.sort", "test_tl desc", "expand.rows", "1", "fl","*,score"); //Test zero results - query("q", "test_ti:5434343", "fq", "{!collapse field=group_s}", "defType", "edismax", "bf", "field(test_ti)", "expand", "true", "expand.sort", "test_tl desc", "expand.rows", "1", "fl","*,score"); + query("q", "test_ti:5434343", "fq", "{!collapse field="+group+"}", "defType", "edismax", "bf", "field(test_ti)", "expand", "true", "expand.sort", "test_tl desc", "expand.rows", "1", "fl","*,score"); //Test page 2 - query("q", "*:*", "start","1", "rows", "1", "fq", "{!collapse field=group_s}", "defType", "edismax", "bf", "field(test_ti)", "expand", "true", "fl","*,score"); + query("q", "*:*", "start","1", "rows", "1", "fq", "{!collapse field="+group+"}", "defType", "edismax", "bf", "field(test_ti)", "expand", "true", "fl","*,score"); //First basic test case. ModifiableSolrParams params = new ModifiableSolrParams(); params.add("q", "*:*"); - params.add("fq", "{!collapse field=group_s}"); + params.add("fq", "{!collapse field="+group+"}"); params.add("defType", "edismax"); params.add("bf", "field(test_ti)"); params.add("expand", "true"); @@ -113,7 +115,7 @@ public class DistributedExpandComponentTest extends BaseDistributedSearchTestCas params = new ModifiableSolrParams(); params.add("q", "*:*"); - params.add("fq", "{!collapse field=group_s}"); + params.add("fq", "{!collapse field="+group+"}"); params.add("defType", "edismax"); params.add("bf", "field(test_ti)"); params.add("expand", "true"); @@ -132,7 +134,7 @@ public class DistributedExpandComponentTest extends BaseDistributedSearchTestCas params = new ModifiableSolrParams(); params.add("q", "*:*"); - params.add("fq", "{!collapse field=group_s}"); + params.add("fq", "{!collapse field="+group+"}"); params.add("defType", "edismax"); params.add("bf", "field(test_ti)"); params.add("expand", "true"); @@ -147,6 +149,45 @@ public class DistributedExpandComponentTest extends BaseDistributedSearchTestCas assertExpandGroupCountAndOrder("group3", 1, results, "9.0"); assertExpandGroupCountAndOrder("group4", 1, results, "14.0"); + + //Test key-only fl + + params = new ModifiableSolrParams(); + params.add("q", "*:*"); + params.add("fq", "{!collapse field="+group+"}"); + params.add("defType", "edismax"); + params.add("bf", "field(test_ti)"); + params.add("expand", "true"); + params.add("fl", "id"); + + setDistributedParams(params); + rsp = queryServer(params); + results = rsp.getExpandedResults(); + assertExpandGroups(results, "group1","group2", "group3", "group4"); + assertExpandGroupCountAndOrder("group1", 2, results, "1.0", "7.0"); + assertExpandGroupCountAndOrder("group2", 2, results, "5.0", "8.0"); + assertExpandGroupCountAndOrder("group3", 2, results, "11.0", "9.0"); + assertExpandGroupCountAndOrder("group4", 2, results, "12.0", "14.0"); + + //Test distrib.singlePass true + + params = new ModifiableSolrParams(); + params.add("q", "*:*"); + params.add("fq", "{!collapse field="+group+"}"); + params.add("defType", "edismax"); + params.add("bf", "field(test_ti)"); + params.add("expand", "true"); + params.add("distrib.singlePass", "true"); + + setDistributedParams(params); + rsp = queryServer(params); + results = rsp.getExpandedResults(); + assertExpandGroups(results, "group1","group2", "group3", "group4"); + assertExpandGroupCountAndOrder("group1", 2, results, "1.0", "7.0"); + assertExpandGroupCountAndOrder("group2", 2, results, "5.0", "8.0"); + assertExpandGroupCountAndOrder("group3", 2, results, "11.0", "9.0"); + assertExpandGroupCountAndOrder("group4", 2, results, "12.0", "14.0"); + } private void assertExpandGroups(Map expandedResults, String... groups) throws Exception { diff --git a/solr/core/src/test/org/apache/solr/handler/component/TestExpandComponent.java b/solr/core/src/test/org/apache/solr/handler/component/TestExpandComponent.java index 4d1b3a05746..2da638dccaa 100644 --- a/solr/core/src/test/org/apache/solr/handler/component/TestExpandComponent.java +++ b/solr/core/src/test/org/apache/solr/handler/component/TestExpandComponent.java @@ -45,10 +45,12 @@ public class TestExpandComponent extends SolrTestCaseJ4 { @Test public void testExpand() throws Exception { - String[] doc = {"id","1", "term_s", "YYYY", "group_s", "group1", "test_ti", "5", "test_tl", "10", "test_tf", "2000", "type_s", "parent"}; + final String group = (random().nextBoolean() ? "group_s" : "group_s_dv"); + + String[] doc = {"id","1", "term_s", "YYYY", group, "group1", "test_ti", "5", "test_tl", "10", "test_tf", "2000", "type_s", "parent"}; assertU(adoc(doc)); assertU(commit()); - String[] doc1 = {"id","2", "term_s","YYYY", "group_s", "group1", "test_ti", "50", "test_tl", "100", "test_tf", "200", "type_s", "child"}; + String[] doc1 = {"id","2", "term_s","YYYY", group, "group1", "test_ti", "50", "test_tl", "100", "test_tf", "200", "type_s", "child"}; assertU(adoc(doc1)); String[] doc2 = {"id","3", "term_s", "YYYY", "test_ti", "5000", "test_tl", "100", "test_tf", "200"}; @@ -58,17 +60,17 @@ public class TestExpandComponent extends SolrTestCaseJ4 { assertU(adoc(doc3)); - String[] doc4 = {"id","5", "term_s", "YYYY", "group_s", "group2", "test_ti", "4", "test_tl", "10", "test_tf", "2000", "type_s", "parent"}; + String[] doc4 = {"id","5", "term_s", "YYYY", group, "group2", "test_ti", "4", "test_tl", "10", "test_tf", "2000", "type_s", "parent"}; assertU(adoc(doc4)); assertU(commit()); - String[] doc5 = {"id","6", "term_s","YYYY", "group_s", "group2", "test_ti", "10", "test_tl", "100", "test_tf", "200", "type_s", "child"}; + String[] doc5 = {"id","6", "term_s","YYYY", group, "group2", "test_ti", "10", "test_tl", "100", "test_tf", "200", "type_s", "child"}; assertU(adoc(doc5)); assertU(commit()); - String[] doc6 = {"id","7", "term_s", "YYYY", "group_s", "group1", "test_ti", "1", "test_tl", "100000", "test_tf", "2000", "type_s", "child"}; + String[] doc6 = {"id","7", "term_s", "YYYY", group, "group1", "test_ti", "1", "test_tl", "100000", "test_tf", "2000", "type_s", "child"}; assertU(adoc(doc6)); assertU(commit()); - String[] doc7 = {"id","8", "term_s","YYYY", "group_s", "group2", "test_ti", "2", "test_tl", "100000", "test_tf", "200", "type_s", "child"}; + String[] doc7 = {"id","8", "term_s","YYYY", group, "group2", "test_ti", "2", "test_tl", "100000", "test_tf", "200", "type_s", "child"}; assertU(adoc(doc7)); assertU(commit()); @@ -76,7 +78,7 @@ public class TestExpandComponent extends SolrTestCaseJ4 { //First basic test case. ModifiableSolrParams params = new ModifiableSolrParams(); params.add("q", "*:*"); - params.add("fq", "{!collapse field=group_s}"); + params.add("fq", "{!collapse field="+group+"}"); params.add("defType", "edismax"); params.add("bf", "field(test_ti)"); params.add("expand", "true"); @@ -94,7 +96,7 @@ public class TestExpandComponent extends SolrTestCaseJ4 { params = new ModifiableSolrParams(); params.add("q", "*:*"); - params.add("fq", "{!collapse field=group_s}"); + params.add("fq", "{!collapse field="+group+"}"); params.add("defType", "edismax"); params.add("bf", "field(test_ti)"); params.add("expand", "true"); @@ -110,7 +112,7 @@ public class TestExpandComponent extends SolrTestCaseJ4 { //Test expand.sort params = new ModifiableSolrParams(); params.add("q", "*:*"); - params.add("fq", "{!collapse field=group_s}"); + params.add("fq", "{!collapse field="+group+"}"); params.add("defType", "edismax"); params.add("bf", "field(test_ti)"); params.add("expand", "true"); @@ -129,7 +131,7 @@ public class TestExpandComponent extends SolrTestCaseJ4 { //Main result set should include the doc with null value in the collapse field. params = new ModifiableSolrParams(); params.add("q", "*:*"); - params.add("fq", "{!collapse field=group_s nullPolicy=collapse}"); + params.add("fq", "{!collapse field="+group+" nullPolicy=collapse}"); params.add("defType", "edismax"); params.add("bf", "field(test_ti)"); params.add("expand", "true"); @@ -154,7 +156,7 @@ public class TestExpandComponent extends SolrTestCaseJ4 { params.add("bf", "field(test_ti)"); params.add("expand", "true"); params.add("expand.q", "type_s:child"); - params.add("expand.field", "group_s"); + params.add("expand.field", group); params.add("expand.sort", "test_tl desc"); assertQ(req(params), "*[count(/response/result/doc)=2]", "*[count(/response/lst[@name='expanded']/result)=2]", @@ -176,7 +178,7 @@ public class TestExpandComponent extends SolrTestCaseJ4 { params.add("bf", "field(test_ti)"); params.add("expand", "true"); params.add("expand.fq", "type_s:child"); - params.add("expand.field", "group_s"); + params.add("expand.field", group); params.add("expand.sort", "test_tl desc"); assertQ(req(params), "*[count(/response/result/doc)=2]", "*[count(/response/lst[@name='expanded']/result)=2]", @@ -198,7 +200,7 @@ public class TestExpandComponent extends SolrTestCaseJ4 { params.add("expand", "true"); params.add("expand.q", "type_s:child"); params.add("expand.fq", "*:*"); - params.add("expand.field", "group_s"); + params.add("expand.field", group); params.add("expand.sort", "test_tl desc"); assertQ(req(params), "*[count(/response/result/doc)=2]", "*[count(/response/lst[@name='expanded']/result)=2]", @@ -214,7 +216,7 @@ public class TestExpandComponent extends SolrTestCaseJ4 { params = new ModifiableSolrParams(); params.add("q", "*:*"); - params.add("fq", "{!collapse field=group_s}"); + params.add("fq", "{!collapse field="+group+"}"); params.add("defType", "edismax"); params.add("bf", "field(test_ti)"); params.add("expand", "true"); @@ -235,7 +237,7 @@ public class TestExpandComponent extends SolrTestCaseJ4 { params = new ModifiableSolrParams(); params.add("q", "test_ti:5"); - params.add("fq", "{!collapse field=group_s}"); + params.add("fq", "{!collapse field="+group+"}"); params.add("defType", "edismax"); params.add("bf", "field(test_ti)"); params.add("expand", "true"); @@ -249,7 +251,7 @@ public class TestExpandComponent extends SolrTestCaseJ4 { params = new ModifiableSolrParams(); params.add("q", "test_ti:5532535"); - params.add("fq", "{!collapse field=group_s}"); + params.add("fq", "{!collapse field="+group+"}"); params.add("defType", "edismax"); params.add("bf", "field(test_ti)"); params.add("expand", "true"); @@ -258,6 +260,25 @@ public class TestExpandComponent extends SolrTestCaseJ4 { assertQ(req(params), "*[count(/response/result/doc)=0]", "*[count(/response/lst[@name='expanded']/result)=0]" ); + + //Test key-only fl + + params = new ModifiableSolrParams(); + params.add("q", "*:*"); + params.add("fq", "{!collapse field="+group+"}"); + params.add("defType", "edismax"); + params.add("bf", "field(test_ti)"); + params.add("expand", "true"); + params.add("fl", "id"); + assertQ(req(params), "*[count(/response/result/doc)=2]", + "*[count(/response/lst[@name='expanded']/result)=2]", + "/response/result/doc[1]/float[@name='id'][.='2.0']", + "/response/result/doc[2]/float[@name='id'][.='6.0']", + "/response/lst[@name='expanded']/result[@name='group1']/doc[1]/float[@name='id'][.='1.0']", + "/response/lst[@name='expanded']/result[@name='group1']/doc[2]/float[@name='id'][.='7.0']", + "/response/lst[@name='expanded']/result[@name='group2']/doc[1]/float[@name='id'][.='5.0']", + "/response/lst[@name='expanded']/result[@name='group2']/doc[2]/float[@name='id'][.='8.0']" + ); } }