diff options
author | Arne H Juul <arnej27959@users.noreply.github.com> | 2023-11-03 18:14:57 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-11-03 18:14:57 +0100 |
commit | a99d32f24eb23f77cd9a21800af9fc03b4715fde (patch) | |
tree | 6dbc3c7ff160c5643ddf7b73e78a1e6eda3b02ee /config-model | |
parent | 0230a1f4c33ebbd27d296a5cf5536f5f422117ec (diff) | |
parent | a946b12ff2a04cf2152a5b98cf0ed9c3ca542fba (diff) |
Merge pull request #29191 from vespa-engine/arnej/warn-when-using-array-attribute
validate for array/wset attributes, take 2:
Diffstat (limited to 'config-model')
4 files changed, 84 insertions, 4 deletions
diff --git a/config-model/src/main/java/com/yahoo/schema/FeatureNames.java b/config-model/src/main/java/com/yahoo/schema/FeatureNames.java index ab2e63aa469..c0f40cedea2 100644 --- a/config-model/src/main/java/com/yahoo/schema/FeatureNames.java +++ b/config-model/src/main/java/com/yahoo/schema/FeatureNames.java @@ -27,6 +27,7 @@ public class FeatureNames { /** Returns true if the given reference is an attribute, constant or query feature */ public static boolean isSimpleFeature(Reference reference) { if ( ! reference.isSimple()) return false; + if (reference.output() != null) return false; String name = reference.name(); return name.equals("attribute") || name.equals("constant") || name.equals("query"); } diff --git a/config-model/src/main/java/com/yahoo/schema/RankProfile.java b/config-model/src/main/java/com/yahoo/schema/RankProfile.java index 1ff85c9c89f..502b054f84e 100644 --- a/config-model/src/main/java/com/yahoo/schema/RankProfile.java +++ b/config-model/src/main/java/com/yahoo/schema/RankProfile.java @@ -1265,14 +1265,45 @@ public class RankProfile implements Cloneable { return Optional.empty(); // if this context does not contain this input } + private static class AttributeErrorType extends TensorType { + private final DeployLogger deployLogger; + private final String attr; + private final Attribute.CollectionType collType; + private boolean shouldWarn = true; + AttributeErrorType(DeployLogger deployLogger, String attr, Attribute.CollectionType collType) { + super(TensorType.Value.DOUBLE, List.of()); + this.deployLogger = deployLogger; + this.attr = attr; + this.collType = collType; + } + private void warnOnce() { + if (shouldWarn) { + deployLogger.log(Level.WARNING, "Using attribute(" + attr +") " + collType + " in ranking expression will always evaluate to 0.0"); + shouldWarn = false; + } + } + @Override public TensorType.Value valueType() { warnOnce(); return super.valueType(); } + @Override public int rank() { warnOnce(); return super.rank(); } + @Override public List<TensorType.Dimension> dimensions() { warnOnce(); return super.dimensions(); } + @Override public boolean equals(Object o) { + if (o instanceof TensorType other) { + return (other.rank() == 0); + } + return false; + } + } + private void addAttributeFeatureTypes(ImmutableSDField field, Map<Reference, TensorType> featureTypes) { Attribute attribute = field.getAttribute(); field.getAttributes().forEach((k, a) -> { String name = k; if (attribute == a) // this attribute should take the fields name name = field.getName(); // switch to that - it is separate for imported fields - featureTypes.put(FeatureNames.asAttributeFeature(name), - a.tensorType().orElse(TensorType.empty)); + if (a.getCollectionType().equals(Attribute.CollectionType.SINGLE)) { + featureTypes.put(FeatureNames.asAttributeFeature(name), a.tensorType().orElse(TensorType.empty)); + } else { + featureTypes.put(FeatureNames.asAttributeFeature(name), new AttributeErrorType(deployLogger, name, a.getCollectionType())); + } }); } diff --git a/config-model/src/test/java/com/yahoo/schema/processing/RankingExpressionTypeResolverTestCase.java b/config-model/src/test/java/com/yahoo/schema/processing/RankingExpressionTypeResolverTestCase.java index 037c5c31334..e1f3aea3525 100644 --- a/config-model/src/test/java/com/yahoo/schema/processing/RankingExpressionTypeResolverTestCase.java +++ b/config-model/src/test/java/com/yahoo/schema/processing/RankingExpressionTypeResolverTestCase.java @@ -459,6 +459,54 @@ public class RankingExpressionTypeResolverTestCase { } @Test + void requireThatUsingArrayWarns() throws Exception { + InspectableDeployLogger logger = new InspectableDeployLogger(); + ApplicationBuilder builder = new ApplicationBuilder(logger); + builder.addSchema(joinLines( + "search test {", + " document test { ", + " field foo type array<float> {", + " indexing: attribute", + " }", + " }", + " rank-profile my_rank_profile {", + " first-phase {", + " expression: map(attribute(foo), f(x)(42*x))", + " }", + " }", + "}" + )); + builder.build(true); + String message = logger.findMessage("collection"); + assertNotNull(message); + assertEquals("WARNING: Using attribute(foo) collectiontype: ARRAY in ranking expression will always evaluate to 0.0", message); + } + + @Test + void requireThatUsingWsetWarns() throws Exception { + InspectableDeployLogger logger = new InspectableDeployLogger(); + ApplicationBuilder builder = new ApplicationBuilder(logger); + builder.addSchema(joinLines( + "search test {", + " document test { ", + " field foo type weightedset<int> {", + " indexing: attribute", + " }", + " }", + " rank-profile my_rank_profile {", + " first-phase {", + " expression: attribute(foo)", + " }", + " }", + "}" + )); + builder.build(true); + String message = logger.findMessage("collection"); + assertNotNull(message); + assertEquals("WARNING: Using attribute(foo) collectiontype: WEIGHTEDSET in ranking expression will always evaluate to 0.0", message); + } + + @Test void noWarningWhenUsingTensorsWhenQueryFeaturesAreDeclared() throws Exception { InspectableDeployLogger logger = new InspectableDeployLogger(); ApplicationBuilder builder = new ApplicationBuilder(logger); diff --git a/config-model/src/test/java/com/yahoo/schema/processing/TensorTransformTestCase.java b/config-model/src/test/java/com/yahoo/schema/processing/TensorTransformTestCase.java index 58a0b54e6cc..1e1fdf67734 100644 --- a/config-model/src/test/java/com/yahoo/schema/processing/TensorTransformTestCase.java +++ b/config-model/src/test/java/com/yahoo/schema/processing/TensorTransformTestCase.java @@ -35,8 +35,8 @@ public class TensorTransformTestCase extends AbstractSchemaTestCase { "max(1.0,2.0)"); assertTransformedExpression("min(attribute(double_field),x)", "min(attribute(double_field),x)"); - assertTransformedExpression("max(attribute(double_field),attribute(double_array_field))", - "max(attribute(double_field),attribute(double_array_field))"); + assertTransformedExpression("max(attribute(double_field),attribute(double_array_field).count)", + "max(attribute(double_field),attribute(double_array_field).count)"); assertTransformedExpression("min(attribute(tensor_field_1),attribute(double_field))", "min(attribute(tensor_field_1),attribute(double_field))"); assertTransformedExpression("reduce(max(attribute(tensor_field_1),attribute(tensor_field_2)),sum)", |