Skip to content
This repository has been archived by the owner on Sep 24, 2019. It is now read-only.

Commit

Permalink
SQL: Fix issue with options for QUERY() and MATCH(). (elastic#33828)
Browse files Browse the repository at this point in the history
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: elastic#32602
  • Loading branch information
Marios Trivyzas authored Sep 19, 2018
1 parent c9765d5 commit d22b383
Show file tree
Hide file tree
Showing 11 changed files with 1,037 additions and 868 deletions.
10 changes: 7 additions & 3 deletions x-pack/plugin/sql/src/main/antlr/SqlBase.g4
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -26,7 +26,7 @@ static Map<String, String> parseSettings(String options, Location location) {
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, "=");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
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;
Expand Down Expand Up @@ -99,6 +100,7 @@
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;
Expand Down Expand Up @@ -324,18 +326,27 @@ public Object visitArithmeticBinary(ArithmeticBinaryContext ctx) {
//
@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
Expand Down Expand Up @@ -676,4 +687,4 @@ public Literal visitGuidEscapedLiteral(GuidEscapedLiteralContext ctx) {

return new Literal(source(ctx), string, DataType.KEYWORD);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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}
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit d22b383

Please sign in to comment.