Remove support for boost in copy_to field

Currently, boosting on `copy_to` is misleading and does not work as originally specified in #4520. Instead of boosting just the terms from the origin field, it boosts the whole destination field.  If two fields copy_to a third field, one with a boost of 2 and another with a boost of 3, all the terms in the third field end up with a boost of 6.  This was not the intention.

  The alternative: to store the boost in a payload for every term, results in poor performance and inflexibility. Instead, users should either (1) query the common field AND the field that requires boosting, or (2) the multi_match query will soon be able to perform term-centric cross-field matching that will allow per-field boosting at query time (coming in 1.1).
This commit is contained in:
Igor Motov 2014-01-31 12:33:38 -05:00
parent 5448477c54
commit 90da268237
16 changed files with 44 additions and 142 deletions

View File

@ -637,33 +637,14 @@ the parameter. In the following example all values from fields `title` and `abst
}
--------------------------------------------------
Multiple fields with optional boost factors are also supported:
Multiple fields are also supported:
[source,js]
--------------------------------------------------
{
"book" : {
"properties" : {
"title" : { "type" : "string", "copy_to" : ["meta_data", "article_info^10.0"] },
}
}
--------------------------------------------------
The same mapping can be also defined using extended syntax:
[source,js]
--------------------------------------------------
{
"book" : {
"properties" : {
"title" : {
"type" : "string",
"copy_to" : [{
"field": "meta_data"
}, {
"field": "article_info"
"boost": 10.0
}
"title" : { "type" : "string", "copy_to" : ["meta_data", "article_info"] },
}
}
--------------------------------------------------

View File

@ -165,8 +165,6 @@ public class ParseContext {
private float docBoost = 1.0f;
private float copyToBoost = 1.0f;
public ParseContext(String index, @Nullable Settings indexSettings, DocumentMapperParser docMapperParser, DocumentMapper docMapper, ContentPath path) {
this.index = index;
this.indexSettings = indexSettings;
@ -227,14 +225,12 @@ public class ParseContext {
return withinNewMapper;
}
public void setWithinCopyTo(float copyToBoost) {
public void setWithinCopyTo() {
this.withinCopyTo = true;
this.copyToBoost = copyToBoost;
}
public void clearWithinCopyTo() {
this.withinCopyTo = false;
this.copyToBoost = 1.0f;
}
public boolean isWithinCopyTo() {
@ -400,14 +396,6 @@ public class ParseContext {
return externalValue;
}
public float fieldBoost(AbstractFieldMapper mapper) {
if (withinCopyTo) {
return copyToBoost;
} else {
return mapper.boost();
}
}
public float docBoost() {
return this.docBoost;
}

View File

@ -236,6 +236,7 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
multiFieldsBuilder.add(mapperBuilder);
return builder;
}
public T copyTo(CopyTo copyTo) {
this.copyTo = copyTo;
return builder;
@ -283,7 +284,7 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
this(names, boost, fieldType, docValues, indexAnalyzer, searchAnalyzer, postingsFormat, docValuesFormat, similarity,
normsLoading, fieldDataSettings, indexSettings, MultiFields.empty(), null);
}
protected AbstractFieldMapper(Names names, float boost, FieldType fieldType, Boolean docValues, NamedAnalyzer indexAnalyzer,
NamedAnalyzer searchAnalyzer, PostingsFormatProvider postingsFormat,
DocValuesFormatProvider docValuesFormat, SimilarityProvider similarity,
@ -996,9 +997,9 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
*/
public static class CopyTo {
private final ImmutableList<CopyToField> copyToFields;
private final ImmutableList<String> copyToFields;
private CopyTo(ImmutableList<CopyToField> copyToFields) {
private CopyTo(ImmutableList<String> copyToFields) {
this.copyToFields = copyToFields;
}
@ -1007,8 +1008,8 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
*/
public void parse(ParseContext context) throws IOException {
if (!context.isWithinCopyTo()) {
for (CopyToField field : copyToFields) {
field.parse(context);
for (String field : copyToFields) {
parse(field, context);
}
}
}
@ -1016,8 +1017,8 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
if (!copyToFields.isEmpty()) {
builder.startArray("copy_to");
for (CopyToField field : copyToFields) {
field.toXContent(builder, params);
for (String field : copyToFields) {
builder.value(field);
}
builder.endArray();
}
@ -1025,10 +1026,10 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
}
public static class Builder {
private final ImmutableList.Builder<CopyToField> copyToBuilders = ImmutableList.builder();
private final ImmutableList.Builder<String> copyToBuilders = ImmutableList.builder();
public Builder add(String field, float boost) {
copyToBuilders.add(new CopyToField(field, boost));
public Builder add(String field) {
copyToBuilders.add(field);
return this;
}
@ -1037,28 +1038,15 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
}
}
public ImmutableList<CopyToField> copyToFields() {
public ImmutableList<String> copyToFields() {
return copyToFields;
}
}
public static class CopyToField {
private final String field;
protected final float boost;
public CopyToField(String field, float boost) {
this.field = field;
this.boost = boost;
}
/**
* Creates an copy of the current field with given field name and boost
*/
public void parse(ParseContext context) throws IOException {
context.setWithinCopyTo(boost);
public void parse(String field, ParseContext context) throws IOException {
context.setWithinCopyTo();
FieldMappers mappers = context.docMapper().mappers().indexName(field);
if (mappers != null && !mappers.isEmpty()) {
mappers.mapper().parse(context);
@ -1115,23 +1103,7 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
context.clearWithinCopyTo();
}
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
builder.field("field", field);
if (boost != 1.0 || params.paramAsBoolean("include_defaults", false)) {
builder.field("boost", boost);
}
builder.endObject();
return builder;
}
public String field() {
return field;
}
public float boost() {
return boost;
}
}
}

View File

@ -249,7 +249,7 @@ public class ByteFieldMapper extends NumberFieldMapper<Byte> {
@Override
protected void innerParseCreateField(ParseContext context, List<Field> fields) throws IOException {
byte value;
float boost = context.fieldBoost(this);
float boost = this.boost;
if (context.externalValueSet()) {
Object externalValue = context.externalValue();
if (externalValue == null) {

View File

@ -458,7 +458,7 @@ public class DateFieldMapper extends NumberFieldMapper<Long> {
protected void innerParseCreateField(ParseContext context, List<Field> fields) throws IOException {
String dateAsString = null;
Long value = null;
float boost = context.fieldBoost(this);
float boost = this.boost;
if (context.externalValueSet()) {
Object externalValue = context.externalValue();
if (externalValue instanceof Number) {

View File

@ -244,7 +244,7 @@ public class DoubleFieldMapper extends NumberFieldMapper<Double> {
@Override
protected void innerParseCreateField(ParseContext context, List<Field> fields) throws IOException {
double value;
float boost = context.fieldBoost(this);
float boost = this.boost;
if (context.externalValueSet()) {
Object externalValue = context.externalValue();
if (externalValue == null) {

View File

@ -249,7 +249,7 @@ public class FloatFieldMapper extends NumberFieldMapper<Float> {
@Override
protected void innerParseCreateField(ParseContext context, List<Field> fields) throws IOException {
float value;
float boost = context.fieldBoost(this);
float boost = this.boost;
if (context.externalValueSet()) {
Object externalValue = context.externalValue();
if (externalValue == null) {

View File

@ -244,7 +244,7 @@ public class IntegerFieldMapper extends NumberFieldMapper<Integer> {
@Override
protected void innerParseCreateField(ParseContext context, List<Field> fields) throws IOException {
int value;
float boost = context.fieldBoost(this);
float boost = this.boost;
if (context.externalValueSet()) {
Object externalValue = context.externalValue();
if (externalValue == null) {

View File

@ -234,7 +234,7 @@ public class LongFieldMapper extends NumberFieldMapper<Long> {
@Override
protected void innerParseCreateField(ParseContext context, List<Field> fields) throws IOException {
long value;
float boost = context.fieldBoost(this);
float boost = this.boost;
if (context.externalValueSet()) {
Object externalValue = context.externalValue();
if (externalValue == null) {

View File

@ -249,7 +249,7 @@ public class ShortFieldMapper extends NumberFieldMapper<Short> {
@Override
protected void innerParseCreateField(ParseContext context, List<Field> fields) throws IOException {
short value;
float boost = context.fieldBoost(this);
float boost = this.boost;
if (context.externalValueSet()) {
Object externalValue = context.externalValue();
if (externalValue == null) {

View File

@ -274,7 +274,7 @@ public class StringFieldMapper extends AbstractFieldMapper<String> implements Al
@Override
protected void parseCreateField(ParseContext context, List<Field> fields) throws IOException {
ValueAndBoost valueAndBoost = parseCreateFieldForString(context, nullValue, context.fieldBoost(this));
ValueAndBoost valueAndBoost = parseCreateFieldForString(context, nullValue, boost);
if (valueAndBoost.value() == null) {
return;
}

View File

@ -126,7 +126,7 @@ public class TokenCountFieldMapper extends IntegerFieldMapper {
@Override
protected void parseCreateField(ParseContext context, List<Field> fields) throws IOException {
ValueAndBoost valueAndBoost = StringFieldMapper.parseCreateFieldForString(context, null /* Out null value is an int so we convert*/, context.fieldBoost(this));
ValueAndBoost valueAndBoost = StringFieldMapper.parseCreateFieldForString(context, null /* Out null value is an int so we convert*/, boost);
if (valueAndBoost.value() == null && nullValue() == null) {
return;
}

View File

@ -240,7 +240,7 @@ public class TypeParsers {
final Settings settings = ImmutableSettings.builder().put(SettingsLoader.Helper.loadNestedFromMap(nodeMapValue(propNode, "fielddata"))).build();
builder.fieldDataSettings(settings);
} else if (propName.equals("copy_to")) {
parseCopyFields(name, propNode, builder);
parseCopyFields(propNode, builder);
}
}
}
@ -367,43 +367,16 @@ public class TypeParsers {
}
@SuppressWarnings("unchecked")
public static void parseCopyFields(String fieldName, Object propNode, AbstractFieldMapper.Builder builder) {
public static void parseCopyFields(Object propNode, AbstractFieldMapper.Builder builder) {
AbstractFieldMapper.CopyTo.Builder copyToBuilder = new AbstractFieldMapper.CopyTo.Builder();
if (isObject(propNode)) {
parseCopyToField(fieldName, propNode, copyToBuilder);
} else if (isArray(propNode)) {
if (isArray(propNode)) {
for(Object node : (List<Object>) propNode) {
if (isObject(node)) {
parseCopyToField(fieldName, node, copyToBuilder);
} else {
parseCopyToField(nodeStringValue(node, null), copyToBuilder);
}
copyToBuilder.add(nodeStringValue(node, null));
}
} else {
parseCopyToField(nodeStringValue(propNode, null), copyToBuilder);
copyToBuilder.add(nodeStringValue(propNode, null));
}
builder.copyTo(copyToBuilder.build());
}
public static void parseCopyToField(String fieldValue, AbstractFieldMapper.CopyTo.Builder builder) {
int carrotPos = fieldValue.indexOf('^');
if (carrotPos > 0) {
builder.add(fieldValue.substring(0, carrotPos), Float.parseFloat(fieldValue.substring(carrotPos + 1)));
} else {
builder.add(fieldValue, 1.0f);
}
}
public static void parseCopyToField(String fieldName, Object propNode, AbstractFieldMapper.CopyTo.Builder builder) {
Map<String, Object> copyToNode = (Map<String, Object>) propNode;
String field = nodeStringValue(copyToNode.get("field"), null);
if (field == null) {
throw new MapperParsingException("No type specified for property [" + fieldName + "]");
}
float boost = nodeFloatValue(copyToNode.get("boost"), 1.0f);
builder.add(field, boost);
}
}

View File

@ -234,7 +234,7 @@ public class GeoShapeFieldMapper extends AbstractFieldMapper<String> {
}
for (Field field : fields) {
if (!customBoost()) {
field.setBoost(context.fieldBoost(this));
field.setBoost(boost);
}
if (context.listener().beforeFieldAdded(this, field, context)) {
context.doc().add(field);

View File

@ -295,7 +295,7 @@ public class IpFieldMapper extends NumberFieldMapper<Long> {
final long value = ipToLong(ipAsString);
if (fieldType.indexed() || fieldType.stored()) {
CustomLongNumericField field = new CustomLongNumericField(this, value, fieldType);
field.setBoost(context.fieldBoost(this));
field.setBoost(boost);
fields.add(field);
}
if (hasDocValues()) {

View File

@ -50,7 +50,7 @@ public class CopyToMapperTests extends ElasticsearchTestCase {
String mapping = jsonBuilder().startObject().startObject("type1").startObject("properties")
.startObject("copy_test")
.field("type", "string")
.array("copy_to", "another_field^10", "cyclic_test")
.array("copy_to", "another_field", "cyclic_test")
.endObject()
.startObject("another_field")
@ -64,16 +64,7 @@ public class CopyToMapperTests extends ElasticsearchTestCase {
.startObject("int_to_str_test")
.field("type", "integer")
.startArray("copy_to")
.startObject()
.field("field", "another_field")
.field("boost", 5.5f)
.endObject()
.startObject()
.field("field", "new_field")
.endObject()
.endArray()
.array("copy_to", "another_field", "new_field")
.endObject()
.endObject().endObject().endObject().string();
@ -89,11 +80,10 @@ public class CopyToMapperTests extends ElasticsearchTestCase {
Map<String, Object> serializedMap = JsonXContent.jsonXContent.createParser(builder.bytes()).mapAndClose();
Map<String, Object> copyTestMap = (Map<String, Object>) serializedMap.get("copy_test");
assertThat(copyTestMap.get("type").toString(), is("string"));
List<Map<String, Object>> copyToMap = (List<Map<String, Object>>) copyTestMap.get("copy_to");
assertThat(copyToMap.size(), equalTo(2));
assertThat(copyToMap.get(0).get("field").toString(), equalTo("another_field"));
assertThat((Double) copyToMap.get(0).get("boost"), closeTo(10.0, 0.001));
assertThat(copyToMap.get(1).get("field").toString(), equalTo("cyclic_test"));
List<String> copyToList = (List<String>) copyTestMap.get("copy_to");
assertThat(copyToList.size(), equalTo(2));
assertThat(copyToList.get(0).toString(), equalTo("another_field"));
assertThat(copyToList.get(1).toString(), equalTo("cyclic_test"));
// Check data parsing
BytesReference json = jsonBuilder().startObject()
@ -109,9 +99,7 @@ public class CopyToMapperTests extends ElasticsearchTestCase {
assertThat(doc.getFields("another_field").length, equalTo(2));
assertThat(doc.getFields("another_field")[0].stringValue(), equalTo("foo"));
assertThat((double) (doc.getFields("another_field")[0].boost()), closeTo(10.0, 0.001));
assertThat(doc.getFields("another_field")[1].stringValue(), equalTo("42"));
assertThat((double) (doc.getFields("another_field")[1].boost()), closeTo(5.5, 0.001));
assertThat(doc.getFields("cyclic_test").length, equalTo(2));
assertThat(doc.getFields("cyclic_test")[0].stringValue(), equalTo("foo"));
@ -213,11 +201,11 @@ public class CopyToMapperTests extends ElasticsearchTestCase {
DocumentMapper docMapperBefore = MapperTestUtils.newParser().parse(mappingBefore);
ImmutableList<AbstractFieldMapper.CopyToField> fields = docMapperBefore.mappers().name("copy_test").mapper().copyTo().copyToFields();
ImmutableList<String> fields = docMapperBefore.mappers().name("copy_test").mapper().copyTo().copyToFields();
assertThat(fields.size(), equalTo(2));
assertThat(fields.get(0).field(), equalTo("foo"));
assertThat(fields.get(1).field(), equalTo("bar"));
assertThat(fields.get(0), equalTo("foo"));
assertThat(fields.get(1), equalTo("bar"));
DocumentMapper docMapperAfter = MapperTestUtils.newParser().parse(mappingAfter);
@ -231,8 +219,8 @@ public class CopyToMapperTests extends ElasticsearchTestCase {
fields = docMapperBefore.mappers().name("copy_test").mapper().copyTo().copyToFields();
assertThat(fields.size(), equalTo(2));
assertThat(fields.get(0).field(), equalTo("baz"));
assertThat(fields.get(1).field(), equalTo("bar"));
assertThat(fields.get(0), equalTo("baz"));
assertThat(fields.get(1), equalTo("bar"));
}
}