From 14c88807292244d13bc954dd22b150cdb053b59e Mon Sep 17 00:00:00 2001 From: Erick Erickson Date: Mon, 5 Jan 2015 20:50:39 +0000 Subject: [PATCH] SOLR-6666: Dynamic copy fields are considering all dynamic fields, causing a significant performance impact on indexing documents git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1649657 13f79535-47bb-0310-9956-ffa450edef68 --- solr/CHANGES.txt | 4 ++ .../org/apache/solr/schema/IndexSchema.java | 40 ++++++++++++------- .../TestCopyFieldCollectionResource.java | 29 +++++++------- .../solr/rest/schema/TestSchemaResource.java | 20 +++++----- 4 files changed, 54 insertions(+), 39 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 5a836721ab1..c4403c99615 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -419,6 +419,10 @@ Optimizations hl.usePhraseHighlighter, and can be more efficient handling data from term vectors. (David Smiley) +* SOLR-6666: Dynamic copy fields are considering all dynamic fields, causing + a significant performance impact on indexing documents. (Liram Vardi via Erick + Erickson, Steve Rowe) + Other Changes ---------------------- diff --git a/solr/core/src/java/org/apache/solr/schema/IndexSchema.java b/solr/core/src/java/org/apache/solr/schema/IndexSchema.java index 30da7ab1232..2f8b764a357 100644 --- a/solr/core/src/java/org/apache/solr/schema/IndexSchema.java +++ b/solr/core/src/java/org/apache/solr/schema/IndexSchema.java @@ -901,37 +901,49 @@ public class IndexSchema { String msg = "copyField dest :'" + dest + "' is not an explicit field and doesn't match a dynamicField."; throw new SolrException(ErrorCode.SERVER_ERROR, msg); } - if (sourceIsDynamicFieldReference || sourceIsGlob) { - if (null != destDynamicField) { // source: glob or no-asterisk dynamic field ref; dest: dynamic field ref + if (sourceIsGlob) { + if (null != destDynamicField) { // source: glob ; dest: dynamic field ref registerDynamicCopyField(new DynamicCopy(source, destDynamicField, maxChars, sourceDynamicBase, destDynamicBase)); incrementCopyFieldTargetCount(destSchemaField); - } else { // source: glob or no-asterisk dynamic field ref; dest: explicit field + } else { // source: glob ; dest: explicit field destDynamicField = new DynamicField(destSchemaField); registerDynamicCopyField(new DynamicCopy(source, destDynamicField, maxChars, sourceDynamicBase, null)); incrementCopyFieldTargetCount(destSchemaField); } - } else { - if (null != destDynamicField) { // source: explicit field; dest: dynamic field reference + } else if (sourceIsDynamicFieldReference) { + if (null != destDynamicField) { // source: no-asterisk dynamic field ref ; dest: dynamic field ref + registerDynamicCopyField(new DynamicCopy(source, destDynamicField, maxChars, sourceDynamicBase, destDynamicBase)); + incrementCopyFieldTargetCount(destSchemaField); + } else { // source: no-asterisk dynamic field ref ; dest: explicit field + sourceSchemaField = getField(source); + registerExplicitSrcAndDestFields(source, maxChars, destSchemaField, sourceSchemaField); + } + } else { + if (null != destDynamicField) { // source: explicit field ; dest: dynamic field reference if (destDynamicField.pattern instanceof DynamicReplacement.DynamicPattern.NameEquals) { // Dynamic dest with no asterisk is acceptable registerDynamicCopyField(new DynamicCopy(source, destDynamicField, maxChars, sourceDynamicBase, destDynamicBase)); incrementCopyFieldTargetCount(destSchemaField); - } else { + } else { // source: explicit field ; dest: dynamic field with an asterisk String msg = "copyField only supports a dynamic destination with an asterisk " + "if the source also has an asterisk"; throw new SolrException(ErrorCode.SERVER_ERROR, msg); } - } else { // source & dest: explicit fields - List copyFieldList = copyFieldsMap.get(source); - if (copyFieldList == null) { - copyFieldList = new ArrayList<>(); - copyFieldsMap.put(source, copyFieldList); - } - copyFieldList.add(new CopyField(sourceSchemaField, destSchemaField, maxChars)); - incrementCopyFieldTargetCount(destSchemaField); + } else { // source & dest: explicit fields + registerExplicitSrcAndDestFields(source, maxChars, destSchemaField, sourceSchemaField); } } } + + private void registerExplicitSrcAndDestFields(String source, int maxChars, SchemaField destSchemaField, SchemaField sourceSchemaField) { + List copyFieldList = copyFieldsMap.get(source); + if (copyFieldList == null) { + copyFieldList = new ArrayList<>(); + copyFieldsMap.put(source, copyFieldList); + } + copyFieldList.add(new CopyField(sourceSchemaField, destSchemaField, maxChars)); + incrementCopyFieldTargetCount(destSchemaField); + } private void incrementCopyFieldTargetCount(SchemaField dest) { copyFieldTargetCounts.put(dest, copyFieldTargetCounts.containsKey(dest) ? copyFieldTargetCounts.get(dest) + 1 : 1); diff --git a/solr/core/src/test/org/apache/solr/rest/schema/TestCopyFieldCollectionResource.java b/solr/core/src/test/org/apache/solr/rest/schema/TestCopyFieldCollectionResource.java index 004ee56e88f..383e2b3b1ff 100644 --- a/solr/core/src/test/org/apache/solr/rest/schema/TestCopyFieldCollectionResource.java +++ b/solr/core/src/test/org/apache/solr/rest/schema/TestCopyFieldCollectionResource.java @@ -23,7 +23,10 @@ public class TestCopyFieldCollectionResource extends SolrRestletTestBase { @Test public void testGetAllCopyFields() throws Exception { assertQ("/schema/copyfields?indent=on&wt=xml", - "/response/arr[@name='copyFields']/lst[ str[@name='source'][.='title']" + "/response/arr[@name='copyFields']/lst[ str[@name='source'][.='src_sub_no_ast_i']" + +" and str[@name='dest'][.='title']]", + + "/response/arr[@name='copyFields']/lst[ str[@name='source'][.='title']" +" and str[@name='dest'][.='title_stemmed']" +" and int[@name='maxChars'][.='200']]", @@ -65,10 +68,6 @@ public class TestCopyFieldCollectionResource extends SolrRestletTestBase { "/response/arr[@name='copyFields']/lst[ str[@name='source'][.='src_sub_no_ast_i']" +" and str[@name='sourceDynamicBase'][.='*_i']" - +" and str[@name='dest'][.='title']]", - - "/response/arr[@name='copyFields']/lst[ str[@name='source'][.='src_sub_no_ast_i']" - +" and str[@name='sourceDynamicBase'][.='*_i']" +" and str[@name='dest'][.='*_s']]", "/response/arr[@name='copyFields']/lst[ str[@name='source'][.='src_sub_no_ast_i']" @@ -105,19 +104,19 @@ public class TestCopyFieldCollectionResource extends SolrRestletTestBase { @Test public void testJsonGetAllCopyFields() throws Exception { assertJQ("/schema/copyfields?indent=on&wt=json", - "/copyFields/[6]=={'source':'title','dest':'dest_sub_no_ast_s','destDynamicBase':'*_s'}", + "/copyFields/[1]=={'source':'src_sub_no_ast_i','dest':'title'}", + "/copyFields/[7]=={'source':'title','dest':'dest_sub_no_ast_s','destDynamicBase':'*_s'}", - "/copyFields/[7]=={'source':'*_i','dest':'title'}", - "/copyFields/[8]=={'source':'*_i','dest':'*_s'}", - "/copyFields/[9]=={'source':'*_i','dest':'*_dest_sub_s','destDynamicBase':'*_s'}", - "/copyFields/[10]=={'source':'*_i','dest':'dest_sub_no_ast_s','destDynamicBase':'*_s'}", + "/copyFields/[8]=={'source':'*_i','dest':'title'}", + "/copyFields/[9]=={'source':'*_i','dest':'*_s'}", + "/copyFields/[10]=={'source':'*_i','dest':'*_dest_sub_s','destDynamicBase':'*_s'}", + "/copyFields/[11]=={'source':'*_i','dest':'dest_sub_no_ast_s','destDynamicBase':'*_s'}", - "/copyFields/[11]=={'source':'*_src_sub_i','sourceDynamicBase':'*_i','dest':'title'}", - "/copyFields/[12]=={'source':'*_src_sub_i','sourceDynamicBase':'*_i','dest':'*_s'}", - "/copyFields/[13]=={'source':'*_src_sub_i','sourceDynamicBase':'*_i','dest':'*_dest_sub_s','destDynamicBase':'*_s'}", - "/copyFields/[14]=={'source':'*_src_sub_i','sourceDynamicBase':'*_i','dest':'dest_sub_no_ast_s','destDynamicBase':'*_s'}", + "/copyFields/[12]=={'source':'*_src_sub_i','sourceDynamicBase':'*_i','dest':'title'}", + "/copyFields/[13]=={'source':'*_src_sub_i','sourceDynamicBase':'*_i','dest':'*_s'}", + "/copyFields/[14]=={'source':'*_src_sub_i','sourceDynamicBase':'*_i','dest':'*_dest_sub_s','destDynamicBase':'*_s'}", + "/copyFields/[15]=={'source':'*_src_sub_i','sourceDynamicBase':'*_i','dest':'dest_sub_no_ast_s','destDynamicBase':'*_s'}", - "/copyFields/[15]=={'source':'src_sub_no_ast_i','sourceDynamicBase':'*_i','dest':'title'}", "/copyFields/[16]=={'source':'src_sub_no_ast_i','sourceDynamicBase':'*_i','dest':'*_s'}", "/copyFields/[17]=={'source':'src_sub_no_ast_i','sourceDynamicBase':'*_i','dest':'*_dest_sub_s','destDynamicBase':'*_s'}", "/copyFields/[18]=={'source':'src_sub_no_ast_i','sourceDynamicBase':'*_i','dest':'dest_sub_no_ast_s','destDynamicBase':'*_s'}"); diff --git a/solr/core/src/test/org/apache/solr/rest/schema/TestSchemaResource.java b/solr/core/src/test/org/apache/solr/rest/schema/TestSchemaResource.java index aed5d7c4782..fb59f1f7d41 100644 --- a/solr/core/src/test/org/apache/solr/rest/schema/TestSchemaResource.java +++ b/solr/core/src/test/org/apache/solr/rest/schema/TestSchemaResource.java @@ -131,19 +131,19 @@ public class TestSchemaResource extends SolrRestletTestBase { "/schema/dynamicFields/[1]/name=='ignored_*'", "/schema/dynamicFields/[2]/name=='*_mfacet'", - "/schema/copyFields/[6]=={'source':'title','dest':'dest_sub_no_ast_s'}", + "/schema/copyFields/[1]=={'source':'src_sub_no_ast_i','dest':'title'}", - "/schema/copyFields/[7]=={'source':'*_i','dest':'title'}", - "/schema/copyFields/[8]=={'source':'*_i','dest':'*_s'}", - "/schema/copyFields/[9]=={'source':'*_i','dest':'*_dest_sub_s'}", - "/schema/copyFields/[10]=={'source':'*_i','dest':'dest_sub_no_ast_s'}", + "/schema/copyFields/[7]=={'source':'title','dest':'dest_sub_no_ast_s'}", + "/schema/copyFields/[8]=={'source':'*_i','dest':'title'}", + "/schema/copyFields/[9]=={'source':'*_i','dest':'*_s'}", + "/schema/copyFields/[10]=={'source':'*_i','dest':'*_dest_sub_s'}", + "/schema/copyFields/[11]=={'source':'*_i','dest':'dest_sub_no_ast_s'}", - "/schema/copyFields/[11]=={'source':'*_src_sub_i','dest':'title'}", - "/schema/copyFields/[12]=={'source':'*_src_sub_i','dest':'*_s'}", - "/schema/copyFields/[13]=={'source':'*_src_sub_i','dest':'*_dest_sub_s'}", - "/schema/copyFields/[14]=={'source':'*_src_sub_i','dest':'dest_sub_no_ast_s'}", + "/schema/copyFields/[12]=={'source':'*_src_sub_i','dest':'title'}", + "/schema/copyFields/[13]=={'source':'*_src_sub_i','dest':'*_s'}", + "/schema/copyFields/[14]=={'source':'*_src_sub_i','dest':'*_dest_sub_s'}", + "/schema/copyFields/[15]=={'source':'*_src_sub_i','dest':'dest_sub_no_ast_s'}", - "/schema/copyFields/[15]=={'source':'src_sub_no_ast_i','dest':'title'}", "/schema/copyFields/[16]=={'source':'src_sub_no_ast_i','dest':'*_s'}", "/schema/copyFields/[17]=={'source':'src_sub_no_ast_i','dest':'*_dest_sub_s'}", "/schema/copyFields/[18]=={'source':'src_sub_no_ast_i','dest':'dest_sub_no_ast_s'}");