Thanks for comitting Mike, sorry i never got around to reviewing the patch.
Just out of curiousity though, can anyone explain why ConvertedLegacyTest was functioning properly before this change? I'm just wondering if we're breaking a case people might have been relying on because it functioned (even if it wasn't intended to). : Date: Mon, 19 Mar 2007 20:25:20 -0000 : From: [EMAIL PROTECTED] : Reply-To: solr-dev@lucene.apache.org : To: solr-commits@lucene.apache.org : Subject: svn commit: r520088 - in /lucene/solr/trunk: ./ : src/java/org/apache/solr/update/ src/test/org/apache/solr/ : src/test/org/apache/solr/update/ : : Author: klaas : Date: Mon Mar 19 13:25:19 2007 : New Revision: 520088 : : URL: http://svn.apache.org/viewvc?view=rev&rev=520088 : Log: : commit SOLR-172 : : Added: : lucene/solr/trunk/src/test/org/apache/solr/update/DirectUpdateHandlerTest.java : Modified: : lucene/solr/trunk/CHANGES.txt : lucene/solr/trunk/src/java/org/apache/solr/update/DirectUpdateHandler.java : lucene/solr/trunk/src/java/org/apache/solr/update/DirectUpdateHandler2.java : lucene/solr/trunk/src/java/org/apache/solr/update/UpdateHandler.java : lucene/solr/trunk/src/test/org/apache/solr/ConvertedLegacyTest.java : : Modified: lucene/solr/trunk/CHANGES.txt : URL: http://svn.apache.org/viewvc/lucene/solr/trunk/CHANGES.txt?view=diff&rev=520088&r1=520087&r2=520088 : ============================================================================== : --- lucene/solr/trunk/CHANGES.txt (original) : +++ lucene/solr/trunk/CHANGES.txt Mon Mar 19 13:25:19 2007 : @@ -147,6 +147,10 @@ : 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 : : Modified: lucene/solr/trunk/src/java/org/apache/solr/update/DirectUpdateHandler.java : URL: http://svn.apache.org/viewvc/lucene/solr/trunk/src/java/org/apache/solr/update/DirectUpdateHandler.java?view=diff&rev=520088&r1=520087&r2=520088 : ============================================================================== : --- lucene/solr/trunk/src/java/org/apache/solr/update/DirectUpdateHandler.java (original) : +++ lucene/solr/trunk/src/java/org/apache/solr/update/DirectUpdateHandler.java Mon Mar 19 13:25:19 2007 : @@ -310,6 +310,14 @@ : : : 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) { : : Modified: lucene/solr/trunk/src/java/org/apache/solr/update/DirectUpdateHandler2.java : URL: http://svn.apache.org/viewvc/lucene/solr/trunk/src/java/org/apache/solr/update/DirectUpdateHandler2.java?view=diff&rev=520088&r1=520087&r2=520088 : ============================================================================== : --- lucene/solr/trunk/src/java/org/apache/solr/update/DirectUpdateHandler2.java (original) : +++ lucene/solr/trunk/src/java/org/apache/solr/update/DirectUpdateHandler2.java Mon Mar 19 13:25:19 2007 : @@ -214,6 +214,13 @@ : 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 { : : Modified: lucene/solr/trunk/src/java/org/apache/solr/update/UpdateHandler.java : URL: http://svn.apache.org/viewvc/lucene/solr/trunk/src/java/org/apache/solr/update/UpdateHandler.java?view=diff&rev=520088&r1=520087&r2=520088 : ============================================================================== : --- lucene/solr/trunk/src/java/org/apache/solr/update/UpdateHandler.java (original) : +++ lucene/solr/trunk/src/java/org/apache/solr/update/UpdateHandler.java Mon Mar 19 13:25:19 2007 : @@ -21,6 +21,7 @@ : 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 @@ : } : : 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) { : : Modified: lucene/solr/trunk/src/test/org/apache/solr/ConvertedLegacyTest.java : URL: http://svn.apache.org/viewvc/lucene/solr/trunk/src/test/org/apache/solr/ConvertedLegacyTest.java?view=diff&rev=520088&r1=520087&r2=520088 : ============================================================================== : --- lucene/solr/trunk/src/test/org/apache/solr/ConvertedLegacyTest.java (original) : +++ lucene/solr/trunk/src/test/org/apache/solr/ConvertedLegacyTest.java Mon Mar 19 13:25:19 2007 : @@ -732,47 +732,47 @@ : // test sorting with some docs missing the sort field : : assertU("<delete><query>id_i:[1000 TO 1010]</query></delete>"); : - assertU("<add allowDups=\"true\"><doc><field name=\"id_i\">1000</field><field name=\"a_i\">1</field><field name=\"nullfirst\">Z</field></doc></add>"); : - assertU("<add allowDups=\"true\"><doc><field name=\"id_i\">1001</field><field name=\"a_i\">10</field><field name=\"nullfirst\">A</field></doc></add>"); : - assertU("<add allowDups=\"true\"><doc><field name=\"id_i\">1002</field><field name=\"a_i\">1</field><field name=\"b_i\">100</field></doc></add>"); : - assertU("<add allowDups=\"true\"><doc><field name=\"id_i\">1003</field><field name=\"a_i\">-1</field></doc></add>"); : - assertU("<add allowDups=\"true\"><doc><field name=\"id_i\">1004</field><field name=\"a_i\">15</field></doc></add>"); : - assertU("<add allowDups=\"true\"><doc><field name=\"id_i\">1005</field><field name=\"a_i\">1</field><field name=\"b_i\">50</field></doc></add>"); : - assertU("<add allowDups=\"true\"><doc><field name=\"id_i\">1006</field><field name=\"a_i\">0</field></doc></add>"); : + assertU("<add allowDups=\"true\"><doc><field name=\"id\">1000</field><field name=\"a_i\">1</field><field name=\"nullfirst\">Z</field></doc></add>"); : + assertU("<add allowDups=\"true\"><doc><field name=\"id\">1001</field><field name=\"a_i\">10</field><field name=\"nullfirst\">A</field></doc></add>"); : + assertU("<add allowDups=\"true\"><doc><field name=\"id\">1002</field><field name=\"a_i\">1</field><field name=\"b_i\">100</field></doc></add>"); : + assertU("<add allowDups=\"true\"><doc><field name=\"id\">1003</field><field name=\"a_i\">-1</field></doc></add>"); : + assertU("<add allowDups=\"true\"><doc><field name=\"id\">1004</field><field name=\"a_i\">15</field></doc></add>"); : + assertU("<add allowDups=\"true\"><doc><field name=\"id\">1005</field><field name=\"a_i\">1</field><field name=\"b_i\">50</field></doc></add>"); : + assertU("<add allowDups=\"true\"><doc><field name=\"id\">1006</field><field name=\"a_i\">0</field></doc></add>"); : assertU("<commit/>"); : - 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 @@ : : // 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 : : : Added: lucene/solr/trunk/src/test/org/apache/solr/update/DirectUpdateHandlerTest.java : URL: http://svn.apache.org/viewvc/lucene/solr/trunk/src/test/org/apache/solr/update/DirectUpdateHandlerTest.java?view=auto&rev=520088 : ============================================================================== : --- lucene/solr/trunk/src/test/org/apache/solr/update/DirectUpdateHandlerTest.java (added) : +++ lucene/solr/trunk/src/test/org/apache/solr/update/DirectUpdateHandlerTest.java Mon Mar 19 13:25:19 2007 : @@ -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 : + } : + : +} : : -Hoss
