diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index ddbcb71cfd5..c0e40cc03ea 100644 --- a/lucene/CHANGES.txt +++ b/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 diff --git a/solr/core/src/java/org/apache/solr/schema/CurrencyField.java b/solr/core/src/java/org/apache/solr/schema/CurrencyField.java index 034c5703578..88b569093fd 100644 --- a/solr/core/src/java/org/apache/solr/schema/CurrencyField.java +++ b/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) { diff --git a/solr/core/src/java/org/apache/solr/schema/LatLonType.java b/solr/core/src/java/org/apache/solr/schema/LatLonType.java index 5e2c4f25ce6..112419d6b50 100644 --- a/solr/core/src/java/org/apache/solr/schema/LatLonType.java +++ b/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); } diff --git a/solr/core/src/java/org/apache/solr/update/DocumentBuilder.java b/solr/core/src/java/org/apache/solr/update/DocumentBuilder.java index a1ad8fc12fc..91bac620ad2 100644 --- a/solr/core/src/java/org/apache/solr/update/DocumentBuilder.java +++ b/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 diff --git a/solr/core/src/test-files/solr/conf/schema.xml b/solr/core/src/test-files/solr/conf/schema.xml index c6b286768c5..05f3b24eef0 100644 --- a/solr/core/src/test-files/solr/conf/schema.xml +++ b/solr/core/src/test-files/solr/conf/schema.xml @@ -431,6 +431,9 @@ + + + @@ -573,6 +576,10 @@ + + + + + text @@ -670,6 +680,7 @@ --> + diff --git a/solr/core/src/test/org/apache/solr/update/DocumentBuilderTest.java b/solr/core/src/test/org/apache/solr/update/DocumentBuilderTest.java index 4a4df13b0f7..4b784115bf0 100644 --- a/solr/core/src/test/org/apache/solr/update/DocumentBuilderTest.java +++ b/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 = "" + + "0" + + "mytitle" + + ""; + 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 = "" + + "1" + + "mytitle" + + ""; + 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 = "" + + "2" + + "mytitle" + + ""; + assertNull(h.validateUpdate(add(xml, new String[0]))); + } }