Skip to content

Commit

Permalink
Adding IT tests for permissive fields, moving field validation to Ope…
Browse files Browse the repository at this point in the history
…nSearchFunctions.

Signed-off-by: forestmvey <[email protected]>
  • Loading branch information
forestmvey committed Nov 9, 2022
1 parent dee89e8 commit 8144b38
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 66 deletions.
3 changes: 2 additions & 1 deletion core/src/main/java/org/opensearch/sql/analysis/Analyzer.java
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
import org.opensearch.sql.expression.aggregation.NamedAggregator;
import org.opensearch.sql.expression.function.BuiltinFunctionRepository;
import org.opensearch.sql.expression.function.FunctionName;
import org.opensearch.sql.expression.function.OpenSearchFunctions;
import org.opensearch.sql.expression.function.TableFunctionImplementation;
import org.opensearch.sql.expression.parse.ParseExpression;
import org.opensearch.sql.planner.logical.LogicalAD;
Expand Down Expand Up @@ -218,7 +219,7 @@ public LogicalPlan visitFilter(Filter node, AnalysisContext context) {
LogicalPlan child = node.getChild().get(0).accept(this, context);
Expression condition = expressionAnalyzer.analyze(node.getCondition(), context);

RelevanceQueryAnalyzer.validateFieldList((FunctionExpression)condition, context);
OpenSearchFunctions.validateFieldList((FunctionExpression)condition, context);

ExpressionReferenceOptimizer optimizer =
new ExpressionReferenceOptimizer(expressionAnalyzer.getRepository(), child);
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,14 @@
import static org.opensearch.sql.data.type.ExprCoreType.STRUCT;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import java.util.List;
import java.util.stream.Collectors;
import lombok.experimental.UtilityClass;
import org.opensearch.sql.analysis.AnalysisContext;
import org.opensearch.sql.analysis.TypeEnvironment;
import org.opensearch.sql.analysis.symbol.Namespace;
import org.opensearch.sql.analysis.symbol.Symbol;
import org.opensearch.sql.common.utils.StringUtils;
import org.opensearch.sql.data.model.ExprValue;
import org.opensearch.sql.data.type.ExprCoreType;
import org.opensearch.sql.data.type.ExprType;
Expand All @@ -36,14 +40,53 @@ public class OpenSearchFunctions {
BuiltinFunctionName.QUERY_STRING.name()
);

/**
* Check if supplied function name is valid SingleFieldRelevanceFunction.
* @param funcName : Name of function
* @return : True if function is single-field function
*/
public static boolean isSingleFieldFunction(String funcName) {
return singleFieldFunctionNames.contains(funcName.toUpperCase());
}

/**
* Check if supplied function name is valid MultiFieldRelevanceFunction.
* @param funcName : Name of function
* @return : True if function is multi-field function
*/
public static boolean isMultiFieldFunction(String funcName) {
return multiFieldFunctionNames.contains(funcName.toUpperCase());
}

/**
* Verify if function queries fields available in type environment.
* @param node : Function used in query.
* @param context : Context of fields querying.
*/
public static void validateFieldList(FunctionExpression node, AnalysisContext context) {
String funcName = node.getFunctionName().toString();

TypeEnvironment typeEnv = context.peek();
if (isSingleFieldFunction(funcName)) {
node.getArguments().stream().map(NamedArgumentExpression.class::cast).filter(arg ->
((arg.getArgName().equals("field")
&& !arg.getValue().toString().contains("*"))
)).findFirst().ifPresent(arg ->
typeEnv.resolve(new Symbol(Namespace.FIELD_NAME,
StringUtils.unquoteText(arg.getValue().toString()))
)
);
} else if (isMultiFieldFunction(funcName)) {
node.getArguments().stream().map(NamedArgumentExpression.class::cast).filter(arg ->
arg.getArgName().equals("fields")
).findFirst().ifPresent(fields ->
fields.getValue().valueOf(null).tupleValue()
.entrySet().stream().filter(k -> !(k.getKey().contains("*"))
).forEach(key -> typeEnv.resolve(new Symbol(Namespace.FIELD_NAME, key.getKey())))
);
}
}

/**
* Add functions specific to OpenSearch to repository.
*/
Expand Down
11 changes: 11 additions & 0 deletions integ-test/src/test/java/org/opensearch/sql/sql/MatchIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.json.JSONObject;
import org.junit.Test;
import org.opensearch.sql.legacy.SQLIntegTestCase;
import org.opensearch.sql.legacy.utils.StringUtils;

public class MatchIT extends SQLIntegTestCase {
@Override
Expand All @@ -35,4 +36,14 @@ public void match_in_having() throws IOException {
verifySchema(result, schema("lastname", "text"));
verifyDataRows(result, rows("Bates"));
}

@Test
public void missing_field_test() {
String query = StringUtils.format("SELECT * FROM %s WHERE match(invalid, 'Bates')", TEST_INDEX_ACCOUNT);
final RuntimeException exception =
expectThrows(RuntimeException.class, () -> executeJdbcRequest(query));
assertTrue(exception.getMessage()
.contains("can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env") &&
exception.getMessage().contains("SemanticCheckException"));
}
}
16 changes: 6 additions & 10 deletions integ-test/src/test/java/org/opensearch/sql/sql/QueryStringIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,12 @@

package org.opensearch.sql.sql;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BEER;

import java.io.IOException;
import org.json.JSONObject;
import org.junit.Test;
import org.opensearch.client.ResponseException;
import org.opensearch.sql.exception.SemanticCheckException;
import org.opensearch.sql.legacy.SQLIntegTestCase;
import org.opensearch.sql.legacy.TestUtils;
import org.opensearch.sql.legacy.TestsConstants;
import org.opensearch.sql.legacy.utils.StringUtils;

public class QueryStringIT extends SQLIntegTestCase {
Expand Down Expand Up @@ -74,10 +69,11 @@ public void wildcard_test() throws IOException {

@Test
public void missing_field_test() {
String query = StringUtils.format("SELECT * FROM %s WHERE query_string([missing], 'beer')", TEST_INDEX_BEER);
final SemanticCheckException exception =
expectThrows(SemanticCheckException.class, () -> executeJdbcRequest(query));
assertEquals("can't resolve Symbol(namespace=FIELD_NAME, name=missing) in type env",
exception.getMessage());
String query = StringUtils.format("SELECT * FROM %s WHERE query_string([invalid], 'beer')", TEST_INDEX_BEER);
final RuntimeException exception =
expectThrows(RuntimeException.class, () -> executeJdbcRequest(query));
assertTrue(exception.getMessage()
.contains("can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env") &&
exception.getMessage().contains("SemanticCheckException"));
}
}

0 comments on commit 8144b38

Please sign in to comment.