diff --git a/CHANGES.txt b/CHANGES.txt index 6a902364e2f..1e23d0d6073 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -147,6 +147,10 @@ Changes in runtime behavior 5. Query are re-written before highlighting is performed. This enables proper highlighting of prefix and wildcard queries (klaas). + 6. A meaningful exception is raised when attempting to add a doc missing + a unique id if it is declared in the schema and allowDups=false. + (ryan via klaas) + Optimizations 1. SOLR-114: HashDocSet specific implementations of union() and andNot() for a 20x performance improvement for those set operations, and a new diff --git a/src/java/org/apache/solr/update/DirectUpdateHandler.java b/src/java/org/apache/solr/update/DirectUpdateHandler.java index b34d39eab7b..8db3285acff 100644 --- a/src/java/org/apache/solr/update/DirectUpdateHandler.java +++ b/src/java/org/apache/solr/update/DirectUpdateHandler.java @@ -310,6 +310,14 @@ public class DirectUpdateHandler extends UpdateHandler { public int addDoc(AddUpdateCommand cmd) throws IOException { + + // if there is no ID field, use allowDups + if( idField == null ) { + cmd.allowDups = true; + cmd.overwriteCommitted = false; + cmd.overwritePending = false; + } + if (!cmd.allowDups && !cmd.overwritePending && !cmd.overwriteCommitted) { return addNoOverwriteNoDups(cmd); } else if (!cmd.allowDups && !cmd.overwritePending && cmd.overwriteCommitted) { diff --git a/src/java/org/apache/solr/update/DirectUpdateHandler2.java b/src/java/org/apache/solr/update/DirectUpdateHandler2.java index 3b4e0e027e3..276dfe64a92 100644 --- a/src/java/org/apache/solr/update/DirectUpdateHandler2.java +++ b/src/java/org/apache/solr/update/DirectUpdateHandler2.java @@ -214,6 +214,13 @@ public class DirectUpdateHandler2 extends UpdateHandler { addCommands.incrementAndGet(); addCommandsCumulative.incrementAndGet(); int rc=-1; + + // if there is no ID field, use allowDups + if( idField == null ) { + cmd.allowDups = true; + cmd.overwriteCommitted = false; + cmd.overwritePending = false; + } iwAccess.lock(); try { diff --git a/src/java/org/apache/solr/update/UpdateHandler.java b/src/java/org/apache/solr/update/UpdateHandler.java index 1a783d9de65..b3c66a74892 100644 --- a/src/java/org/apache/solr/update/UpdateHandler.java +++ b/src/java/org/apache/solr/update/UpdateHandler.java @@ -21,6 +21,7 @@ package org.apache.solr.update; import org.apache.lucene.index.Term; import org.apache.lucene.document.Document; import org.apache.lucene.document.Field; +import org.apache.lucene.document.Fieldable; import org.apache.lucene.search.HitCollector; import org.w3c.dom.NodeList; import org.w3c.dom.Node; @@ -127,13 +128,18 @@ public abstract class UpdateHandler implements SolrInfoMBean { } protected final String getIndexedId(Document doc) { - if (idField == null) throw new SolrException(400,"Operation requires schema to have a unique key field"); + if (idField == null) + throw new SolrException(400,"Operation requires schema to have a unique key field"); + // Right now, single valued fields that require value transformation from external to internal (indexed) // form have that transformation already performed and stored as the field value. - // This means - String id = idFieldType.storedToIndexed(doc.getField(idField.getName())); - if (id == null) throw new SolrException(400,"Document is missing uniqueKey field " + idField.getName()); - return id; + Fieldable[] id = doc.getFieldables( idField.getName() ); + if (id == null || id.length < 1) + throw new SolrException(400,"Document is missing uniqueKey field " + idField.getName()); + if( id.length > 1 ) + throw new SolrException(400,"Document specifies multiple unique ids! " + idField.getName()); + + return idFieldType.storedToIndexed( id[0] ); } protected final String getIndexedIdOptional(Document doc) { diff --git a/src/test/org/apache/solr/ConvertedLegacyTest.java b/src/test/org/apache/solr/ConvertedLegacyTest.java index 367adc6fd0c..3640f98b7ef 100644 --- a/src/test/org/apache/solr/ConvertedLegacyTest.java +++ b/src/test/org/apache/solr/ConvertedLegacyTest.java @@ -732,47 +732,47 @@ public class ConvertedLegacyTest extends AbstractSolrTestCase { // test sorting with some docs missing the sort field assertU("id_i:[1000 TO 1010]"); - assertU("10001Z"); - assertU("100110A"); - assertU("10021100"); - assertU("1003-1"); - assertU("100415"); - assertU("1005150"); - assertU("10060"); + assertU("10001Z"); + assertU("100110A"); + assertU("10021100"); + assertU("1003-1"); + assertU("100415"); + assertU("1005150"); + assertU("10060"); assertU(""); - assertQ(req("id_i:[1000 TO 1010]") + assertQ(req("id:[1000 TO 1010]") ,"*[count(//doc)=7]" ); - assertQ(req("id_i:[1000 TO 1010]; b_i asc") + assertQ(req("id:[1000 TO 1010]; b_i asc") ,"*[count(//doc)=7] " ,"//doc[1]/int[.='50'] " ,"//doc[2]/int[.='100']" ); - assertQ(req("id_i:[1000 TO 1010]; b_i desc") + assertQ(req("id:[1000 TO 1010]; b_i desc") ,"*[count(//doc)=7] " ,"//doc[1]/int[.='100'] " ,"//doc[2]/int[.='50']" ); - assertQ(req("id_i:[1000 TO 1010]; a_i asc,b_i desc") + assertQ(req("id:[1000 TO 1010]; a_i asc,b_i desc") ,"*[count(//doc)=7] " ,"//doc[3]/int[.='100'] " ,"//doc[4]/int[.='50'] " ,"//doc[5]/int[.='1000']" ); - assertQ(req("id_i:[1000 TO 1010]; a_i asc,b_i asc") + assertQ(req("id:[1000 TO 1010]; a_i asc,b_i asc") ,"*[count(//doc)=7] " ,"//doc[3]/int[.='50'] " ,"//doc[4]/int[.='100'] " ,"//doc[5]/int[.='1000']" ); // nullfirst tests - assertQ(req("id_i:[1000 TO 1002]; nullfirst asc") + assertQ(req("id:[1000 TO 1002]; nullfirst asc") ,"*[count(//doc)=3] " ,"//doc[1]/int[.='1002']" ,"//doc[2]/int[.='1001'] " ,"//doc[3]/int[.='1000']" ); - assertQ(req("id_i:[1000 TO 1002]; nullfirst desc") + assertQ(req("id:[1000 TO 1002]; nullfirst desc") ,"*[count(//doc)=3] " ,"//doc[1]/int[.='1002']" ,"//doc[2]/int[.='1000'] " @@ -781,16 +781,16 @@ public class ConvertedLegacyTest extends AbstractSolrTestCase { // Sort parsing exception tests. (SOLR-6, SOLR-99) assertQEx( "can not sort unindexed fields", - req( "id_i:1000; shouldbeunindexed asc" ), 400 ); + req( "id:1000; shouldbeunindexed asc" ), 400 ); assertQEx( "invalid query format", - req( "id_i:1000; nullfirst" ), 400 ); + req( "id:1000; nullfirst" ), 400 ); assertQEx( "unknown sort field", - req( "id_i:1000; abcde12345 asc" ), 1 ); + req( "id:1000; abcde12345 asc" ), 1 ); assertQEx( "unknown sort order", - req( "id_i:1000; nullfirst aaa" ), 400 ); + req( "id:1000; nullfirst aaa" ), 400 ); // test prefix query diff --git a/src/test/org/apache/solr/update/DirectUpdateHandlerTest.java b/src/test/org/apache/solr/update/DirectUpdateHandlerTest.java new file mode 100644 index 00000000000..cc3ef38f36c --- /dev/null +++ b/src/test/org/apache/solr/update/DirectUpdateHandlerTest.java @@ -0,0 +1,91 @@ +/** + * 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.solr.update; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; + +import org.apache.lucene.document.Document; +import org.apache.lucene.document.Field; +import org.apache.lucene.document.Field.Index; +import org.apache.lucene.document.Field.Store; +import org.apache.solr.core.SolrCore; +import org.apache.solr.core.SolrException; +import org.apache.solr.handler.XmlUpdateRequestHandler; +import org.apache.solr.request.ContentStream; +import org.apache.solr.request.MapSolrParams; +import org.apache.solr.request.SolrQueryRequestBase; +import org.apache.solr.request.SolrQueryResponse; +import org.apache.solr.util.AbstractSolrTestCase; + +/** + * + * @author ryan + * + */ +public class DirectUpdateHandlerTest extends AbstractSolrTestCase { + + public String getSchemaFile() { return "schema.xml"; } + public String getSolrConfigFile() { return "solrconfig.xml"; } + + + public void testRequireUniqueKey() throws Exception + { + SolrCore core = SolrCore.getSolrCore(); + + UpdateHandler updater = core.getUpdateHandler(); + + AddUpdateCommand cmd = new AddUpdateCommand(); + cmd.overwriteCommitted = true; + cmd.overwritePending = true; + cmd.allowDups = false; + + // Add a valid document + cmd.doc = new Document(); + cmd.doc.add( new Field( "id", "AAA", Store.YES, Index.UN_TOKENIZED ) ); + cmd.doc.add( new Field( "subject", "xxxxx", Store.YES, Index.UN_TOKENIZED ) ); + updater.addDoc( cmd ); + + // Add a document with multiple ids + cmd.indexedId = null; // reset the id for this add + cmd.doc = new Document(); + cmd.doc.add( new Field( "id", "AAA", Store.YES, Index.UN_TOKENIZED ) ); + cmd.doc.add( new Field( "id", "BBB", Store.YES, Index.UN_TOKENIZED ) ); + cmd.doc.add( new Field( "subject", "xxxxx", Store.YES, Index.UN_TOKENIZED ) ); + try { + updater.addDoc( cmd ); + fail( "added a document with multiple ids" ); + } + catch( SolrException ex ) { } // expected + + // Add a document without an id + cmd.indexedId = null; // reset the id for this add + cmd.doc = new Document(); + cmd.doc.add( new Field( "subject", "xxxxx", Store.YES, Index.UN_TOKENIZED ) ); + try { + updater.addDoc( cmd ); + fail( "added a document without an ids" ); + } + catch( SolrException ex ) { } // expected + } + +}