Fix improper handling of the table expression (#89)

* Throws ParserException for case, when the query was incorrect, allowing to identify it as a client error
* Improved log message to differentiate server and client side errors in log

src/main/java/com/amazon/opendistroforelasticsearch/sql/plugin/RestSqlAction.java

@@ -120,12 +120,14 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
             }
         } catch (Exception e) {
             if (isClientError(e)) {
+                LOG.error(String.format(Locale.ROOT, "[%s] Client side error during query execution", LogUtils.getRequestId()), e);
                 Metrics.getInstance().getNumericalMetric(MetricName.FAILED_REQ_COUNT_CUS).increment();
+                return reportError(e, BAD_REQUEST);
             } else {
+                LOG.error(String.format(Locale.ROOT, "[%s] Server side error during query execution", LogUtils.getRequestId()), e);
                 Metrics.getInstance().getNumericalMetric(MetricName.FAILED_REQ_COUNT_SYS).increment();
+                return reportError(e, SERVICE_UNAVAILABLE);
             }
-            LOG.error(String.format(Locale.ROOT, "[%s] Failed during query execution", LogUtils.getRequestId()), e);
-            return reportError(e, isClientError(e) ? BAD_REQUEST : SERVICE_UNAVAILABLE);
         }
     }
 

src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/matchtoterm/TermFieldRewriter.java

@@ -15,6 +15,7 @@
 
 package com.amazon.opendistroforelasticsearch.sql.rewriter.matchtoterm;
 
+import com.alibaba.druid.sql.ast.SQLExpr;
 import com.alibaba.druid.sql.ast.expr.SQLBinaryOpExpr;
 import com.alibaba.druid.sql.ast.expr.SQLBinaryOperator;
 import com.alibaba.druid.sql.ast.expr.SQLIdentifierExpr;
@@ -28,6 +29,7 @@
 import com.alibaba.druid.sql.dialect.mysql.ast.expr.MySqlSelectGroupByExpr;
 import com.alibaba.druid.sql.dialect.mysql.ast.statement.MySqlSelectQueryBlock;
 import com.alibaba.druid.sql.dialect.mysql.visitor.MySqlASTVisitorAdapter;
+import com.alibaba.druid.sql.parser.ParserException;
 import com.amazon.opendistroforelasticsearch.sql.esdomain.LocalClusterState;
 import org.elasticsearch.client.Client;
 import org.json.JSONObject;
@@ -147,24 +149,32 @@ public boolean visit(SQLIdentifierExpr expr) {
 
     public void collect(SQLTableSource tableSource, Map<String, String> indexToType,  Map<String, String> aliases) {
         if (tableSource instanceof SQLExprTableSource) {
-            String table = null;
-            if (((SQLExprTableSource) tableSource).getExpr() instanceof SQLIdentifierExpr) {
-                table = ((SQLIdentifierExpr) ((SQLExprTableSource) tableSource).getExpr()).getName();
-                indexToType.put(
-                    table,
-                    null
-                );
-            } else if (((SQLExprTableSource) tableSource).getExpr() instanceof SQLBinaryOpExpr) {
-                table = ((SQLIdentifierExpr)((SQLBinaryOpExpr) ((SQLExprTableSource) tableSource).getExpr()).getLeft()).getName();
-                indexToType.put(
-                    table,
-                    ((SQLIdentifierExpr)((SQLBinaryOpExpr) ((SQLExprTableSource) tableSource).getExpr()).getRight()).getName()
-                );
+
+            String tableName = null;
+            SQLExprTableSource sqlExprTableSource = (SQLExprTableSource) tableSource;
+
+            if (sqlExprTableSource.getExpr() instanceof SQLIdentifierExpr) {
+                SQLIdentifierExpr sqlIdentifier = (SQLIdentifierExpr) sqlExprTableSource.getExpr();
+                tableName = sqlIdentifier.getName();
+                indexToType.put(tableName, null);
+            } else if (sqlExprTableSource.getExpr() instanceof SQLBinaryOpExpr) {
+                SQLBinaryOpExpr sqlBinaryOpExpr = (SQLBinaryOpExpr) sqlExprTableSource.getExpr();
+                tableName = ((SQLIdentifierExpr) sqlBinaryOpExpr.getLeft()).getName();
+                SQLExpr rightSideOfExpression = sqlBinaryOpExpr.getRight();
+
+                // This assumes that right side of the expression is different name in query
+                if (rightSideOfExpression instanceof SQLIdentifierExpr) {
+                    SQLIdentifierExpr right = (SQLIdentifierExpr) rightSideOfExpression;
+                    indexToType.put(tableName, right.getName());
+                } else {
+                    throw new ParserException("Right side of the expression [" + rightSideOfExpression.toString() +
+                            "] is expected to be an identifier");
+                }
             }
             if (tableSource.getAlias() != null) {
-                aliases.put(tableSource.getAlias(), table);
+                aliases.put(tableSource.getAlias(), tableName);
             } else {
-                aliases.put(table, table);
+                aliases.put(tableName, tableName);
             }
 
         } else if (tableSource instanceof SQLJoinTableSource) {

src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/regression/RegressionTest.java

@@ -0,0 +1,42 @@
+/*
+ *   Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ *   Licensed under the Apache License, Version 2.0 (the "License").
+ *   You may not use this file except in compliance with the License.
+ *   A copy of the License is located at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *   or in the "license" file accompanying this file. This file is distributed
+ *   on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ *   express or implied. See the License for the specific language governing
+ *   permissions and limitations under the License.
+ */
+
+package com.amazon.opendistroforelasticsearch.sql.unittest.regression;
+
+import com.alibaba.druid.sql.parser.ParserException;
+import com.amazon.opendistroforelasticsearch.sql.exception.SqlParseException;
+import com.amazon.opendistroforelasticsearch.sql.query.ESActionFactory;
+import org.elasticsearch.client.Client;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import java.sql.SQLFeatureNotSupportedException;
+
+/**
+ * Special cases for queries that had broken our parser in past.
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class RegressionTest {
+
+    @Mock
+    Client client;
+
+    @Test(expected = ParserException.class)
+    public void missingWhereAndFieldName() throws SQLFeatureNotSupportedException, SqlParseException {
+        ESActionFactory.create(client, "select * from products like 'SomeProduct*' limit 10");
+    }
+}