Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Manual Backport] Improve error handling for some more edge cases (#3080) #3115

Merged
merged 2 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.sql.legacy;

import java.io.IOException;
import java.util.Locale;
import org.apache.http.ParseException;
import org.apache.http.util.EntityUtils;
import org.json.JSONObject;
import org.junit.Assert;
import org.opensearch.client.ResponseException;

/** Tests for clean handling of various types of invalid queries */
public class MalformedQueryIT extends SQLIntegTestCase {
@Override
protected void init() throws Exception {
loadIndex(Index.BANK);
loadIndex(Index.BANK_TWO);
}

public void testJoinWithInvalidCondition() throws IOException, ParseException {
ResponseException result =
assertThrows(
"Expected Join query with malformed 'ON' to raise error, but didn't",
ResponseException.class,
() ->
executeQuery(
String.format(
Locale.ROOT,
"SELECT a.firstname, b.age FROM %s AS a INNER JOIN %s AS b %%"
+ " a.account_number=b.account_number",
TestsConstants.TEST_INDEX_BANK,
TestsConstants.TEST_INDEX_BANK_TWO)));
var errMsg = new JSONObject(EntityUtils.toString(result.getResponse().getEntity()));

Assert.assertEquals("SqlParseException", errMsg.getJSONObject("error").getString("type"));
Assert.assertEquals(400, errMsg.getInt("status"));
}

public void testWrappedWildcardInSubquery() throws IOException, ParseException {
ResponseException result =
assertThrows(
"Expected wildcard subquery to raise error, but didn't",
ResponseException.class,
() ->
executeQuery(
String.format(
Locale.ROOT,
"SELECT a.first_name FROM %s AS a WHERE a.age IN (SELECT age FROM"
+ " `opensearch-sql_test_index_*` WHERE age > 30)",
TestsConstants.TEST_INDEX_BANK,
TestsConstants.TEST_INDEX_BANK_TWO)));
var errMsg = new JSONObject(EntityUtils.toString(result.getResponse().getEntity()));
System.err.println("Full response: " + errMsg);

Assert.assertEquals("IndexNotFoundException", errMsg.getJSONObject("error").getString("type"));
Assert.assertEquals(404, errMsg.getInt("status"));
}

public void testUnwrappedWildcardInSubquery() throws IOException, ParseException {
ResponseException result =
assertThrows(
"Expected wildcard subquery to raise error, but didn't",
ResponseException.class,
() ->
executeQuery(
String.format(
Locale.ROOT,
"SELECT a.first_name FROM %s AS a WHERE a.age IN (SELECT age FROM * WHERE"
+ " age > 30)",
TestsConstants.TEST_INDEX_BANK,
TestsConstants.TEST_INDEX_BANK_TWO)));
var errMsg = new JSONObject(EntityUtils.toString(result.getResponse().getEntity()));
System.err.println("Full response: " + errMsg);

Assert.assertEquals("IndexNotFoundException", errMsg.getJSONObject("error").getString("type"));
Assert.assertEquals(404, errMsg.getInt("status"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static org.opensearch.sql.legacy.domain.IndexStatement.StatementType;
import static org.opensearch.sql.legacy.utils.Util.toSqlExpr;

import com.alibaba.druid.sql.ast.SQLExpr;
import com.alibaba.druid.sql.ast.expr.SQLAggregateExpr;
import com.alibaba.druid.sql.ast.expr.SQLAllColumnExpr;
import com.alibaba.druid.sql.ast.expr.SQLMethodInvokeExpr;
Expand Down Expand Up @@ -86,7 +87,14 @@ public static QueryAction create(Client client, QueryActionRequest request)

switch (getFirstWord(sql)) {
case "SELECT":
SQLQueryExpr sqlExpr = (SQLQueryExpr) toSqlExpr(sql);
SQLExpr rawExpr = toSqlExpr(sql);
if (!(rawExpr instanceof SQLQueryExpr)) {
throw new SqlParseException(
"Expected a query expression, but found a "
+ rawExpr.getClass().getSimpleName()
+ ". The query is not runnable.");
}
SQLQueryExpr sqlExpr = (SQLQueryExpr) rawExpr;

RewriteRuleExecutor<SQLQueryExpr> ruleExecutor =
RewriteRuleExecutor.builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.util.Map;
import java.util.Optional;
import java.util.stream.Stream;
import org.opensearch.index.IndexNotFoundException;
import org.opensearch.sql.legacy.esdomain.LocalClusterState;
import org.opensearch.sql.legacy.esdomain.mapping.FieldMappings;
import org.opensearch.sql.legacy.esdomain.mapping.IndexMappings;
Expand Down Expand Up @@ -122,7 +123,23 @@ public boolean visit(SQLIdentifierExpr expr) {
String fullFieldName = arr[1];

String index = curScope().getAliases().get(alias);
if (index == null) {
throw new IndexNotFoundException(
String.format(
"The requested table '%s' does not correspond to any known index. Only indices or"
+ " table aliases are allowed.",
alias.replaceFirst("_\\d+$", "")));
}

FieldMappings fieldMappings = curScope().getMapper().mapping(index);
if (fieldMappings == null) {
throw new IndexNotFoundException(
String.format(
"The index '%s' could not be found. Note that wildcard indices are not permitted"
+ " in SQL.",
index));
}

if (fieldMappings.has(fullFieldName)) {
source = fieldMappings.mapping(fullFieldName);
} else {
Expand Down
Loading