From 1c070a2d8abcc6305ea71d80c8ea197524ef1dd7 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Thu, 6 Aug 2020 08:59:33 +0200 Subject: Allow references when expecting numbers --- .../java/com/yahoo/search/yql/OperatorNode.java | 2 - .../main/java/com/yahoo/search/yql/YqlParser.java | 46 ++++++++++++++-------- .../com/yahoo/search/yql/UserInputTestCase.java | 9 +++++ .../com/yahoo/search/yql/YqlParserTestCase.java | 18 ++++----- 4 files changed, 46 insertions(+), 29 deletions(-) (limited to 'container-search') diff --git a/container-search/src/main/java/com/yahoo/search/yql/OperatorNode.java b/container-search/src/main/java/com/yahoo/search/yql/OperatorNode.java index 431f159db01..e3ef4b5d4df 100644 --- a/container-search/src/main/java/com/yahoo/search/yql/OperatorNode.java +++ b/container-search/src/main/java/com/yahoo/search/yql/OperatorNode.java @@ -13,9 +13,7 @@ import java.util.Map; /** * Represents a use of an operator against concrete arguments. The types of arguments depend on the operator. - *

* The extension point of this scheme is the Operator rather than new types of Nodes. - *

* Operators SHOULD take a fixed number of arguments -- wrap variable argument counts in Lists. */ final class OperatorNode { diff --git a/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java b/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java index 6a464a1503b..31a22fd3b58 100644 --- a/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java +++ b/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java @@ -1004,40 +1004,52 @@ public class YqlParser implements Parser { private String fetchConditionIndex(OperatorNode ast) { OperatorNode lhs = ast.getArgument(0); OperatorNode rhs = ast.getArgument(1); - if (lhs.getOperator() == ExpressionOperator.LITERAL || lhs.getOperator() == ExpressionOperator.NEGATE) { + if (isNumber(lhs)) return getIndex(rhs); - } - if (rhs.getOperator() == ExpressionOperator.LITERAL || rhs.getOperator() == ExpressionOperator.NEGATE) { + else if (isNumber(rhs)) return getIndex(lhs); - } - throw new IllegalArgumentException("Expected LITERAL and READ_FIELD/PROPREF, got " + lhs.getOperator() + - " and " + rhs.getOperator() + "."); + else + throw new IllegalArgumentException("Expected LITERAL/VARREF and READ_FIELD/PROPREF, got " + lhs.getOperator() + + " and " + rhs.getOperator() + "."); } - private static String getNumberAsString(OperatorNode ast) { + private boolean isNumber(OperatorNode ast) { + return ast.getOperator() == ExpressionOperator.NEGATE || + ast.getOperator() == ExpressionOperator.LITERAL || ast.getOperator() == ExpressionOperator.VARREF; + } + + private String getNumberAsString(OperatorNode ast) { String negative = ""; - OperatorNode currentAst = ast; - if (currentAst.getOperator() == ExpressionOperator.NEGATE) { + if (ast.getOperator() == ExpressionOperator.NEGATE) { negative = "-"; - currentAst = currentAst.getArgument(0); + ast = ast.getArgument(0); + } + switch (ast.getOperator()) { + case VARREF: + Preconditions.checkState(userQuery != null, + "properties must be available when trying to fetch user input"); + return negative + userQuery.properties().getString(ast.getArgument(0, String.class)); + case LITERAL: + return negative + ast.getArgument(0).toString(); + default: + throw new IllegalArgumentException("Expected VARREF or LITERAL, got " + ast.getOperator()); } - assertHasOperator(currentAst, ExpressionOperator.LITERAL); - return negative + currentAst.getArgument(0).toString(); } - private static String fetchConditionWord(OperatorNode ast) { + private String fetchConditionWord(OperatorNode ast) { OperatorNode lhs = ast.getArgument(0); OperatorNode rhs = ast.getArgument(1); - if (lhs.getOperator() == ExpressionOperator.LITERAL || lhs.getOperator() == ExpressionOperator.NEGATE) { + if (isNumber(lhs)) { assertFieldName(rhs); return getNumberAsString(lhs); } - if (rhs.getOperator() == ExpressionOperator.LITERAL || rhs.getOperator() == ExpressionOperator.NEGATE) { + else if (isNumber(rhs)) { assertFieldName(lhs); return getNumberAsString(rhs); } - throw new IllegalArgumentException("Expected LITERAL/NEGATE and READ_FIELD/PROPREF, got " - + lhs.getOperator() + " and " + rhs.getOperator() + "."); + else + throw new IllegalArgumentException("Expected LITERAL/NEGATE and READ_FIELD/PROPREF, got " + + lhs.getOperator() + " and " + rhs.getOperator() + "."); } private static boolean isIndexOnLeftHandSide(OperatorNode ast) { diff --git a/container-search/src/test/java/com/yahoo/search/yql/UserInputTestCase.java b/container-search/src/test/java/com/yahoo/search/yql/UserInputTestCase.java index 6173d710434..75517a25909 100644 --- a/container-search/src/test/java/com/yahoo/search/yql/UserInputTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/yql/UserInputTestCase.java @@ -195,6 +195,15 @@ public class UserInputTestCase { assertEquals("select * from sources * where (foo contains \"bamse\" AND foo contains phrase(\"bamse\", \"syntactic\", \"bamse\"));", query.yqlRepresentation()); } + @Test + public void testReferenceInComparison() { + URIBuilder builder = searchUri(); + builder.setParameter("varref", "1980"); + builder.setParameter("yql", "select * from sources * where year > @varref;"); + Query query = searchAndAssertNoErrors(builder); + assertEquals("select * from sources * where year > 1980;", query.yqlRepresentation()); + } + private Query searchAndAssertNoErrors(URIBuilder builder) { Query query = new Query(builder.toString()); Result r = execution.search(query); diff --git a/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java b/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java index 62a9e27cd96..6e7aa752e29 100644 --- a/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java @@ -767,12 +767,12 @@ public class YqlParserTestCase { "select foo from bar where title contains \"madonna\"" + " order by [{\"function\": \"uca\", \"locale\": \"en_US\", \"strength\": \"IDENTICAL\"}]other desc" + " limit 600" + " timeout 3;", "title:madonna"); - final FieldOrder fieldOrder = parser.getSorting().fieldOrders().get(0); + FieldOrder fieldOrder = parser.getSorting().fieldOrders().get(0); assertEquals("other", fieldOrder.getFieldName()); assertEquals(Order.DESCENDING, fieldOrder.getSortOrder()); - final AttributeSorter sorter = fieldOrder.getSorter(); + AttributeSorter sorter = fieldOrder.getSorter(); assertEquals(UcaSorter.class, sorter.getClass()); - final UcaSorter uca = (UcaSorter) sorter; + UcaSorter uca = (UcaSorter) sorter; assertEquals("en_US", uca.getLocale()); assertEquals(UcaSorter.Strength.IDENTICAL, uca.getStrength()); } @@ -785,22 +785,20 @@ public class YqlParserTestCase { + " [{\"function\": \"lowercase\"}]something asc" + " limit 600" + " timeout 3;", "title:madonna"); { - final FieldOrder fieldOrder = parser.getSorting().fieldOrders() - .get(0); + FieldOrder fieldOrder = parser.getSorting().fieldOrders().get(0); assertEquals("other", fieldOrder.getFieldName()); assertEquals(Order.DESCENDING, fieldOrder.getSortOrder()); - final AttributeSorter sorter = fieldOrder.getSorter(); + AttributeSorter sorter = fieldOrder.getSorter(); assertEquals(UcaSorter.class, sorter.getClass()); - final UcaSorter uca = (UcaSorter) sorter; + UcaSorter uca = (UcaSorter) sorter; assertEquals("en_US", uca.getLocale()); assertEquals(UcaSorter.Strength.IDENTICAL, uca.getStrength()); } { - final FieldOrder fieldOrder = parser.getSorting().fieldOrders() - .get(1); + FieldOrder fieldOrder = parser.getSorting().fieldOrders().get(1); assertEquals("something", fieldOrder.getFieldName()); assertEquals(Order.ASCENDING, fieldOrder.getSortOrder()); - final AttributeSorter sorter = fieldOrder.getSorter(); + AttributeSorter sorter = fieldOrder.getSorter(); assertEquals(LowerCaseSorter.class, sorter.getClass()); } } -- cgit v1.2.3