From 083aa54d59aecc9f9d045d4bde6cdb6c6cbe4dec Mon Sep 17 00:00:00 2001
From: Jon Bratseth <bratseth@gmail.com>
Date: Tue, 20 Sep 2022 11:00:48 +0200
Subject: Short circuit boolean expressions

Short circuit boolean expressions by converting them
to (nested) if expressions. This also fixes a bug in
Java expression evaluation where evaluation of arithmetic
operations with the same precedence would be from right
to left rather than left to right.
---
 .../schema/RankingExpressionInliningTestCase.java  |  4 +-
 .../BooleanExpressionTransformerTestCase.java      | 57 ++++++++++++++++++++++
 2 files changed, 59 insertions(+), 2 deletions(-)
 create mode 100644 config-model/src/test/java/com/yahoo/schema/expressiontransforms/BooleanExpressionTransformerTestCase.java

(limited to 'config-model/src/test/java/com')

diff --git a/config-model/src/test/java/com/yahoo/schema/RankingExpressionInliningTestCase.java b/config-model/src/test/java/com/yahoo/schema/RankingExpressionInliningTestCase.java
index 789c4ac5577..5eecee516ec 100644
--- a/config-model/src/test/java/com/yahoo/schema/RankingExpressionInliningTestCase.java
+++ b/config-model/src/test/java/com/yahoo/schema/RankingExpressionInliningTestCase.java
@@ -163,7 +163,7 @@ public class RankingExpressionInliningTestCase extends AbstractSchemaTestCase {
                         "    \n" +
                         "    rank-profile test {\n" +
                         "        first-phase {\n" +
-                        "            expression: A + C + D\n" +
+                        "            expression: A + C - D\n" +
                         "        }\n" +
                         "        function inline D() {\n" +
                         "            expression: B + 1\n" +
@@ -184,7 +184,7 @@ public class RankingExpressionInliningTestCase extends AbstractSchemaTestCase {
         Schema s = builder.getSchema();
 
         RankProfile test = rankProfileRegistry.get(s, "test").compile(new QueryProfileRegistry(), new ImportedMlModels());
-        assertEquals("attribute(a) + C + (attribute(b) + 1)", test.getFirstPhaseRanking().getRoot().toString());
+        assertEquals("attribute(a) + C - (attribute(b) + 1)", test.getFirstPhaseRanking().getRoot().toString());
         assertEquals("attribute(a) + attribute(b)", getRankingExpression("C", test, s));
         assertEquals("attribute(b) + 1", getRankingExpression("D", test, s));
     }
diff --git a/config-model/src/test/java/com/yahoo/schema/expressiontransforms/BooleanExpressionTransformerTestCase.java b/config-model/src/test/java/com/yahoo/schema/expressiontransforms/BooleanExpressionTransformerTestCase.java
new file mode 100644
index 00000000000..71d0657c701
--- /dev/null
+++ b/config-model/src/test/java/com/yahoo/schema/expressiontransforms/BooleanExpressionTransformerTestCase.java
@@ -0,0 +1,57 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.schema.expressiontransforms;
+
+import com.yahoo.searchlib.rankingexpression.RankingExpression;
+import com.yahoo.searchlib.rankingexpression.evaluation.MapContext;
+import com.yahoo.searchlib.rankingexpression.evaluation.MapTypeContext;
+import com.yahoo.searchlib.rankingexpression.transform.TransformContext;
+import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.util.Map;
+
+/**
+ * @author bratseth
+ */
+public class BooleanExpressionTransformerTestCase {
+
+    @Test
+    public void testTransformer() throws Exception {
+        assertTransformed("if (a, b, false)", "a && b");
+        assertTransformed("if (a, true, b)", "a || b");
+        assertTransformed("if (a, true, b + c)", "a || b + c");
+        assertTransformed("if (c + a, true, b)", "c + a || b");
+        assertTransformed("if (c + a, true, b + c)", "c + a || b + c");
+        assertTransformed("if (a + b, true, if (c - d * e, f, false))", "a + b || c - d * e && f");
+        assertTransformed("if (a, true, if (b, c, false))", "a || b && c");
+        assertTransformed("if (a + b, true, if (if (c, d, false), e * f - g, false))", "a + b || c && d && e * f - g");
+        assertTransformed("if(1 - 1, true, 1 - 1)", "1 - 1 || 1 - 1");
+    }
+
+    @Test
+    public void testIt() throws Exception {
+        assertTransformed("if(1 - 1, true, 1 - 1)", "1 - 1 || 1 - 1");
+    }
+
+    private void assertTransformed(String expected, String input) throws Exception {
+        var transformedExpression = new BooleanExpressionTransformer()
+                                            .transform(new RankingExpression(input),
+                                                       new TransformContext(Map.of(), new MapTypeContext()));
+
+        assertEquals(new RankingExpression(expected), transformedExpression, "Transformed as expected");
+
+        MapContext context = contextWithSingleLetterVariables();
+        var inputExpression = new RankingExpression(input);
+        assertEquals(inputExpression.evaluate(new MapContext()).asBoolean(),
+                     transformedExpression.evaluate(new MapContext()).asBoolean(),
+                     "Transform and original input are equivalent");
+    }
+
+    private MapContext contextWithSingleLetterVariables() {
+        var context = new MapContext();
+        for (int i = 0; i < 26; i++)
+            context.put(Character.toString(i + 97), Math.floorMod(i, 2));
+        return context;
+    }
+
+}
-- 
cgit v1.2.3