Skip to content

Commit

Permalink
Merge pull request #30869 from vespa-engine/toregge/improve-error-mes…
Browse files Browse the repository at this point in the history 
…sage-when-field-cannot-have-attribute-aspect

Improve error message when field cannot have attribute aspect.
  • Loading branch information
@toregge
toregge authored Apr 10, 2024
2 parents 6adf464 + 6a53177 commit af8bae3
Show file tree
Hide file tree
Showing 30 changed files with 125 additions and 63 deletions.
24 changes: 20 additions & 4 deletions config-model/src/main/java/com/yahoo/schema/document/Attribute.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
import com.yahoo.document.CollectionDataType;
import com.yahoo.document.DataType;
import com.yahoo.document.DocumentType;
import com.yahoo.document.MapDataType;
import com.yahoo.document.PrimitiveDataType;
import com.yahoo.document.StructDataType;
import com.yahoo.documentmodel.NewDocumentReferenceDataType;
import com.yahoo.document.StructuredDataType;
import com.yahoo.document.TensorDataType;
Expand Down Expand Up @@ -143,8 +145,8 @@ public String toString() {
}

/** Creates an attribute with default settings */
public Attribute(String name, DataType fieldType) {
this(name, convertDataType(fieldType), convertCollectionType(fieldType), convertTensorType(fieldType), convertTargetType(fieldType));
public Attribute(String schemaName, String fieldName, String name, DataType fieldType) {
this(name, convertDataType(schemaName, fieldName, fieldType), convertCollectionType(fieldType), convertTensorType(fieldType), convertTargetType(fieldType));
setRemoveIfZero(fieldType instanceof WeightedSetDataType wsdt && wsdt.removeIfZero());
setCreateIfNonExistent(fieldType instanceof WeightedSetDataType wsdt && wsdt.createIfNonExistent());
}
Expand Down Expand Up @@ -266,12 +268,26 @@ public void setFastRank(boolean value) {
private void setType(Type type) { this.type=type; }
public void setCollectionType(CollectionType type) { this.collectionType=type; }

private static void failDataType(String schemaName, String fieldName, String dataType) throws IllegalArgumentException {
throw new IllegalArgumentException("For schema '" + schemaName + "': Field '" + fieldName + "' of type '" + dataType + "' cannot be an attribute. " +
"Instead specify the struct fields to be searchable as attribute");
}
public static void validateDataType(String schemaName, String fieldName, DataType fieldType) throws IllegalArgumentException {
if (fieldType instanceof MapDataType mapType) {
failDataType(schemaName, fieldName, "map<" + mapType.getKeyType().getName() + "," + mapType.getValueType().getName() + ">");
}
if (fieldType instanceof ArrayDataType arrayType && arrayType.getNestedType() instanceof StructDataType nestedType) {
failDataType(schemaName, fieldName, "array<" + nestedType.getName() + ">");
}
}

/** Converts to the right attribute type from a field datatype */
public static Type convertDataType(DataType fieldType) {
public static Type convertDataType(String schemaName, String fieldName, DataType fieldType) {
validateDataType(schemaName, fieldName, fieldType);
if (fieldType instanceof NewDocumentReferenceDataType) {
return Type.REFERENCE;
} else if (fieldType instanceof CollectionDataType) {
return convertDataType(((CollectionDataType) fieldType).getNestedType());
return convertDataType(schemaName, fieldName, ((CollectionDataType) fieldType).getNestedType());
}
FieldValue fval = fieldType.createFieldValue();
if (fval instanceof StringFieldValue) {
Expand Down
15 changes: 8 additions & 7 deletions config-model/src/main/java/com/yahoo/schema/document/SDField.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,22 +398,23 @@ public boolean hasSingleAttribute() {
}

/** Parse an indexing expression which will use the simple linguistics implementation suitable for testing */
public void parseIndexingScript(String script) {
parseIndexingScript(script, new SimpleLinguistics(), Embedder.throwsOnUse.asMap());
public void parseIndexingScript(String schemaName, String script) {
parseIndexingScript(schemaName, script, new SimpleLinguistics(), Embedder.throwsOnUse.asMap());
}

public void parseIndexingScript(String script, Linguistics linguistics, Map<String, Embedder> embedders) {
public void parseIndexingScript(String schemaName, String script, Linguistics linguistics, Map<String, Embedder> embedders) {
try {
ScriptParserContext config = new ScriptParserContext(linguistics, embedders);
config.setInputStream(new IndexingInput(script));
setIndexingScript(ScriptExpression.newInstance(config));
setIndexingScript(schemaName, ScriptExpression.newInstance(config));
} catch (ParseException e) {
throw new IllegalArgumentException("Failed to parse script '" + script + "'", e);
}
}

/** Sets the indexing script of this, or null to not use a script */
public void setIndexingScript(ScriptExpression exp) {

public void setIndexingScript(String schemaName, ScriptExpression exp) {
if (exp == null) {
exp = new ScriptExpression();
}
Expand Down Expand Up @@ -441,13 +442,13 @@ protected void doVisit(Expression exp) {
}
Attribute attribute = attributes.get(fieldName);
if (attribute == null) {
addAttribute(new Attribute(fieldName, getDataType()));
addAttribute(new Attribute(schemaName, fieldName, fieldName, getDataType()));
}
}
}.visit(indexingScript);
}
for (SDField structField : getStructFields()) {
structField.setIndexingScript(exp);
structField.setIndexingScript(schemaName, exp);
}
}

Expand Down
2 changes: 1 addition & 1 deletion config-model/src/main/java/com/yahoo/schema/fieldoperation/FieldOperation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
public interface FieldOperation extends Comparable<FieldOperation> {

/** Apply this operation on the given field */
void apply(SDField field);
void apply(String schemaName, SDField field);

@Override
default int compareTo(FieldOperation other) {
Expand Down
4 changes: 2 additions & 2 deletions config-model/src/main/java/com/yahoo/schema/fieldoperation/IndexingOperation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ public IndexingOperation(ScriptExpression script) {

public ScriptExpression getScript() { return script; }

public void apply(SDField field) {
field.setIndexingScript(script);
public void apply(String schemaName, SDField field) {
field.setIndexingScript(schemaName, script);
}

/** Creates an indexing operation which will use the simple linguistics implementation suitable for testing */
Expand Down
26 changes: 15 additions & 11 deletions config-model/src/main/java/com/yahoo/schema/parser/ConvertParsedFields.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
package com.yahoo.schema.parser;

import com.yahoo.document.DataType;
import com.yahoo.schema.document.GeoPos;
import com.yahoo.schema.parser.ConvertParsedTypes.TypeResolver;
import com.yahoo.schema.Index;
import com.yahoo.schema.Schema;
Expand Down Expand Up @@ -49,10 +50,10 @@ static void convertMatchSettings(SDField field, ParsedMatchSettings parsed) {
(exactMatchTerminator -> field.getMatching().setExactMatchTerminator(exactMatchTerminator));
}

void convertSorting(SDField field, ParsedSorting parsed, String name) {
void convertSorting(Schema schema, SDField field, ParsedSorting parsed, String name) {
Attribute attribute = field.getAttributes().get(name);
if (attribute == null) {
attribute = new Attribute(name, field.getDataType());
attribute = new Attribute(schema.getName(), field.getName(), name, field.getDataType());
field.addAttribute(attribute);
}
Sorting sorting = attribute.getSorting();
Expand All @@ -66,7 +67,7 @@ void convertSorting(SDField field, ParsedSorting parsed, String name) {
parsed.getLocale().ifPresent(locale -> sorting.setLocale(locale));
}

void convertAttribute(SDField field, ParsedAttribute parsed) {
void convertAttribute(Schema schema, SDField field, ParsedAttribute parsed) {
String name = parsed.name();
String fieldName = field.getName();
Attribute attribute = null;
Expand All @@ -76,7 +77,7 @@ void convertAttribute(SDField field, ParsedAttribute parsed) {
if (attribute == null) {
attribute = field.getAttributes().get(name);
if (attribute == null) {
attribute = new Attribute(name, field.getDataType());
attribute = new Attribute(schema.getName(), field.getName(), name, field.getDataType());
field.addAttribute(attribute);
}
}
Expand All @@ -102,7 +103,7 @@ void convertAttribute(SDField field, ParsedAttribute parsed) {
}
var sorting = parsed.getSorting();
if (sorting.isPresent()) {
convertSorting(field, sorting.get(), name);
convertSorting(schema, field, sorting.get(), name);
}
}

Expand Down Expand Up @@ -143,13 +144,16 @@ private void convertCommonFieldSettings(Schema schema, SDField field, ParsedFiel
convertMatchSettings(field, parsed.matchSettings());
var indexing = parsed.getIndexing();
if (indexing.isPresent()) {
field.setIndexingScript(indexing.get().script());
field.setIndexingScript(schema.getName(), indexing.get().script());
}
if (field.doesAttributing() && !GeoPos.isAnyPos(field.getDataType())) {
Attribute.validateDataType(schema.getName(), field.getName(), field.getDataType());
}
parsed.getWeight().ifPresent(value -> field.setWeight(value));
parsed.getStemming().ifPresent(value -> field.setStemming(value));
parsed.getNormalizing().ifPresent(value -> convertNormalizing(field, value));
for (var attribute : parsed.getAttributes()) {
convertAttribute(field, attribute);
convertAttribute(schema, field, attribute);
}
for (var summaryField : parsed.getSummaryFields()) {
var dataType = field.getDataType();
Expand Down Expand Up @@ -190,7 +194,7 @@ private void convertStructField(Schema schema, SDField field, ParsedField parsed
convertCommonFieldSettings(schema, structField, parsed);
}

private void convertExtraFieldSettings(SDField field, ParsedField parsed) {
private void convertExtraFieldSettings(Schema schema, SDField field, ParsedField parsed) {
String name = parsed.name();
for (var dictOp : parsed.getDictionaryOptions()) {
var dictionary = field.getOrSetDictionary();
Expand All @@ -208,7 +212,7 @@ private void convertExtraFieldSettings(SDField field, ParsedField parsed) {
field.getAliasToName().put(alias, parsed.lookupAliasedFrom(alias));
}
parsed.getRankTypes().forEach((indexName, rankType) -> convertRankType(field, indexName, rankType));
parsed.getSorting().ifPresent(sortInfo -> convertSorting(field, sortInfo, name));
parsed.getSorting().ifPresent(sortInfo -> convertSorting(schema, field, sortInfo, name));
if (parsed.hasBolding()) {
// TODO must it be so ugly:
SummaryField summaryField = field.getSummaryField(name, true);
Expand Down Expand Up @@ -288,7 +292,7 @@ SDField convertDocumentField(Schema schema, SDDocumentType document, ParsedField
DataType dataType = context.resolveType(parsed.getType());
var field = new SDField(document, name, dataType);
convertCommonFieldSettings(schema, field, parsed);
convertExtraFieldSettings(field, parsed);
convertExtraFieldSettings(schema, field, parsed);
document.addField(field);
return field;
}
Expand All @@ -298,7 +302,7 @@ void convertExtraField(Schema schema, ParsedField parsed) {
DataType dataType = context.resolveType(parsed.getType());
var field = new SDField(schema.getDocument(), name, dataType);
convertCommonFieldSettings(schema, field, parsed);
convertExtraFieldSettings(field, parsed);
convertExtraFieldSettings(schema, field, parsed);
schema.addExtraField(field);
}

Expand Down
8 changes: 4 additions & 4 deletions config-model/src/main/java/com/yahoo/schema/processing/CreatePositionZCurve.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,14 @@ public void process(boolean validate, boolean documentsOnly) {
SummaryTransform.DISTANCE, summaryTo, validate);
}
// clear indexing script
field.setIndexingScript(null);
field.setIndexingScript(schema.getName(), null);
SDField posX = field.getStructField(PositionDataType.FIELD_X);
if (posX != null) {
posX.setIndexingScript(null);
posX.setIndexingScript(schema.getName(), null);
}
SDField posY = field.getStructField(PositionDataType.FIELD_Y);
if (posY != null) {
posY.setIndexingScript(null);
posY.setIndexingScript(schema.getName(), null);
}
if (doesSummary) ensureCompatibleSummary(field, zName,
field.getName(),
Expand All @@ -118,7 +118,7 @@ private SDField createZCurveField(SDField inputField, String fieldName, boolean
ScriptExpression script = inputField.getIndexingScript();
script = (ScriptExpression)new RemoveSummary(inputField.getName()).convert(script);
script = (ScriptExpression)new PerformZCurve(field, fieldName).convert(script);
field.setIndexingScript(script);
field.setIndexingScript(schema.getName(), script);
return field;
}

Expand Down
2 changes: 1 addition & 1 deletion config-model/src/main/java/com/yahoo/schema/processing/ExactMatch.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ private void implementExactMatch(SDField field, Schema schema) {
}
ScriptExpression script = field.getIndexingScript();
if (new ExpressionSearcher<>(IndexExpression.class).containedIn(script)) {
field.setIndexingScript((ScriptExpression)new MyProvider(schema).convert(field.getIndexingScript()));
field.setIndexingScript(schema.getName(), (ScriptExpression)new MyProvider(schema).convert(field.getIndexingScript()));
}
}

Expand Down
2 changes: 1 addition & 1 deletion config-model/src/main/java/com/yahoo/schema/processing/IndexingInputs.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public void process(boolean validate, boolean documentsOnly) {
if (validate)
new VerifyInputExpression(schema, field).visit(script);

field.setIndexingScript(script);
field.setIndexingScript(schema.getName(), script);
}
}

Expand Down
2 changes: 1 addition & 1 deletion config-model/src/main/java/com/yahoo/schema/processing/IndexingOutputs.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public void process(boolean validate, boolean documentsOnly) {
Set<String> summaryFields = new TreeSet<>();
findSummaryTo(schema, field, summaryFields, summaryFields);
MyConverter converter = new MyConverter(schema, field, summaryFields, validate);
field.setIndexingScript((ScriptExpression)converter.convert(script));
field.setIndexingScript(schema.getName(), (ScriptExpression)converter.convert(script));
}
}

Expand Down
2 changes: 1 addition & 1 deletion config-model/src/main/java/com/yahoo/schema/processing/IntegerIndex2Attribute.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public void process(boolean validate, boolean documentsOnly) {
ScriptExpression script = field.getIndexingScript();
Set<String> attributeNames = new HashSet<>();
new MyVisitor(attributeNames).visit(script);
field.setIndexingScript((ScriptExpression)new MyConverter(attributeNames).convert(script));
field.setIndexingScript(schema.getName(), (ScriptExpression)new MyConverter(attributeNames).convert(script));
warn(schema, field, "Changed to attribute because numerical indexes (field has type " +
field.getDataType().getName() + ") is not currently supported." +
" Index-only settings may fail. Ignore this warning for streaming search.");
Expand Down
2 changes: 1 addition & 1 deletion config-model/src/main/java/com/yahoo/schema/processing/NGramMatch.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ private void implementGramMatch(Schema schema, SDField field, boolean validate)
field.getNormalizing().inferCodepoint();
field.setStemming(Stemming.NONE); // not compatible with stemming and normalizing
field.addQueryCommand("ngram " + n);
field.setIndexingScript((ScriptExpression)new MyProvider(schema, n).convert(field.getIndexingScript()));
field.setIndexingScript(schema.getName(), (ScriptExpression)new MyProvider(schema, n).convert(field.getIndexingScript()));
}

private static class MyProvider extends TypedTransformProvider {
Expand Down
2 changes: 1 addition & 1 deletion config-model/src/main/java/com/yahoo/schema/processing/OptimizeIlscript.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public void process(boolean validate, boolean documentsOnly) {
ScriptExpression script = field.getIndexingScript();
if (script == null) continue;

field.setIndexingScript((ScriptExpression)new ExpressionOptimizer().convert(script));
field.setIndexingScript(schema.getName(), (ScriptExpression)new ExpressionOptimizer().convert(script));
if ( ! field.getIndexingScript().toString().equals(script.toString())) {
info(schema, field, "Rewrote ilscript from:\n" + script.toString() +
"\nto\n" + field.getIndexingScript().toString());
Expand Down
2 changes: 1 addition & 1 deletion config-model/src/main/java/com/yahoo/schema/processing/PredicateProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ private void addPredicateOptimizationIlScript(SDField field, BooleanIndexDefinit
script = new StatementExpression(makeSetPredicateVariablesScript(booleanIndexDefiniton), script);

ExpressionConverter converter = new PredicateOutputTransformer(schema);
field.setIndexingScript(new ScriptExpression((StatementExpression)converter.convert(script)));
field.setIndexingScript(schema.getName(), new ScriptExpression((StatementExpression)converter.convert(script)));
}

private Expression makeSetPredicateVariablesScript(BooleanIndexDefinition options) {
Expand Down
2 changes: 1 addition & 1 deletion config-model/src/main/java/com/yahoo/schema/processing/Processor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ protected SDField addField(Schema schema, SDField field, String suffix, String i
implementationField.setRankType(RankType.EMPTY);
implementationField.setStemming(Stemming.NONE);
implementationField.getNormalizing().inferCodepoint();
implementationField.parseIndexingScript(indexing);
implementationField.parseIndexingScript(schema.getName(), indexing);
String indexName = field.getName();
String implementationIndexName = indexName + "_" + suffix;
Index implementationIndex = new Index(implementationIndexName);
Expand Down
2 changes: 1 addition & 1 deletion config-model/src/main/java/com/yahoo/schema/processing/TextMatch.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public void process(boolean validate, boolean documentsOnly) {
if ( ! visitor.requiresTokenize) continue;

ExpressionConverter converter = new MyStringTokenizer(schema, findAnnotatorConfig(schema, field));
field.setIndexingScript((ScriptExpression)converter.convert(script));
field.setIndexingScript(schema.getName(), (ScriptExpression)converter.convert(script));
}
}

Expand Down
4 changes: 2 additions & 2 deletions config-model/src/test/java/com/yahoo/schema/AttributeUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
*/
public class AttributeUtils {

public static void addAttributeAspect(SDField field) {
field.parseIndexingScript("{ attribute }");
public static void addAttributeAspect(String schemaName, SDField field) {
field.parseIndexingScript(schemaName, "{ attribute }");
}

}
4 changes: 2 additions & 2 deletions config-model/src/test/java/com/yahoo/schema/DocumentReferenceResolverTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ void reference_from_one_document_to_another_is_resolved() {
SDDocumentType fooDocument = new SDDocumentType("foo", fooSchema);
SDField fooRefToBarField = new SDField
(fooDocument, "bar_ref", new NewDocumentReferenceDataType(barDocument.getDocumentType()));
AttributeUtils.addAttributeAspect(fooRefToBarField);
AttributeUtils.addAttributeAspect(fooSchema.getName(), fooRefToBarField);
SDField irrelevantField = new SDField(fooDocument, "irrelevant_stuff", DataType.INT);
fooDocument.addField(fooRefToBarField);
fooDocument.addField(irrelevantField);
Expand All @@ -60,7 +60,7 @@ void throws_user_friendly_exception_if_referenced_document_does_not_exist() {
SDField fooRefToBarField = new SDField(
fooDocument,
"bar_ref", NewDocumentReferenceDataType.forDocumentName("bar"));
AttributeUtils.addAttributeAspect(fooRefToBarField);
AttributeUtils.addAttributeAspect(fooSchema.getName(), fooRefToBarField);
fooDocument.addField(fooRefToBarField);
fooSchema.addDocument(fooDocument);

Expand Down
2 changes: 1 addition & 1 deletion config-model/src/test/java/com/yahoo/schema/ImportedFieldsEnumeratorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ void imported_fields_are_enumerated_and_copied_from_correct_search_instance() {
Schema parentSchema = new Schema(PARENT, MockApplicationPackage.createEmpty());
SDDocumentType parentDocument = new SDDocumentType(PARENT, parentSchema);
var parentField = new SDField(parentDocument, "their_field", DataType.INT);
AttributeUtils.addAttributeAspect(parentField);
AttributeUtils.addAttributeAspect(parentSchema.getName(), parentField);
parentDocument.addField(parentField);
parentSchema.addDocument(parentDocument);

Expand Down
Loading

0 comments on commit af8bae3

Please sign in to comment.