Merge pull request #16022 from vespa-engine/lesters/expressions-as-arguments-3

Lesters/expressions as arguments 3

3 files changed, 25 insertions, 24 deletions

diff --git a/searchlib/abi-spec.json b/searchlib/abi-spec.json
index 88eccb4559f..d412f408350 100644
--- a/searchlib/abi-spec.json
+++ b/searchlib/abi-spec.json
@@ -875,7 +875,6 @@
       "public final java.lang.String outs()",
       "public final java.lang.String out()",
       "public final java.util.List args()",
-      "public final com.yahoo.searchlib.rankingexpression.rule.ExpressionNode arg()",
       "public final com.yahoo.searchlib.rankingexpression.rule.ExpressionNode function()",
       "public final com.yahoo.searchlib.rankingexpression.rule.FunctionNode scalarOrTensorFunction()",
       "public final com.yahoo.searchlib.rankingexpression.rule.TensorFunctionNode tensorFunction()",
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/ExpressionFunction.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/ExpressionFunction.java
index 9f900ffed36..3974ca7ed0d 100755
--- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/ExpressionFunction.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/ExpressionFunction.java
@@ -3,7 +3,12 @@ package com.yahoo.searchlib.rankingexpression;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.yahoo.searchlib.rankingexpression.rule.ConstantNode;
 import com.yahoo.searchlib.rankingexpression.rule.ExpressionNode;
+import com.yahoo.searchlib.rankingexpression.rule.FunctionNode;
+import com.yahoo.searchlib.rankingexpression.rule.NameNode;
+import com.yahoo.searchlib.rankingexpression.rule.NegativeNode;
+import com.yahoo.searchlib.rankingexpression.rule.ReferenceNode;
 import com.yahoo.searchlib.rankingexpression.rule.SerializationContext;
 import com.yahoo.tensor.TensorType;
 import com.yahoo.text.Utf8;
@@ -131,7 +136,16 @@ public class ExpressionFunction {
     public Instance expand(SerializationContext context, List<ExpressionNode> argumentValues, Deque<String> path) {
         Map<String, String> argumentBindings = new HashMap<>();
         for (int i = 0; i < arguments.size() && i < argumentValues.size(); ++i) {
-            argumentBindings.put(arguments.get(i), argumentValues.get(i).toString(new StringBuilder(), context, path, null).toString());
+            String key = arguments.get(i);
+            ExpressionNode expr = argumentValues.get(i);
+            String binding = expr.toString(new StringBuilder(), context, path, null).toString();
+
+            if (shouldGenerateFeature(expr)) {
+                String funcName = "autogenerated_ranking_feature@" + Long.toHexString(symbolCode(key + "=" + binding));
+                context.addFunctionSerialization(RankingExpression.propertyName(funcName), binding);
+                binding = "rankingExpression(" + funcName + ")";
+            }
+            argumentBindings.put(key, binding);
         }
         context = argumentBindings.isEmpty() ? context.withoutBindings() : context.withBindings(argumentBindings);
         String symbol = toSymbol(argumentBindings);
@@ -139,6 +153,15 @@ public class ExpressionFunction {
         return new Instance(symbol, expressionString);
     }
 
+    private boolean shouldGenerateFeature(ExpressionNode expr) {
+        if (expr instanceof ConstantNode) return false;
+        if (expr instanceof ReferenceNode) return false;
+        if (expr instanceof NameNode) return false;
+        if (expr instanceof FunctionNode) return false;
+        if (expr instanceof NegativeNode && ((NegativeNode) expr).getValue() instanceof ConstantNode) return false;
+        return true;
+    }
+
     /**
      * Returns a symbolic string that represents this function with a given
      * list of arguments. The arguments are mangled by hashing the string
@@ -161,7 +184,6 @@ public class ExpressionFunction {
         return ret.toString();
     }
 
-
     /**
      * Returns a more unique hash code than what Java's own {@link
      * String#hashCode()} method would produce.
diff --git a/searchlib/src/main/javacc/RankingExpressionParser.jj b/searchlib/src/main/javacc/RankingExpressionParser.jj
index 09880b8dfc3..36b1f9627bb 100755
--- a/searchlib/src/main/javacc/RankingExpressionParser.jj
+++ b/searchlib/src/main/javacc/RankingExpressionParser.jj
@@ -328,30 +328,10 @@ List<ExpressionNode> args() :
     ExpressionNode argument;
 }
 {
-    ( ( argument = arg() { arguments.add(argument); } ( <COMMA> argument = arg() { arguments.add(argument); } )* )? )
+    ( ( argument = expression() { arguments.add(argument); } ( <COMMA> argument = expression() { arguments.add(argument); } )* )? )
     { return arguments; }
 }
 
-// TODO: Replace use of this for function arguments with value()
-// For that to work with the current search execution framework
-// we need to generate another function for the argument such that we can replace
-// instances of the argument with the reference to that function in the same way
-// as we replace by constants/names today (this can make for some fun combinatorial explosion).
-// We should also stop doing function expansion in the toString of a function.
-// - Jon 2014-05-02
-ExpressionNode arg() :
-{
-    ExpressionNode ret;
-    String name;
-    Function fnc;
-}
-{
-    ( ret = constantPrimitive()                                          |
-      LOOKAHEAD(2) ret = feature()                                       |
-      name = identifier()              { ret = new NameNode(name);       } )
-    { return ret; }
-}
-
 ExpressionNode function() :
 {
     ExpressionNode function;