Merge pull request #3570 from vespa-engine/toregge/more-datatype-checking-in-feature-setup

Narrow down allowed attribute vector data types for some feature executors

14 files changed, 53 insertions, 40 deletions

diff --git a/searchlib/src/tests/features/prod_features_attributematch.cpp b/searchlib/src/tests/features/prod_features_attributematch.cpp
index 8dcceec9a22..4ddb3170efe 100644
--- a/searchlib/src/tests/features/prod_features_attributematch.cpp
+++ b/searchlib/src/tests/features/prod_features_attributematch.cpp
@@ -18,6 +18,7 @@ using AVC = search::attribute::Config;
 using AVBT = search::attribute::BasicType;
 using AVCT = search::attribute::CollectionType;
 using CollectionType = FieldInfo::CollectionType;
+using DataType = FieldInfo::DataType;
 
 void
 Test::testAttributeMatch()
@@ -303,4 +304,10 @@ Test::testAttributeMatch()
             addScore("attributeMatch(wint).fieldCompleteness", 0);
         ASSERT_TRUE(ft.execute(exp));
     }
+    { // tensor attribute is not allowed
+        FtFeatureTest ft(_factory, "attributeMatch(tensor)");
+        ft.getIndexEnv().getBuilder().addField(FieldType::ATTRIBUTE, CollectionType::SINGLE, DataType::TENSOR, "tensor");
+        ASSERT_TRUE(ft.getQueryEnv().getBuilder().addAttributeNode("tensor") != nullptr);
+        ASSERT_TRUE(!ft.setup());
+    }
 }
diff --git a/searchlib/src/vespa/searchlib/features/agefeature.cpp b/searchlib/src/vespa/searchlib/features/agefeature.cpp
index 82227344a7f..258648408f8 100644
--- a/searchlib/src/vespa/searchlib/features/agefeature.cpp
+++ b/searchlib/src/vespa/searchlib/features/agefeature.cpp
@@ -76,5 +76,11 @@ AgeBlueprint::createExecutor(const search::fef::IQueryEnvironment &env, vespalib
     return stash.create<AgeExecutor>(attribute);
 }
 
+fef::ParameterDescriptions
+AgeBlueprint::getDescriptions() const
+{
+    return fef::ParameterDescriptions().desc().attribute(fef::ParameterDataTypeSet::normalTypeSet(), fef::ParameterCollection::ANY);
+}
+
 }
 }
diff --git a/searchlib/src/vespa/searchlib/features/agefeature.h b/searchlib/src/vespa/searchlib/features/agefeature.h
index de89edd49b1..99898af910f 100644
--- a/searchlib/src/vespa/searchlib/features/agefeature.h
+++ b/searchlib/src/vespa/searchlib/features/agefeature.h
@@ -37,9 +37,7 @@ public:
     void visitDumpFeatures(const fef::IIndexEnvironment &env, fef::IDumpFeatureVisitor &) const override;
     fef::Blueprint::UP createInstance() const override;
     fef::FeatureExecutor &createExecutor(const fef::IQueryEnvironment &env, vespalib::Stash &stash) const override;
-    fef::ParameterDescriptions getDescriptions() const override {
-        return fef::ParameterDescriptions().desc().attribute(fef::ParameterCollection::ANY);
-    }
+    fef::ParameterDescriptions getDescriptions() const override;
     bool setup(const fef::IIndexEnvironment & env, const fef::ParameterList & params) override;
 };
 
diff --git a/searchlib/src/vespa/searchlib/features/attributefeature.cpp b/searchlib/src/vespa/searchlib/features/attributefeature.cpp
index 029971b3eeb..5f03cda1869 100644
--- a/searchlib/src/vespa/searchlib/features/attributefeature.cpp
+++ b/searchlib/src/vespa/searchlib/features/attributefeature.cpp
@@ -426,5 +426,14 @@ AttributeBlueprint::createExecutor(const search::fef::IQueryEnvironment &env, ve
     }
 }
 
+fef::ParameterDescriptions
+AttributeBlueprint::getDescriptions() const
+{
+    auto dataTypeSet = fef::ParameterDataTypeSet::normalOrTensorTypeSet();
+    return fef::ParameterDescriptions().
+        desc().attribute(dataTypeSet, fef::ParameterCollection::ANY).
+        desc().attribute(dataTypeSet, fef::ParameterCollection::ANY).string();
+}
+
 } // namespace features
 } // namespace search
diff --git a/searchlib/src/vespa/searchlib/features/attributefeature.h b/searchlib/src/vespa/searchlib/features/attributefeature.h
index 18ec54e14c2..2d206fb738e 100644
--- a/searchlib/src/vespa/searchlib/features/attributefeature.h
+++ b/searchlib/src/vespa/searchlib/features/attributefeature.h
@@ -28,12 +28,7 @@ public:
 
     fef::Blueprint::UP createInstance() const override;
     fef::FeatureExecutor &createExecutor(const fef::IQueryEnvironment &env, vespalib::Stash &stash) const override;
-    fef::ParameterDescriptions getDescriptions() const  override{
-        return fef::ParameterDescriptions().
-            desc().attribute(fef::ParameterCollection::ANY).
-            desc().attribute(fef::ParameterCollection::ANY).string();
-    }
-
+    fef::ParameterDescriptions getDescriptions() const  override;
     bool setup(const fef::IIndexEnvironment & env, const fef::ParameterList & params) override;
 };
 
diff --git a/searchlib/src/vespa/searchlib/features/debug_attribute_wait.cpp b/searchlib/src/vespa/searchlib/features/debug_attribute_wait.cpp
index 7c42df8d9bb..20de658f7f6 100644
--- a/searchlib/src/vespa/searchlib/features/debug_attribute_wait.cpp
+++ b/searchlib/src/vespa/searchlib/features/debug_attribute_wait.cpp
@@ -103,6 +103,12 @@ DebugAttributeWaitBlueprint::createExecutor(const IQueryEnvironment &env, vespal
     return stash.create<DebugAttributeWaitExecutor>(env, attribute, _params);
 }
 
+fef::ParameterDescriptions
+DebugAttributeWaitBlueprint::getDescriptions() const
+{
+    return fef::ParameterDescriptions().desc().attribute(fef::ParameterDataTypeSet::normalTypeSet(), fef::ParameterCollection::ANY).number();
+}
+
 //-----------------------------------------------------------------------------
 
 } // namespace features
diff --git a/searchlib/src/vespa/searchlib/features/debug_attribute_wait.h b/searchlib/src/vespa/searchlib/features/debug_attribute_wait.h
index fbe027f96dc..fdb23a3f374 100644
--- a/searchlib/src/vespa/searchlib/features/debug_attribute_wait.h
+++ b/searchlib/src/vespa/searchlib/features/debug_attribute_wait.h
@@ -28,9 +28,7 @@ public:
     DebugAttributeWaitBlueprint();
     void visitDumpFeatures(const fef::IIndexEnvironment & env, fef::IDumpFeatureVisitor & visitor) const override;
     fef::Blueprint::UP createInstance() const override;
-    fef::ParameterDescriptions getDescriptions() const override {
-        return fef::ParameterDescriptions().desc().attribute(fef::ParameterCollection::ANY).number();
-    }
+    fef::ParameterDescriptions getDescriptions() const override;
     bool setup(const fef::IIndexEnvironment &env, const fef::ParameterList &params) override;
     fef::FeatureExecutor &createExecutor(const fef::IQueryEnvironment &env, vespalib::Stash &stash) const override;
 };
diff --git a/searchlib/src/vespa/searchlib/features/dotproductfeature.cpp b/searchlib/src/vespa/searchlib/features/dotproductfeature.cpp
index cd84eadf536..cbc8a5158a2 100644
--- a/searchlib/src/vespa/searchlib/features/dotproductfeature.cpp
+++ b/searchlib/src/vespa/searchlib/features/dotproductfeature.cpp
@@ -293,7 +293,7 @@ DotProductBlueprint::setup(const IIndexEnvironment & env, const ParameterList &
 
 ParameterDescriptions
 DotProductBlueprint::getDescriptions() const {
-    return ParameterDescriptions().desc().attribute(ParameterCollection::ANY).string();
+    return ParameterDescriptions().desc().attribute(ParameterDataTypeSet::normalTypeSet(), ParameterCollection::ANY).string();
 }
 
 Blueprint::UP
diff --git a/searchlib/src/vespa/searchlib/features/freshnessfeature.cpp b/searchlib/src/vespa/searchlib/features/freshnessfeature.cpp
index 8b484820fb5..11ae8305e16 100644
--- a/searchlib/src/vespa/searchlib/features/freshnessfeature.cpp
+++ b/searchlib/src/vespa/searchlib/features/freshnessfeature.cpp
@@ -89,6 +89,12 @@ FreshnessBlueprint::createInstance() const
     return Blueprint::UP(new FreshnessBlueprint());
 }
 
+fef::ParameterDescriptions
+FreshnessBlueprint::getDescriptions() const
+{
+    return fef::ParameterDescriptions().desc().attribute(fef::ParameterDataTypeSet::normalTypeSet(), fef::ParameterCollection::ANY);
+}
+
 FeatureExecutor &
 FreshnessBlueprint::createExecutor(const IQueryEnvironment &, vespalib::Stash &stash) const
 {
diff --git a/searchlib/src/vespa/searchlib/features/freshnessfeature.h b/searchlib/src/vespa/searchlib/features/freshnessfeature.h
index a3a893ab3fc..e156cad53ed 100644
--- a/searchlib/src/vespa/searchlib/features/freshnessfeature.h
+++ b/searchlib/src/vespa/searchlib/features/freshnessfeature.h
@@ -37,9 +37,7 @@ public:
 
     void visitDumpFeatures(const fef::IIndexEnvironment & env, fef::IDumpFeatureVisitor & visitor) const override;
     fef::Blueprint::UP createInstance() const override;
-    fef::ParameterDescriptions getDescriptions() const override {
-        return fef::ParameterDescriptions().desc().attribute(fef::ParameterCollection::ANY);
-    }
+    fef::ParameterDescriptions getDescriptions() const override;
     bool setup(const fef::IIndexEnvironment & env, const fef::ParameterList & params) override;
     fef::FeatureExecutor &createExecutor(const fef::IQueryEnvironment &env, vespalib::Stash &stash) const override;
 };
diff --git a/searchlib/src/vespa/searchlib/features/internal_max_reduce_prod_join_feature.cpp b/searchlib/src/vespa/searchlib/features/internal_max_reduce_prod_join_feature.cpp
index d07654c0a21..92120b925eb 100644
--- a/searchlib/src/vespa/searchlib/features/internal_max_reduce_prod_join_feature.cpp
+++ b/searchlib/src/vespa/searchlib/features/internal_max_reduce_prod_join_feature.cpp
@@ -130,32 +130,12 @@ InternalMaxReduceProdJoinBlueprint::createInstance() const
 ParameterDescriptions
 InternalMaxReduceProdJoinBlueprint::getDescriptions() const
 {
-    return ParameterDescriptions().desc().attribute(ParameterCollection::ARRAY).string();
-}
-
-bool supportedAttributeType(Parameter param) {
-    const FieldInfo *attributeInfo = param.asField();
-    if (attributeInfo == nullptr) {
-        return false;
-    }
-    if (attributeInfo->collection() != FieldInfo::CollectionType::ARRAY) {
-        return false;
-    }
-    if (attributeInfo->get_data_type() == FieldInfo::DataType::INT64) {
-        return true;
-    }
-    if (attributeInfo->get_data_type() == FieldInfo::DataType::INT32) {
-        return true;
-    }
-    return false;
+    return ParameterDescriptions().desc().attribute(ParameterDataTypeSet::int32OrInt64TypeSet(), ParameterCollection::ARRAY).string();
 }
 
 bool
 InternalMaxReduceProdJoinBlueprint::setup(const IIndexEnvironment &env, const ParameterList &params)
 {
-    if (!supportedAttributeType(params[0])) {
-        return false;
-    }
     _attribute = params[0].getValue();
     _query = params[1].getValue();
     describeOutput("scalar", "Internal executor for optimized execution of reduce(join(A,Q,f(x,y)(x*y)),max)");
diff --git a/searchlib/src/vespa/searchlib/features/nativeattributematchfeature.cpp b/searchlib/src/vespa/searchlib/features/nativeattributematchfeature.cpp
index 1e6423f9de8..5dda159f629 100644
--- a/searchlib/src/vespa/searchlib/features/nativeattributematchfeature.cpp
+++ b/searchlib/src/vespa/searchlib/features/nativeattributematchfeature.cpp
@@ -118,6 +118,12 @@ NativeAttributeMatchBlueprint::createInstance() const
     return Blueprint::UP(new NativeAttributeMatchBlueprint());
 }
 
+fef::ParameterDescriptions
+NativeAttributeMatchBlueprint::getDescriptions() const
+{
+    return fef::ParameterDescriptions().desc().attribute(fef::ParameterDataTypeSet::normalTypeSet(), fef::ParameterCollection::ANY).repeat();
+}
+
 bool
 NativeAttributeMatchBlueprint::setup(const IIndexEnvironment & env,
                                      const ParameterList & params)
diff --git a/searchlib/src/vespa/searchlib/features/nativeattributematchfeature.h b/searchlib/src/vespa/searchlib/features/nativeattributematchfeature.h
index e47cbed0344..e2c9dc7aef1 100644
--- a/searchlib/src/vespa/searchlib/features/nativeattributematchfeature.h
+++ b/searchlib/src/vespa/searchlib/features/nativeattributematchfeature.h
@@ -94,9 +94,7 @@ public:
     void visitDumpFeatures(const fef::IIndexEnvironment & env, fef::IDumpFeatureVisitor & visitor) const override;
     fef::Blueprint::UP createInstance() const override;
 
-    fef::ParameterDescriptions getDescriptions() const override {
-        return fef::ParameterDescriptions().desc().attribute(fef::ParameterCollection::ANY).repeat();
-    }
+    fef::ParameterDescriptions getDescriptions() const override;
     bool setup(const fef::IIndexEnvironment & env, const fef::ParameterList & params) override;
     fef::FeatureExecutor &createExecutor(const fef::IQueryEnvironment &env, vespalib::Stash &stash) const override;
     
diff --git a/searchlib/src/vespa/searchlib/fef/parameterdescriptions.h b/searchlib/src/vespa/searchlib/fef/parameterdescriptions.h
index 59dd38cf0cd..4c5d2c785cb 100644
--- a/searchlib/src/vespa/searchlib/fef/parameterdescriptions.h
+++ b/searchlib/src/vespa/searchlib/fef/parameterdescriptions.h
@@ -83,6 +83,12 @@ public:
     static ParameterDataTypeSet normalTypeSet() {
         return ParameterDataTypeSet(normalTypesMask());
     }
+    static ParameterDataTypeSet int32OrInt64TypeSet() {
+        return ParameterDataTypeSet(asMask(DataType::INT32) | asMask(DataType::INT64));
+    }
+    static ParameterDataTypeSet normalOrTensorTypeSet() {
+        return ParameterDataTypeSet(normalTypesMask() | asMask(DataType::TENSOR));
+    }
     bool allowedType(DataType dataType) const {
         return ((asMask(dataType) & _typeMask) != 0);
     }