Fix comments related to PR

3 files changed, 31 insertions, 21 deletions

diff --git a/searchlib/src/tests/features/internal_max_reduce_prod_join_feature/internal_max_reduce_prod_join_feature_test.cpp b/searchlib/src/tests/features/internal_max_reduce_prod_join_feature/internal_max_reduce_prod_join_feature_test.cpp
index 3553d27aa42..86eaaf2e083 100644
--- a/searchlib/src/tests/features/internal_max_reduce_prod_join_feature/internal_max_reduce_prod_join_feature_test.cpp
+++ b/searchlib/src/tests/features/internal_max_reduce_prod_join_feature/internal_max_reduce_prod_join_feature_test.cpp
@@ -34,25 +34,20 @@ struct SetupFixture
     }
 };
 
-TEST_F("require that blueprint can be created", SetupFixture("attribute(foo)"))
+TEST_F("require that blueprint can be created", SetupFixture("attr"))
 {
     EXPECT_TRUE(FTA::assertCreateInstance(f.blueprint, "internalMaxReduceProdJoin"));
 }
 
-TEST_F("require that setup fails if source spec is invalid", SetupFixture("attribute(foo)"))
+TEST_F("require that setup fails if attribute does not exist", SetupFixture("attr"))
 {
-    FTA::FT_SETUP_FAIL(f.blueprint, f.indexEnv, StringList().add("source(foo)"));
+    FTA::FT_SETUP_FAIL(f.blueprint, f.indexEnv, StringList().add("foo").add("bar"));
 }
 
-TEST_F("require that setup fails if attribute does not exist", SetupFixture("attribute(foo)"))
-{
-    FTA::FT_SETUP_FAIL(f.blueprint, f.indexEnv, StringList().add("attribute(bar)").add("query(baz)"));
-}
-
-TEST_F("require that setup succeeds with attribute and query parameters", SetupFixture("attribute(foo)"))
+TEST_F("require that setup succeeds with attribute and query parameters", SetupFixture("attr"))
 {
     FTA::FT_SETUP_OK(f.blueprint, f.indexEnv,
-                    StringList().add("attribute(foo)").add("query(bar)"),
+                    StringList().add("attr").add("query"),
                     StringList(),
                     StringList().add("scalar"));
 }
@@ -72,8 +67,8 @@ struct ExecFixture
     }
 
     void setupAttributeVectors() {
-        vespalib::string attrIntArray = "attribute(intarray)";
-        vespalib::string attrLongArray = "attribute(longarray)";
+        vespalib::string attrIntArray = "intarray";
+        vespalib::string attrLongArray = "longarray";
 
         test.getIndexEnv().getBuilder().addField(FieldType::ATTRIBUTE, CollectionType::ARRAY, attrLongArray);
         test.getIndexEnv().getBuilder().addField(FieldType::ATTRIBUTE, CollectionType::ARRAY, attrIntArray);
@@ -102,9 +97,10 @@ struct ExecFixture
     }
 
     void setupQueryEnvironment() {
-        test.getQueryEnv().getProperties().add("query(wset)", "{1111:1234, 2222:2245}");
-        test.getQueryEnv().getProperties().add("query(wsetnomatch)", "{1:1000, 2:2000}");
-        test.getQueryEnv().getProperties().add("query(array)", "[1111,2222]");
+        test.getQueryEnv().getProperties().add("wset", "{1111:1234, 2222:2245}");
+        test.getQueryEnv().getProperties().add("wsetnomatch", "{1:1000, 2:2000}");
+        test.getQueryEnv().getProperties().add("array", "[1111,2222]");
+        test.getQueryEnv().getProperties().add("negativewset", "{1111:-1000, 78:-42}");
     }
 
     bool evaluatesTo(feature_t expectedValue) {
@@ -114,29 +110,36 @@ struct ExecFixture
 };
 
 TEST_F("require that executor returns correct result for long array",
-       ExecFixture("internalMaxReduceProdJoin(attribute(longarray),query(wset))"))
+       ExecFixture("internalMaxReduceProdJoin(longarray,wset)"))
 {
     EXPECT_FALSE(f.evaluatesTo(1234));
     EXPECT_TRUE(f.evaluatesTo(2245));
 }
 
 TEST_F("require that executor returns correct result for int array",
-       ExecFixture("internalMaxReduceProdJoin(attribute(intarray),query(wset))"))
+       ExecFixture("internalMaxReduceProdJoin(intarray,wset)"))
 {
     EXPECT_TRUE(f.evaluatesTo(1234));
     EXPECT_FALSE(f.evaluatesTo(2245));
 }
 
 TEST_F("require that executor returns 0 if no items match",
-       ExecFixture("internalMaxReduceProdJoin(attribute(longarray),query(wsetnomatch))"))
+       ExecFixture("internalMaxReduceProdJoin(longarray,wsetnomatch)"))
 {
     EXPECT_TRUE(f.evaluatesTo(0.0));
 }
 
 TEST_F("require that executor return 0 if query is not a weighted set",
-       ExecFixture("internalMaxReduceProdJoin(attribute(longarray),query(array))"))
+       ExecFixture("internalMaxReduceProdJoin(longarray,array)"))
 {
     EXPECT_TRUE(f.evaluatesTo(0.0));
 }
 
+TEST_F("require that executor supports negative numbers",
+       ExecFixture("internalMaxReduceProdJoin(intarray,negativewset)"))
+{
+    EXPECT_FALSE(f.evaluatesTo(-1000));
+    EXPECT_TRUE(f.evaluatesTo(-42));
+}
+
 TEST_MAIN() { TEST_RUN_ALL(); }
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 593407d86eb..07086c6e7ac 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,7 +130,7 @@ InternalMaxReduceProdJoinBlueprint::createInstance() const
 ParameterDescriptions
 InternalMaxReduceProdJoinBlueprint::getDescriptions() const
 {
-    return ParameterDescriptions().desc().attribute(ParameterCollection::ANY).string();
+    return ParameterDescriptions().desc().attribute(ParameterCollection::ARRAY).string();
 }
 
 bool
diff --git a/searchlib/src/vespa/searchlib/features/internal_max_reduce_prod_join_feature.h b/searchlib/src/vespa/searchlib/features/internal_max_reduce_prod_join_feature.h
index 5650b91092e..82dfbd3231e 100644
--- a/searchlib/src/vespa/searchlib/features/internal_max_reduce_prod_join_feature.h
+++ b/searchlib/src/vespa/searchlib/features/internal_max_reduce_prod_join_feature.h
@@ -10,7 +10,14 @@ namespace features {
 /**
  * Feature for the specific replacement of the expression:
  *
- *      reduce(join(tensorFromLabels(A),tensorFromWeightedset(Q),f(x,y)(x*y)),max)
+ *      reduce(
+ *          join(
+ *              tensorFromLabels(attribute(A)),
+ *              tensorFromWeightedset(query(Q)),
+ *              f(x,y)(x*y)
+ *          ),
+ *          max
+ *      )
  *
  * where A is an array attribute of int or long type and Q is a query that parses as
  * a weighted set. This expression is replaced with this feature to avoid incurring