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: klaas@apache.org
: 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
|