honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

SQL: Fix issue with options for QUERY() and MATCH(). (#33828) 

Previously multiple comma separated lists of options where not
recognized correctly which resulted in only the last of them
to be taked into account, e.g.:

For the following query:
  SELECT * FROM test WHERE QUERY('search', 'default_field=foo', 'default_operator=and')"
only the `default_operator=and` was finally passed to the ES query.

Fixes: #32602
This commit is contained in:
Marios Trivyzas 2018-09-19 10:16:24 +02:00 committed by GitHub
parent c9765d5fb9
commit d22b383b9c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 1049 additions and 880 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
x-pack/plugin/sql/src/main/antlr/SqlBase.g4
View File

@ -163,14 +163,18 @@ expression
booleanExpression
: NOT booleanExpression #logicalNot
| EXISTS '(' query ')' #exists
| QUERY '(' queryString=string (',' options=string)* ')' #stringQuery
| MATCH '(' singleField=qualifiedName ',' queryString=string (',' options=string)* ')' #matchQuery
| MATCH '(' multiFields=string ',' queryString=string (',' options=string)* ')' #multiMatchQuery
| QUERY '(' queryString=string matchQueryOptions ')' #stringQuery
| MATCH '(' singleField=qualifiedName ',' queryString=string matchQueryOptions ')' #matchQuery
| MATCH '(' multiFields=string ',' queryString=string matchQueryOptions ')' #multiMatchQuery
| predicated #booleanDefault
| left=booleanExpression operator=AND right=booleanExpression #logicalBinary
| left=booleanExpression operator=OR right=booleanExpression #logicalBinary
;
matchQueryOptions
: (',' string)*
;
// workaround for:
// https://github.com/antlr/antlr4/issues/780
// https://github.com/antlr/antlr4/issues/781

12
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/fulltext/FullTextUtils.java
View File

@ -5,16 +5,16 @@
*/
package org.elasticsearch.xpack.sql.expression.predicate.fulltext;
import java.util.LinkedHashMap;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import org.elasticsearch.common.Strings;
import org.elasticsearch.xpack.sql.expression.predicate.fulltext.FullTextPredicate.Operator;
import org.elasticsearch.xpack.sql.parser.ParsingException;
import org.elasticsearch.xpack.sql.tree.Location;
import java.util.LinkedHashMap;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import static java.util.Collections.emptyMap;
abstract class FullTextUtils {
@ -26,7 +26,7 @@ abstract class FullTextUtils {
return emptyMap();
}
String[] list = Strings.delimitedListToStringArray(options, DELIMITER);
Map<String, String> op = new LinkedHashMap<String, String>(list.length);
Map<String, String> op = new LinkedHashMap<>(list.length);
for (String entry : list) {
String[] split = splitInTwo(entry, "=");

19
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java
View File

@ -67,6 +67,7 @@ import org.elasticsearch.xpack.sql.parser.SqlBaseParser.LikePatternContext;
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.LogicalBinaryContext;
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.LogicalNotContext;
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.MatchQueryContext;
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.MatchQueryOptionsContext;
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.MultiMatchQueryContext;
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.NullLiteralContext;
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.OrderByContext;
@ -99,6 +100,7 @@ import java.math.BigInteger;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.StringJoiner;
import static java.util.Collections.singletonList;
import static org.elasticsearch.xpack.sql.type.DataTypeConversion.conversionFor;
@ -324,18 +326,27 @@ abstract class ExpressionBuilder extends IdentifierBuilder {
//
@Override
public Object visitStringQuery(StringQueryContext ctx) {
return new StringQueryPredicate(source(ctx), string(ctx.queryString), string(ctx.options));
return new StringQueryPredicate(source(ctx), string(ctx.queryString), getQueryOptions(ctx.matchQueryOptions()));
}
@Override
public Object visitMatchQuery(MatchQueryContext ctx) {
return new MatchQueryPredicate(source(ctx), new UnresolvedAttribute(source(ctx.singleField),
visitQualifiedName(ctx.singleField)), string(ctx.queryString), string(ctx.options));
visitQualifiedName(ctx.singleField)), string(ctx.queryString), getQueryOptions(ctx.matchQueryOptions()));
}
@Override
public Object visitMultiMatchQuery(MultiMatchQueryContext ctx) {
return new MultiMatchQueryPredicate(source(ctx), string(ctx.multiFields), string(ctx.queryString), string(ctx.options));
return new MultiMatchQueryPredicate(source(ctx), string(ctx.multiFields), string(ctx.queryString),
getQueryOptions(ctx.matchQueryOptions()));
}
private String getQueryOptions(MatchQueryOptionsContext optionsCtx) {
StringJoiner sj = new StringJoiner(";");
for (StringContext sc: optionsCtx.string()) {
sj.add(string(sc));
}
return sj.toString();
}
@Override
@ -676,4 +687,4 @@ abstract class ExpressionBuilder extends IdentifierBuilder {
return new Literal(source(ctx), string, DataType.KEYWORD);
}
}
}

12
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/SqlBaseBaseListener.java
View File

@ -527,6 +527,18 @@ class SqlBaseBaseListener implements SqlBaseListener {
* <p>The default implementation does nothing.</p>
*/
@Override public void exitLogicalBinary(SqlBaseParser.LogicalBinaryContext ctx) { }
/**
* {@inheritDoc}
*
* <p>The default implementation does nothing.</p>
*/
@Override public void enterMatchQueryOptions(SqlBaseParser.MatchQueryOptionsContext ctx) { }
/**
* {@inheritDoc}
*
* <p>The default implementation does nothing.</p>
*/
@Override public void exitMatchQueryOptions(SqlBaseParser.MatchQueryOptionsContext ctx) { }
/**
* {@inheritDoc}
*

7
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/SqlBaseBaseVisitor.java
View File

@ -312,6 +312,13 @@ class SqlBaseBaseVisitor<T> extends AbstractParseTreeVisitor<T> implements SqlBa
* {@link #visitChildren} on {@code ctx}.</p>
*/
@Override public T visitLogicalBinary(SqlBaseParser.LogicalBinaryContext ctx) { return visitChildren(ctx); }
/**
* {@inheritDoc}
*
* <p>The default implementation returns the result of calling
* {@link #visitChildren} on {@code ctx}.</p>
*/
@Override public T visitMatchQueryOptions(SqlBaseParser.MatchQueryOptionsContext ctx) { return visitChildren(ctx); }
/**
* {@inheritDoc}
*

10
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/SqlBaseListener.java
View File

@ -489,6 +489,16 @@ interface SqlBaseListener extends ParseTreeListener {
* @param ctx the parse tree
*/
void exitLogicalBinary(SqlBaseParser.LogicalBinaryContext ctx);
/**
* Enter a parse tree produced by {@link SqlBaseParser#matchQueryOptions}.
* @param ctx the parse tree
*/
void enterMatchQueryOptions(SqlBaseParser.MatchQueryOptionsContext ctx);
/**
* Exit a parse tree produced by {@link SqlBaseParser#matchQueryOptions}.
* @param ctx the parse tree
*/
void exitMatchQueryOptions(SqlBaseParser.MatchQueryOptionsContext ctx);
/**
* Enter a parse tree produced by {@link SqlBaseParser#predicated}.
* @param ctx the parse tree

1745
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/SqlBaseParser.java
View File

File diff suppressed because it is too large Load Diff

6
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/SqlBaseVisitor.java
View File

@ -294,6 +294,12 @@ interface SqlBaseVisitor<T> extends ParseTreeVisitor<T> {
* @return the visitor result
*/
T visitLogicalBinary(SqlBaseParser.LogicalBinaryContext ctx);
/**
* Visit a parse tree produced by {@link SqlBaseParser#matchQueryOptions}.
* @param ctx the parse tree
* @return the visitor result
*/
T visitMatchQueryOptions(SqlBaseParser.MatchQueryOptionsContext ctx);
/**
* Visit a parse tree produced by {@link SqlBaseParser#predicated}.
* @param ctx the parse tree

41
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/predicate/fulltext/FullTextUtilsTests.java Normal file
View File

@ -0,0 +1,41 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.xpack.sql.expression.predicate.fulltext;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.sql.parser.ParsingException;
import org.elasticsearch.xpack.sql.tree.Location;
import java.util.Map;
import static org.hamcrest.Matchers.hasEntry;
import static org.hamcrest.Matchers.is;
public class FullTextUtilsTests extends ESTestCase {
public void testColonDelimited() {
Map<String, String> options = FullTextUtils.parseSettings("k1=v1;k2=v2", new Location(1, 1));
assertThat(options.size(), is(2));
assertThat(options, hasEntry("k1", "v1"));
assertThat(options, hasEntry("k2", "v2"));
}
public void testColonDelimitedErrorString() {
ParsingException e = expectThrows(ParsingException.class,
() -> FullTextUtils.parseSettings("k1=v1;k2v2", new Location(1, 1)));
assertThat(e.getMessage(), is("line 1:3: Cannot parse entry k2v2 in options k1=v1;k2v2"));
assertThat(e.getLineNumber(), is(1));
assertThat(e.getColumnNumber(), is(3));
}
public void testColonDelimitedErrorDuplicate() {
ParsingException e = expectThrows(ParsingException.class,
() -> FullTextUtils.parseSettings("k1=v1;k1=v2", new Location(1, 1)));
assertThat(e.getMessage(), is("line 1:3: Duplicate option k1=v2 detected in options k1=v1;k1=v2"));
assertThat(e.getLineNumber(), is(1));
assertThat(e.getColumnNumber(), is(3));
}
}

46
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/SqlParserTests.java
View File

@ -11,6 +11,10 @@ import org.elasticsearch.xpack.sql.expression.Order;
import org.elasticsearch.xpack.sql.expression.UnresolvedAttribute;
import org.elasticsearch.xpack.sql.expression.UnresolvedStar;
import org.elasticsearch.xpack.sql.expression.function.UnresolvedFunction;
import org.elasticsearch.xpack.sql.expression.predicate.fulltext.MatchQueryPredicate;
import org.elasticsearch.xpack.sql.expression.predicate.fulltext.MultiMatchQueryPredicate;
import org.elasticsearch.xpack.sql.expression.predicate.fulltext.StringQueryPredicate;
import org.elasticsearch.xpack.sql.plan.logical.Filter;
import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan;
import org.elasticsearch.xpack.sql.plan.logical.OrderBy;
import org.elasticsearch.xpack.sql.plan.logical.Project;
@ -19,6 +23,7 @@ import java.util.ArrayList;
import java.util.List;
import static java.util.stream.Collectors.toList;
import static org.hamcrest.Matchers.hasEntry;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.instanceOf;
@ -92,6 +97,45 @@ public class SqlParserTests extends ESTestCase {
assertEquals("baz", a.name());
}
public void testStringQuery() {
LogicalPlan plan =
parseStatement("SELECT * FROM FOO WHERE " +
"QUERY('foo', 'default_field=last_name;lenient=true', 'fuzzy_rewrite=scoring_boolean')");
StringQueryPredicate sqp = (StringQueryPredicate) ((Filter) plan.children().get(0).children().get(0)).condition();
assertEquals("foo", sqp.query());
assertEquals(3, sqp.optionMap().size());
assertThat(sqp.optionMap(), hasEntry("default_field", "last_name"));
assertThat(sqp.optionMap(), hasEntry("lenient", "true"));
assertThat(sqp.optionMap(), hasEntry("fuzzy_rewrite", "scoring_boolean"));
}
public void testMatchQuery() {
LogicalPlan plan = parseStatement("SELECT * FROM FOO WHERE " +
"MATCH(first_name, 'foo', 'operator=AND;lenient=true', 'fuzzy_rewrite=scoring_boolean')");
MatchQueryPredicate mqp = (MatchQueryPredicate) ((Filter) plan.children().get(0).children().get(0)).condition();
assertEquals("foo", mqp.query());
assertEquals("?first_name", mqp.field().toString());
assertEquals(3, mqp.optionMap().size());
assertThat(mqp.optionMap(), hasEntry("operator", "AND"));
assertThat(mqp.optionMap(), hasEntry("lenient", "true"));
assertThat(mqp.optionMap(), hasEntry("fuzzy_rewrite", "scoring_boolean"));
}
public void testMultiMatchQuery() {
LogicalPlan plan = parseStatement("SELECT * FROM FOO WHERE " +
"MATCH('first_name,last_name', 'foo', 'operator=AND;type=best_fields', 'fuzzy_rewrite=scoring_boolean')");
MultiMatchQueryPredicate mmqp = (MultiMatchQueryPredicate) ((Filter) plan.children().get(0).children().get(0)).condition();
assertEquals("foo", mmqp.query());
assertEquals("first_name,last_name", mmqp.fieldString());
assertEquals(3, mmqp.optionMap().size());
assertThat(mmqp.optionMap(), hasEntry("operator", "AND"));
assertThat(mmqp.optionMap(), hasEntry("type", "best_fields"));
assertThat(mmqp.optionMap(), hasEntry("fuzzy_rewrite", "scoring_boolean"));
}
private LogicalPlan parseStatement(String sql) {
return new SqlParser().createStatement(sql);
}
@ -132,4 +176,4 @@ public class SqlParserTests extends ESTestCase {
String dirStr = dir.toString();
return randomBoolean() && dirStr.equals("ASC") ? "" : " " + dirStr;
}
}
}

21
x-pack/qa/sql/src/main/resources/fulltext.csv-spec
View File

@ -23,6 +23,13 @@ SELECT emp_no, first_name, gender, last_name FROM test_emp WHERE QUERY('Man*', '
10096 |Jayson |M |Mandell
;
simpleQueryOptionsInMultipleCommaSeparatedStrings
SELECT emp_no, first_name, gender, last_name FROM test_emp WHERE QUERY('Man*', 'default_field=last_name;lenient=true', 'fuzzy_rewrite=scoring_boolean') LIMIT 5;
emp_no:i | first_name:s | gender:s | last_name:s
10096 |Jayson |M |Mandell
;
matchQuery
SELECT emp_no, first_name, gender, last_name FROM test_emp WHERE MATCH(first_name, 'Erez');
@ -37,6 +44,13 @@ SELECT emp_no, first_name, gender, last_name FROM test_emp WHERE MATCH(first_nam
10076 |Erez |F |Ritzmann
;
matchQueryWithOptionsInMultipleCommaSeparatedStrings
SELECT emp_no, first_name, gender, last_name FROM test_emp WHERE MATCH(first_name, 'Erez', 'lenient=true;cutoff_frequency=2','fuzzy_rewrite=scoring_boolean;minimum_should_match=1','operator=AND', 'max_expansions=30;prefix_length=1;analyzer=english;auto_generate_synonyms_phrase_query=true;fuzzy_transpositions=true');
emp_no:i | first_name:s | gender:s | last_name:s
10076 |Erez |F |Ritzmann
;
multiMatchQuery
SELECT emp_no, first_name, gender, last_name FROM test_emp WHERE MATCH('first_name,last_name', 'Morton', 'type=best_fields;operator=OR');
@ -51,6 +65,13 @@ SELECT emp_no, first_name, gender, last_name FROM test_emp WHERE MATCH('first_na
10095 |Hilari |M |Morton
;
multiMatchQueryWithInMultipleCommaSeparatedStrings
SELECT emp_no, first_name, gender, last_name FROM test_emp WHERE MATCH('first_name,last_name', 'Morton', 'slop=1;lenient=true', 'cutoff_frequency=2','tie_breaker=0.1;use_dis_max=true;fuzzy_rewrite=scoring_boolean','minimum_should_match=1;operator=AND;max_expansions=30;prefix_length=1;analyzer=english;type=best_fields;auto_generate_synonyms_phrase_query=true;fuzzy_transpositions=true');
emp_no:i | first_name:s | gender:s | last_name:s
10095 |Hilari |M |Morton
;
score
SELECT emp_no, first_name, SCORE() FROM test_emp WHERE MATCH(first_name, 'Erez') ORDER BY SCORE();