SOLR-3241: document/field boosts would fail if you copyField from a norms field to an omitNorms field

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1300639 13f79535-47bb-0310-9956-ffa450edef68

lucene/CHANGES.txt

@@ -780,10 +780,10 @@ Changes in backwards compatibility policy
   
 Changes in Runtime Behavior
 
-* LUCENE-3796: Throw an exception if you try to set an index-time
+* LUCENE-3796, SOLR-3241: Throw an exception if you try to set an index-time
   boost on a field that omits norms. Because the index-time boost
   is multiplied into the norm, previously your boost would be 
-  silently discarded.  (Robert Muir)
+  silently discarded.  (Tomás Fernández Löbbe, Hoss Man, Robert Muir)
   
 Security fixes
 

solr/core/src/java/org/apache/solr/schema/CurrencyField.java

@@ -141,11 +141,14 @@ public class CurrencyField extends FieldType implements SchemaAware, ResourceLoa
     CurrencyValue value = CurrencyValue.parse(externalVal.toString(), defaultCurrency);
 
     IndexableField[] f = new IndexableField[field.stored() ? 3 : 2];
-    f[0] = getAmountField(field).createField(String.valueOf(value.getAmount()), boost);
-    f[1] = getCurrencyField(field).createField(value.getCurrencyCode(), boost);
+    SchemaField amountField = getAmountField(field);
+    f[0] = amountField.createField(String.valueOf(value.getAmount()), amountField.omitNorms() ? 1F : boost);
+    SchemaField currencyField = getCurrencyField(field);
+    f[1] = currencyField.createField(value.getCurrencyCode(), currencyField.omitNorms() ? 1F : boost);
 
     if (field.stored()) {
       org.apache.lucene.document.FieldType customType = new org.apache.lucene.document.FieldType();
+      assert !customType.omitNorms();
       customType.setStored(true);
       String storedValue = externalVal.toString().trim();
       if (storedValue.indexOf(",") < 0) {

solr/core/src/java/org/apache/solr/schema/LatLonType.java

@@ -73,10 +73,12 @@ public class LatLonType extends AbstractSubTypeFieldType implements SpatialQuery
         throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, e);
       }
       //latitude
-      f[i] = subField(field, i).createField(String.valueOf(latLon[LAT]), boost);
+      SchemaField lat = subField(field, i);
+      f[i] = lat.createField(String.valueOf(latLon[LAT]), lat.omitNorms() ? 1F : boost);
       i++;
       //longitude
-      f[i] = subField(field, i).createField(String.valueOf(latLon[LON]), boost);
+      SchemaField lon = subField(field, i);
+      f[i] = lon.createField(String.valueOf(latLon[LON]), lon.omitNorms() ? 1F : boost);
 
     }
 

solr/core/src/java/org/apache/solr/update/DocumentBuilder.java

@@ -282,13 +282,7 @@ public class DocumentBuilder {
             if( val instanceof String && cf.getMaxChars() > 0 ) {
               val = cf.getLimitedValue((String)val);
             }
-            
-            IndexableField [] fields = destinationField.createFields(val, omitNorms ? 1F : docBoost*boost);
-            if (fields != null) { // null fields are not added
-              for (IndexableField f : fields) {
-                if(f != null) out.add(f);
-              }
-            }
+            addField(out, destinationField, val, destinationField.omitNorms() ? 1F : docBoost*boost);
           }
           
           // In lucene, the boost for a given field is the product of the 

solr/core/src/test-files/solr/conf/schema.xml

@@ -431,6 +431,9 @@
       <tokenizer class="solr.MockTokenizerFactory"/>
     </analyzer>
   </fieldType>
+  
+  <fieldType name="location" class="solr.LatLonType" subFieldSuffix="_coordinate"/>
+  
  </types>
 
 
@@ -573,6 +576,10 @@
    <field name="tlong" type="tlong" indexed="true" stored="true" />
    
    <field name="_version_" type="long" indexed="true" stored="true" multiValued="false"/>
+   
+   <field name="title_stringNoNorms" type="string" omitNorms="true" indexed="true" stored="true"/>
+   
+   <field name="store" type="location" indexed="true" stored="true" omitNorms="false"/>
 
    <!-- Dynamic field definitions.  If a field name is not found, dynamicFields
         will be used if the name matches any of the patterns.
@@ -659,6 +666,9 @@
    <dynamicField name="*_sim1" type="sim1" indexed="true" stored="true"/>
    <dynamicField name="*_sim2" type="sim2" indexed="true" stored="true"/>
    <dynamicField name="*_sim3" type="sim3" indexed="true" stored="true"/>
+   
+   <!-- Type used to index the lat and lon components for the "location" FieldType -->
+   <dynamicField name="*_coordinate"  type="tdouble" indexed="true"  stored="false" omitNorms="true" />
  </fields>
 
  <defaultSearchField>text</defaultSearchField>
@@ -670,6 +680,7 @@
    -->
    <copyField source="title" dest="title_stemmed"/>
    <copyField source="title" dest="title_lettertok"/>
+   <copyField source="title" dest="title_stringNoNorms"/>
 
    <copyField source="title" dest="text"/>
 	 <copyField source="subject" dest="text"/>

solr/core/src/test/org/apache/solr/update/DocumentBuilderTest.java

@@ -23,6 +23,7 @@ import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrInputDocument;
 import org.apache.solr.core.SolrCore;
 import org.apache.solr.schema.FieldType;
+import org.apache.solr.schema.IndexSchema;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
@@ -112,5 +113,126 @@ public class DocumentBuilderTest extends SolrTestCaseJ4 {
     assertNotNull( out.getField( "home_0" + FieldType.POLY_FIELD_SEPARATOR + "double" ) );
     assertNotNull( out.getField( "home_1" + FieldType.POLY_FIELD_SEPARATOR + "double" ) );
   }
+  
+  @Test
+  public void testCopyFieldWithDocumentBoost() {
+    SolrCore core = h.getCore();
+    IndexSchema schema = core.getSchema();
+    assertFalse(schema.getField("title").omitNorms());
+    assertTrue(schema.getField("title_stringNoNorms").omitNorms());
+    SolrInputDocument doc = new SolrInputDocument();
+    doc.setDocumentBoost(3f);
+    doc.addField( "title", "mytitle");
+    Document out = DocumentBuilder.toDocument( doc, core.getSchema() );
+    assertNotNull( out.get( "title_stringNoNorms" ) );
+    assertTrue("title_stringNoNorms has the omitNorms attribute set to true, if the boost is different than 1.0, it will fail",1.0f == out.getField( "title_stringNoNorms" ).boost() );
+    assertTrue("It is OK that title has a boost of 3",3.0f == out.getField( "title" ).boost() );
+  }
+  
+  
+  @Test
+  public void testCopyFieldWithFieldBoost() {
+    SolrCore core = h.getCore();
+    IndexSchema schema = core.getSchema();
+    assertFalse(schema.getField("title").omitNorms());
+    assertTrue(schema.getField("title_stringNoNorms").omitNorms());
+    SolrInputDocument doc = new SolrInputDocument();
+    doc.addField( "title", "mytitle", 3.0f );
+    Document out = DocumentBuilder.toDocument( doc, core.getSchema() );
+    assertNotNull( out.get( "title_stringNoNorms" ) );
+    assertTrue("title_stringNoNorms has the omitNorms attribute set to true, if the boost is different than 1.0, it will fail",1.0f == out.getField( "title_stringNoNorms" ).boost() );
+    assertTrue("It is OK that title has a boost of 3",3.0f == out.getField( "title" ).boost() );
+  }
+  
+  @Test
+  public void testWithPolyFieldsAndFieldBoost() {
+    SolrCore core = h.getCore();
+    IndexSchema schema = core.getSchema();
+    assertFalse(schema.getField("store").omitNorms());
+    assertTrue(schema.getField("store_0_coordinate").omitNorms());
+    assertTrue(schema.getField("store_1_coordinate").omitNorms());
+    assertFalse(schema.getField("amount").omitNorms());
+    assertTrue(schema.getField("amount" + FieldType.POLY_FIELD_SEPARATOR + "_currency").omitNorms());
+    assertTrue(schema.getField("amount" + FieldType.POLY_FIELD_SEPARATOR + "_amount_raw").omitNorms());
+    
+    SolrInputDocument doc = new SolrInputDocument();
+    doc.addField( "store", "40.7143,-74.006", 3.0f );
+    doc.addField( "amount", "10.5", 3.0f );
+    Document out = DocumentBuilder.toDocument( doc, core.getSchema() );
+    assertNotNull( out.get( "store" ) );
+    assertNotNull( out.get( "amount" ) );
+    assertNotNull(out.getField("store_0_coordinate"));
+    //NOTE: As the subtypes have omitNorm=true, they must have boost=1F, otherwise this is going to fail when adding the doc to Lucene.
+    assertTrue(1f == out.getField("store_0_coordinate").boost());
+    assertTrue(1f == out.getField("store_1_coordinate").boost());
+    assertTrue(1f == out.getField("amount" + FieldType.POLY_FIELD_SEPARATOR + "_currency").boost());
+    assertTrue(1f == out.getField("amount" + FieldType.POLY_FIELD_SEPARATOR + "_amount_raw").boost());
+  }
+  
+  @Test
+  public void testWithPolyFieldsAndDocumentBoost() {
+    SolrCore core = h.getCore();
+    IndexSchema schema = core.getSchema();
+    assertFalse(schema.getField("store").omitNorms());
+    assertTrue(schema.getField("store_0_coordinate").omitNorms());
+    assertTrue(schema.getField("store_1_coordinate").omitNorms());
+    assertFalse(schema.getField("amount").omitNorms());
+    assertTrue(schema.getField("amount" + FieldType.POLY_FIELD_SEPARATOR + "_currency").omitNorms());
+    assertTrue(schema.getField("amount" + FieldType.POLY_FIELD_SEPARATOR + "_amount_raw").omitNorms());
+    
+    SolrInputDocument doc = new SolrInputDocument();
+    doc.setDocumentBoost(3.0f);
+    doc.addField( "store", "40.7143,-74.006");
+    doc.addField( "amount", "10.5");
+    Document out = DocumentBuilder.toDocument( doc, core.getSchema() );
+    assertNotNull( out.get( "store" ) );
+    assertNotNull(out.getField("store_0_coordinate"));
+    //NOTE: As the subtypes have omitNorm=true, they must have boost=1F, otherwise this is going to fail when adding the doc to Lucene.
+    assertTrue(1f == out.getField("store_0_coordinate").boost());
+    assertTrue(1f == out.getField("store_1_coordinate").boost());
+    assertTrue(1f == out.getField("amount" + FieldType.POLY_FIELD_SEPARATOR + "_currency").boost());
+    assertTrue(1f == out.getField("amount" + FieldType.POLY_FIELD_SEPARATOR + "_amount_raw").boost());
+  }
+  
+  /**
+   * Its ok to boost a field if it has norms
+   */
+  public void testBoost() throws Exception {
+    XmlDoc xml = new XmlDoc();
+    xml.xml = "<doc>"
+        + "<field name=\"id\">0</field>"
+        + "<field name=\"title\" boost=\"3.0\">mytitle</field>"
+        + "</doc>";
+    assertNull(h.validateUpdate(add(xml, new String[0])));
+  }
+  
+  /**
+   * Its not ok to boost a field if it omits norms
+   */
+  public void testBoostOmitNorms() throws Exception {
+    XmlDoc xml = new XmlDoc();
+    xml.xml = "<doc>"
+        + "<field name=\"id\">1</field>"
+        + "<field name=\"title_stringNoNorms\" boost=\"3.0\">mytitle</field>"
+        + "</doc>";
+    try {
+      assertNull(h.validateUpdate(add(xml, new String[0])));
+      fail("didn't get expected exception for boosting omit norms field");
+    } catch (SolrException expected) {
+      // expected exception
+    }
+  }
+  
+  /**
+   * Its ok to supply a document boost even if a field omits norms
+   */
+  public void testDocumentBoostOmitNorms() throws Exception {
+    XmlDoc xml = new XmlDoc();
+    xml.xml = "<doc boost=\"3.0\">"
+        + "<field name=\"id\">2</field>"
+        + "<field name=\"title_stringNoNorms\">mytitle</field>"
+        + "</doc>";
+    assertNull(h.validateUpdate(add(xml, new String[0])));
+  }
 
 }