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

Added Query Function As Query_string Function #143

Merged
merged 1 commit into from
Nov 2, 2022
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
4 changes: 4 additions & 0 deletions core/src/main/java/org/opensearch/sql/expression/DSL.java
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,10 @@ public FunctionExpression simple_query_string(Expression... args) {
return compile(BuiltinFunctionName.SIMPLE_QUERY_STRING, args);
}

public FunctionExpression query(Expression... args) {
return compile(BuiltinFunctionName.QUERY, args);
}

public FunctionExpression query_string(Expression... args) {
return compile(BuiltinFunctionName.QUERY_STRING, args);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public void register(BuiltinFunctionRepository repository) {
repository.register(match());
repository.register(multi_match());
repository.register(simple_query_string());
repository.register(query());
repository.register(query_string());
// Register MATCHPHRASE as MATCH_PHRASE as well for backwards
// compatibility.
Expand Down Expand Up @@ -68,6 +69,11 @@ private static FunctionResolver simple_query_string() {
return new RelevanceFunctionResolver(funcName, STRUCT);
}

private static FunctionResolver query() {
FunctionName funcName = BuiltinFunctionName.QUERY.getName();
return new RelevanceFunctionResolver(funcName, STRING);
}

private static FunctionResolver query_string() {
FunctionName funcName = BuiltinFunctionName.QUERY_STRING.getName();
return new RelevanceFunctionResolver(funcName, STRUCT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,15 @@ void simple_query_string_expression_two_fields() {
AstDSL.unresolvedArg("query", stringLiteral("sample query"))));
}

@Test
void query_expression() {
assertAnalyzeEqual(
dsl.query(
dsl.namedArgument("query", DSL.literal("field:query"))),
AstDSL.function("query",
AstDSL.unresolvedArg("query", stringLiteral("field:query"))));
}

@Test
void query_string_expression() {
assertAnalyzeEqual(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,13 @@ void simple_query_string() {
expr.toString());
}

@Test
void query() {
FunctionExpression expr = dsl.query(query);
assertEquals(String.format("query(query=%s)", query.getValue()),
expr.toString());
}

@Test
void query_string() {
FunctionExpression expr = dsl.query_string(fields, query);
Expand Down
61 changes: 61 additions & 0 deletions docs/user/dql/functions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3017,6 +3017,67 @@ Another example to show how to set custom values for the optional parameters::
+------+--------------------------+----------------------+


QUERY
-----

Description
>>>>>>>>>>>

``query("query_expression" [, option=<option_value>]*)``

The `query` function is an alternative syntax to the `query_string`_ function. It maps to the query_string query used in search engine, to return the documents that match a provided text, number, date or boolean value with a given query expression.
``query_expression`` must be a string provided in Lucene query string syntax. Please refer to examples below:

| ``query('Tags:taste OR Body:taste', ...)``
| ``query("Tags:taste AND Body:taste", ...)``

Available parameters include:

- analyzer
- escape
- allow_leading_wildcard
- analyze_wildcard
- auto_generate_synonyms_phrase_query
- boost
- default_operator
- enable_position_increments
- fuzziness
- fuzzy_max_expansions
- fuzzy_prefix_length
- fuzzy_transpositions
- fuzzy_rewrite
- tie_breaker
- lenient
- type
- max_determinized_states
- minimum_should_match
- quote_analyzer
- phrase_slop
- quote_field_suffix
- rewrite
- time_zone

Example with only ``query_expressions``, and all other parameters are set default values::

os> select * from books where query('title:Pooh House');
fetched rows / total rows = 2/2
+------+--------------------------+----------------------+
| id | title | author |
|------+--------------------------+----------------------|
| 1 | The House at Pooh Corner | Alan Alexander Milne |
| 2 | Winnie-the-Pooh | Alan Alexander Milne |
+------+--------------------------+----------------------+

Another example to show how to set custom values for the optional parameters::

os> select * from books where query('title:Pooh House', default_operator='AND');
fetched rows / total rows = 1/1
+------+--------------------------+----------------------+
| id | title | author |
|------+--------------------------+----------------------|
| 1 | The House at Pooh Corner | Alan Alexander Milne |
+------+--------------------------+----------------------+

HIGHLIGHT
------------

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public void queryTest() throws IOException {
"select address from %s where query('address:880 Holmes Lane') limit 3",
TestsConstants.TEST_INDEX_ACCOUNT));
Assert.assertThat(result,
containsString("query_string\":{\"query\":\"address:880 Holmes Lane"));
containsString("query_string\\\":{\\\"query\\\":\\\"address:880 Holmes Lane"));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this wasn't working?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were some issues with escaped characters causing it to fail.


}

Expand Down
84 changes: 84 additions & 0 deletions integ-test/src/test/java/org/opensearch/sql/sql/QueryIT.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.sql.sql;

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.sql.legacy.SQLIntegTestCase;

public class QueryIT extends SQLIntegTestCase {
GabeFernandez310 marked this conversation as resolved.
Show resolved Hide resolved
@Override
public void init() throws IOException {
loadIndex(Index.BEER);
}

@Test
public void all_fields_test() throws IOException {
String query = "SELECT * FROM "
+ TEST_INDEX_BEER + " WHERE query('*:taste')";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the wildcard syntax doesn't work? Or does it work with just *?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought so too... I thought we determined that they didn't work when we had our 1:1. I played around with it trying different combinations...

The following all seem to work when passed in as the query for query_string:

"* : taste"
"* : ?aste"
"Tags : *aste"
"Tags : Ta?te"
"Tags : Ta*te"

I tried these according to this documentation for Lucene, but it doesn't seem to follow it exactly since it also allows * and ? to be the first character of a search. Should we add tests for all of these cases as well? I can't find a specific Doc to reference for what should/shouldn't be supported in the query so I'm not really sure what other behaviour we should be testing for that we might be missing.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added Unit Tests in f2d77c6

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was more asking about the field name (* or Tags). It seems like those two cases are fine, but T?g* is not?

Honestly, the checks are handled by OpenSearch not us... so what additional tests are going to do is test another system. We can consider covering some of the general cases, but we do not need to be exhaustive here.

JSONObject result = executeJdbcRequest(query);
assertEquals(16, result.getInt("total"));
}

@Test
public void mandatory_params_test() throws IOException {
forestmvey marked this conversation as resolved.
Show resolved Hide resolved
String query = "SELECT Id FROM "
+ TEST_INDEX_BEER + " WHERE query('Tags:taste OR Body:taste')";
JSONObject result = executeJdbcRequest(query);
assertEquals(16, result.getInt("total"));
}

@Test
public void all_params_test() throws IOException {
String query = "SELECT Id FROM " + TEST_INDEX_BEER
+ " WHERE query('Tags:taste', escape=false,"
+ "allow_leading_wildcard=true, enable_position_increments=true,"
+ "fuzziness= 1, fuzzy_rewrite='constant_score', max_determinized_states = 10000,"
+ "analyzer='standard', analyze_wildcard = false, quote_field_suffix = '.exact',"
+ "auto_generate_synonyms_phrase_query=true, boost = 0.77,"
+ "quote_analyzer='standard', phrase_slop=0, rewrite='constant_score', type='best_fields',"
+ "tie_breaker=0.3, time_zone='Canada/Pacific', default_operator='or',"
+ "fuzzy_transpositions = false, lenient = true, fuzzy_max_expansions = 25,"
+ "minimum_should_match = '2<-25% 9<-3', fuzzy_prefix_length = 7);";
JSONObject result = executeJdbcRequest(query);
assertEquals(8, result.getInt("total"));
}

@Test
public void wildcard_test() throws IOException {
String query1 = "SELECT Id FROM "
+ TEST_INDEX_BEER + " WHERE query('Tags:taste')";
JSONObject result1 = executeJdbcRequest(query1);
String query2 = "SELECT Id FROM "
+ TEST_INDEX_BEER + " WHERE query('*:taste')";
JSONObject result2 = executeJdbcRequest(query2);
assertNotEquals(result2.getInt("total"), result1.getInt("total"));

String query3 = "SELECT Id FROM " + TEST_INDEX_BEER
+ " WHERE query('Tags:tas*');";
JSONObject result3 = executeJdbcRequest(query3);
assertEquals(8, result3.getInt("total"));

String query4 = "SELECT Id FROM " + TEST_INDEX_BEER
+ " WHERE query('Tags:tas?e');";
JSONObject result4 = executeJdbcRequest(query3);
assertEquals(8, result4.getInt("total"));
}

@Test
public void query_string_and_query_return_the_same_results_test() throws IOException {
String query1 = "SELECT Id FROM "
+ TEST_INDEX_BEER + " WHERE query('Tags:taste')";
JSONObject result1 = executeJdbcRequest(query1);
String query2 = "SELECT Id FROM "
+ TEST_INDEX_BEER + " WHERE query_string(['Tags'],'taste')";
JSONObject result2 = executeJdbcRequest(query2);
assertEquals(result2.getInt("total"), result1.getInt("total"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance.MatchPhraseQuery;
import org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance.MatchQuery;
import org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance.MultiMatchQuery;
import org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance.QueryQuery;
import org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance.QueryStringQuery;
import org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance.SimpleQueryStringQuery;
import org.opensearch.sql.opensearch.storage.serialization.ExpressionSerializer;
Expand All @@ -60,7 +61,7 @@ public class FilterQueryBuilder extends ExpressionNodeVisitor<QueryBuilder, Obje
.put(BuiltinFunctionName.MATCH.getName(), new MatchQuery())
.put(BuiltinFunctionName.MATCH_PHRASE.getName(), new MatchPhraseQuery())
.put(BuiltinFunctionName.MATCHPHRASE.getName(), new MatchPhraseQuery())
.put(BuiltinFunctionName.QUERY.getName(), new MatchQuery())
.put(BuiltinFunctionName.QUERY.getName(), new QueryQuery())
.put(BuiltinFunctionName.MATCH_QUERY.getName(), new MatchQuery())
.put(BuiltinFunctionName.MATCHQUERY.getName(), new MatchQuery())
.put(BuiltinFunctionName.MULTI_MATCH.getName(), new MultiMatchQuery())
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance;

import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;
import org.opensearch.index.query.QueryBuilder;
import org.opensearch.sql.common.antlr.SyntaxCheckException;
import org.opensearch.sql.exception.SemanticCheckException;
import org.opensearch.sql.expression.FunctionExpression;
import org.opensearch.sql.expression.NamedArgumentExpression;

/**
* Base class to represent relevance queries that search multiple fields.
*
* @param <T> The builder class for the OpenSearch query.
*/
abstract class NoFieldQuery<T extends QueryBuilder> extends RelevanceQuery<T> {
public NoFieldQuery(Map<String, QueryBuilderStep<T>> queryBuildActions) {
super(queryBuildActions);
}

@Override
protected void ignoreArguments(List<NamedArgumentExpression> arguments) {
arguments.removeIf(a -> a.getArgName().equalsIgnoreCase("query"));
}

@Override
protected void checkValidArguments(String argNormalized, T queryBuilder) {
if (!getQueryBuildActions().containsKey(argNormalized)) {
throw new SemanticCheckException(
String.format("Parameter %s is invalid for %s function.",
argNormalized, getQueryName()));
}
}
/**
* Override build function because RelevanceQuery requires 2 fields,
* but NoFieldQuery must have no fields.
*
* @param func : Contains function name and passed in arguments.
* @return : QueryBuilder object
*/

@Override
public QueryBuilder build(FunctionExpression func) {
var arguments = func.getArguments().stream().map(
a -> (NamedArgumentExpression) a).collect(Collectors.toList());
if (arguments.size() < 1) {
throw new SyntaxCheckException(String.format(
"%s requires at least one parameter", func.getFunctionName()));
}

return loadArguments(arguments);
}


@Override
public T createQueryBuilder(List<NamedArgumentExpression> arguments) {
// Extract 'query'
var query = arguments.stream().filter(a -> a.getArgName().equalsIgnoreCase("query")).findFirst()
.orElseThrow(() -> new SemanticCheckException("'query' parameter is missing"));

return createBuilder(query.getValue().valueOf(null).stringValue());
}

protected abstract T createBuilder(String query);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance;

import org.opensearch.index.query.QueryBuilders;
import org.opensearch.index.query.QueryStringQueryBuilder;

/**
* Class for Lucene query that builds the query_string query.
*/
public class QueryQuery extends NoFieldQuery<QueryStringQueryBuilder> {

final String queryQueryName = "query";

/**
* Default constructor for QueryQuery configures how RelevanceQuery.build() handles
* named arguments by calling the constructor of QueryStringQuery.
*/
public QueryQuery() {
GabeFernandez310 marked this conversation as resolved.
Show resolved Hide resolved
super(FunctionParameterRepository.QueryStringQueryBuildActions);
}

/**
* Builds QueryBuilder with query value and other default parameter values set.
*
* @param query : Query value for query_string query
* @return : Builder for query query
*/
protected QueryStringQueryBuilder createBuilder(String query) {
return QueryBuilders.queryStringQuery(query);
}

@Override
public String getQueryName() {
return queryQueryName;
}
}
Loading