Adding field comparison in Legacy Engine

integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/MethodQueryIT.java

@@ -22,6 +22,7 @@
 import java.io.IOException;
 import java.util.Locale;
 import org.junit.Assert;
+import org.junit.Assume;
 import org.junit.Test;
 
 /**
@@ -144,4 +145,30 @@ public void matchPhraseQueryTest() throws IOException {
     Assert.assertThat(result,
         containsString("{\"match_phrase\":{\"address\":{\"query\":\"671 Bristol Street\""));
   }
+
+  @Test
+  public void testFieldToFieldComparison() throws IOException {
+    Assume.assumeFalse(isNewQueryEngineEabled());
+    String result =  explainQuery("select * from " + TestsConstants.TEST_INDEX_ACCOUNT
+        + " where age > account_number");
+
+    Assert.assertThat(result,
+        containsString("{\"script\":{\"script\":{\"source\":\"doc['age'].value "
+            + "> doc['account_number'].value\",\"lang\":\"painless\"}"));
+
+  }
+
+  @Test
+  public void testFieldToFieldComparisonWithExtraClause() throws IOException {
+    Assume.assumeFalse(isNewQueryEngineEabled());
+    String result =  explainQuery("select * from " + TestsConstants.TEST_INDEX_ACCOUNT
+        + " where age > account_number AND age in (1)");
+
+    Assert.assertThat(result,
+        containsString("{\"script\":{\"script\":{\"source\":\"doc['age'].value "
+            + "> doc['account_number'].value\",\"lang\":\"painless\"}"));
+    Assert.assertThat(result,
+        containsString("{\"term\":{\"age\":{\"value\":1,\"boost\":1.0}}}"));
+
+  }
 }

legacy/src/main/java/com/amazon/opendistroforelasticsearch/sql/legacy/domain/Where.java

@@ -31,16 +31,34 @@ public static Where newInstance() {
         return new Where(CONN.AND);
     }
 
+    public static Where newInstance(boolean isJoin) {
+        return new Where(CONN.AND, isJoin);
+    }
+
     private LinkedList<Where> wheres = new LinkedList<>();
 
     protected CONN conn;
+    protected final boolean isJoin;
 
     public Where(String connStr) {
-        this.conn = CONN.valueOf(connStr.toUpperCase());
+        this(connStr, false);
+    }
+
+    public Where(String connStr, boolean isJoin) {
+        this(CONN.valueOf(connStr.toUpperCase()), isJoin);
     }
 
     public Where(CONN conn) {
+        this(conn, false);
+    }
+
+    public Where(CONN conn, boolean isJoin) {
         this.conn = conn;
+        this.isJoin=isJoin;
+    }
+
+    public boolean isJoin() {
+        return isJoin;
     }
 
     public void addWhere(Where where) {

legacy/src/main/java/com/amazon/opendistroforelasticsearch/sql/legacy/parser/SqlParser.java

@@ -448,7 +448,7 @@ private JoinSelect createBasicJoinSelectAccordingToTableSource(SQLJoinTableSourc
             throws SqlParseException {
         JoinSelect joinSelect = new JoinSelect();
         if (joinTableSource.getCondition() != null) {
-            Where where = Where.newInstance();
+            Where where = Where.newInstance(true);
             WhereParser whereParser = new WhereParser(this, joinTableSource.getCondition());
             whereParser.parseWhere(joinTableSource.getCondition(), where);
             joinSelect.setConnectedWhere(where);

legacy/src/main/java/com/amazon/opendistroforelasticsearch/sql/legacy/parser/WhereParser.java

@@ -214,7 +214,7 @@ private void routeCond(SQLBinaryOpExpr bExpr, SQLExpr sub, Where where) throws S
         if (sub instanceof SQLBinaryOpExpr && !isCond((SQLBinaryOpExpr) sub)) {
             SQLBinaryOpExpr binarySub = (SQLBinaryOpExpr) sub;
             if (binarySub.getOperator().priority != bExpr.getOperator().priority) {
-                Where subWhere = new Where(bExpr.getOperator().name);
+                Where subWhere = new Where(bExpr.getOperator().name, where.isJoin());
                 where.addWhere(subWhere);
                 parseWhere(binarySub, subWhere);
             } else {
@@ -291,7 +291,7 @@ private void explainCond(String opear, SQLExpr expr, Where where) throws SqlPars
                     condition = new Condition(Where.CONN.valueOf(opear), soExpr.getLeft().toString(), soExpr.getLeft(),
                             soExpr.getOperator().name, parseValue(soExpr.getRight()), soExpr.getRight(), childrenType);
                 } else {
-                    SQLMethodInvokeExpr sqlMethodInvokeExpr = parseSQLBinaryOpExprWhoIsConditionInWhere(soExpr);
+                    SQLMethodInvokeExpr sqlMethodInvokeExpr = parseSQLBinaryOpExprWhoIsConditionInWhere(soExpr, where.isJoin());
                     if (sqlMethodInvokeExpr == null) {
                         condition = new Condition(Where.CONN.valueOf(opear), soExpr.getLeft().toString(),
                                 soExpr.getLeft(), soExpr.getOperator().name, parseValue(soExpr.getRight()),
@@ -536,10 +536,20 @@ private MethodField parseSQLCastExprWithFunctionInWhere(SQLCastExpr soExpr) thro
         );
     }
 
-    private SQLMethodInvokeExpr parseSQLBinaryOpExprWhoIsConditionInWhere(SQLBinaryOpExpr soExpr)
+    private SQLMethodInvokeExpr parseSQLBinaryOpExprWhoIsConditionInWhere(SQLBinaryOpExpr soExpr,
+        boolean join)
             throws SqlParseException {
 
-        if (bothSideAreNotFunction(soExpr) && bothSidesAreNotCast(soExpr)) {
+        if (
+            // if one of the two side is a method, it has to be resolved as script
+            neitherSideIsFunction(soExpr)
+            // if one of the two side is a cast, it has to be resolved as script
+            && neitherSidesIsCast(soExpr)
+            // if both sides are field, we cannot use the QueryRange (which would be more efficient
+            // `field > integer` comparisons) but ES doesn't allow comparing two fields and
+            // we must use a SCRIPT
+            && (!bothSidesAreFieldIdentifier(soExpr) || join)
+        ) {
             return null;
         }
 
@@ -608,14 +618,22 @@ private SQLMethodInvokeExpr parseSQLBinaryOpExprWhoIsConditionInWhere(SQLBinaryO
 
     }
 
-    private Boolean bothSideAreNotFunction(SQLBinaryOpExpr soExpr) {
+    private Boolean neitherSideIsFunction(SQLBinaryOpExpr soExpr) {
         return !(soExpr.getLeft() instanceof SQLMethodInvokeExpr || soExpr.getRight() instanceof SQLMethodInvokeExpr);
     }
 
-    private Boolean bothSidesAreNotCast(SQLBinaryOpExpr soExpr) {
+    private Boolean neitherSidesIsCast(SQLBinaryOpExpr soExpr) {
         return !(soExpr.getLeft() instanceof SQLCastExpr || soExpr.getRight() instanceof SQLCastExpr);
     }
 
+    private Boolean bothSidesAreFieldIdentifier(SQLBinaryOpExpr soExpr) {
+        return (soExpr.getLeft() instanceof SQLIdentifierExpr && soExpr.getRight() instanceof SQLIdentifierExpr)
+            // "missing" is a SQLIdentifier but not a Field Identifier.
+            // We might want to have a subclass for "missing" or for "fieldIdentifier" or
+            // change type altogether to avoid conflicts
+            && !"missing".equalsIgnoreCase(((SQLIdentifierExpr) soExpr.getRight()).getLowerName());
+    }
+
     private Object[] getMethodValuesWithSubQueries(SQLMethodInvokeExpr method) throws SqlParseException {
         List<Object> values = new ArrayList<>();
         for (SQLExpr innerExpr : method.getParameters()) {