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

Add position() to V2 engine #177

Merged
merged 8 commits into from
Nov 30, 2022
Merged
Show file tree
Hide file tree
Changes from 7 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
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.opensearch.sql.ast.expression.Literal;
import org.opensearch.sql.ast.expression.Not;
import org.opensearch.sql.ast.expression.Or;
import org.opensearch.sql.ast.expression.PositionFunction;
import org.opensearch.sql.ast.expression.QualifiedName;
import org.opensearch.sql.ast.expression.RelevanceFieldList;
import org.opensearch.sql.ast.expression.Span;
Expand Down Expand Up @@ -213,6 +214,13 @@ public Expression visitHighlightFunction(HighlightFunction node, AnalysisContext
return new HighlightExpression(expr);
}

@Override
public Expression visitPositionFunction(PositionFunction node, AnalysisContext context) {
Expression left = node.getLeft().accept(this, context);
acarbonetto marked this conversation as resolved.
Show resolved Hide resolved
Expression right = node.getRight().accept(this, context);
return DSL.position(left, right);
}

@Override
public Expression visitIn(In node, AnalysisContext context) {
return visitIn(node.getField(), node.getValueList(), context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.opensearch.sql.ast.expression.Map;
import org.opensearch.sql.ast.expression.Not;
import org.opensearch.sql.ast.expression.Or;
import org.opensearch.sql.ast.expression.PositionFunction;
import org.opensearch.sql.ast.expression.QualifiedName;
import org.opensearch.sql.ast.expression.RelevanceFieldList;
import org.opensearch.sql.ast.expression.Span;
Expand Down Expand Up @@ -278,6 +279,10 @@ public T visitHighlightFunction(HighlightFunction node, C context) {
return visitChildren(node, context);
}

public T visitPositionFunction(PositionFunction node, C context) {
return visitChildren(node, context);
}

public T visitStatement(Statement node, C context) {
return visit(node, context);
}
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.opensearch.sql.ast.expression.Not;
import org.opensearch.sql.ast.expression.Or;
import org.opensearch.sql.ast.expression.ParseMethod;
import org.opensearch.sql.ast.expression.PositionFunction;
import org.opensearch.sql.ast.expression.QualifiedName;
import org.opensearch.sql.ast.expression.Span;
import org.opensearch.sql.ast.expression.SpanUnit;
Expand Down Expand Up @@ -288,6 +289,10 @@ public UnresolvedExpression highlight(UnresolvedExpression fieldName,
return new HighlightFunction(fieldName, arguments);
}

public UnresolvedExpression position(UnresolvedExpression left, UnresolvedExpression right) {
return new PositionFunction(left, right);

Choose a reason for hiding this comment

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

Same comment as above: can we used a better description than left and right?

Copy link
Author

Choose a reason for hiding this comment

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

Addressed in 9a6b7b5

}

public UnresolvedExpression window(UnresolvedExpression function,
List<UnresolvedExpression> partitionByList,
List<Pair<SortOption, UnresolvedExpression>> sortList) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.sql.ast.expression;

import java.util.Arrays;
import java.util.List;
import lombok.AllArgsConstructor;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.ToString;
import org.opensearch.sql.ast.AbstractNodeVisitor;


/**
* Expression node of Highlight function.
acarbonetto marked this conversation as resolved.
Show resolved Hide resolved
*/
@AllArgsConstructor
@EqualsAndHashCode(callSuper = false)
@Getter
@ToString
public class PositionFunction extends UnresolvedExpression {
@Getter
private UnresolvedExpression left;

Choose a reason for hiding this comment

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

Same comment as above:
Can we used a better description than left and right?

Copy link
Author

Choose a reason for hiding this comment

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

Addressed in 9a6b7b5

@Getter
private UnresolvedExpression right;

@Override
public List<UnresolvedExpression> getChild() {
return Arrays.asList(left, right);
}

@Override
public <R, C> R accept(AbstractNodeVisitor<R, C> nodeVisitor, C context) {
return nodeVisitor.visitPositionFunction(this, context);
}
}
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 @@ -229,6 +229,10 @@ public static FunctionExpression cbrt(Expression... expressions) {
return compile(BuiltinFunctionName.CBRT, expressions);
}

public static FunctionExpression position(Expression... expressions) {
return compile(BuiltinFunctionName.POSITION, expressions);
}

public static FunctionExpression truncate(Expression... expressions) {
return compile(BuiltinFunctionName.TRUNCATE, expressions);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ public enum BuiltinFunctionName {
ASCII(FunctionName.of("ascii")),
LOCATE(FunctionName.of("locate")),
REPLACE(FunctionName.of("replace")),
POSITION(FunctionName.of("position")),
acarbonetto marked this conversation as resolved.
Show resolved Hide resolved

/**
* NULL Test.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public void register(BuiltinFunctionRepository repository) {
repository.register(ascii());
repository.register(locate());
repository.register(replace());
repository.register(position());
acarbonetto marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down Expand Up @@ -241,6 +242,18 @@ private DefaultFunctionResolver locate() {
TextFunction::exprLocate), INTEGER, STRING, STRING, INTEGER));
}

/**
* Returns the position of the first occurrence of a substring in a string.

Choose a reason for hiding this comment

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

Can you mention that it starts with position 1, rather than 0? Since 0 is a 'failure'.

* Returns 0 if substring is not in string.
* Returns NULL if any argument is NULL.
* Supports following signature:
* (STRING IN STRING) -> INTEGER
*/
private DefaultFunctionResolver position() {
return define(BuiltinFunctionName.POSITION.getName(),
impl(nullMissingHandling(TextFunction::exprPosition), INTEGER, STRING, STRING));
}

/**
* REPLACE(str, from_str, to_str) returns the string str with all occurrences of
* the string from_str replaced by the string to_str.
Expand Down Expand Up @@ -313,6 +326,10 @@ private static ExprValue exprLocate(ExprValue subStr, ExprValue str, ExprValue p
str.stringValue().indexOf(subStr.stringValue(), pos.integerValue() - 1) + 1);
}

private static ExprValue exprPosition(ExprValue subStr, ExprValue str) {
return exprLocate(subStr, str);
}

private static ExprValue exprReplace(ExprValue str, ExprValue from, ExprValue to) {
return new ExprStringValue(str.stringValue().replaceAll(from.stringValue(), to.stringValue()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.opensearch.sql.ast.expression.Alias;
import org.opensearch.sql.ast.expression.HighlightFunction;
import org.opensearch.sql.ast.expression.Literal;
import org.opensearch.sql.ast.expression.PositionFunction;
import org.opensearch.sql.expression.NamedExpression;
import org.springframework.context.annotation.Configuration;
import org.springframework.test.context.ContextConfiguration;
Expand Down Expand Up @@ -48,4 +49,14 @@ void visit_highlight() {
NamedExpression analyze = analyzer.analyze(alias, analysisContext);
assertEquals("highlight(fieldA)", analyze.getNameOrAlias());
}

@Test
void visit_position() {
acarbonetto marked this conversation as resolved.
Show resolved Hide resolved
Alias alias = AstDSL.alias("position(fieldA IN fieldB)",
new PositionFunction(AstDSL.stringLiteral("fieldA"), AstDSL.stringLiteral("fieldB")));
NamedExpressionAnalyzer analyzer = new NamedExpressionAnalyzer(expressionAnalyzer);

NamedExpression analyze = analyzer.analyze(alias, analysisContext);
assertEquals("position(fieldA IN fieldB)", analyze.getNameOrAlias());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,20 @@ void locate() {
DSL.locate(missingRef, DSL.literal("hello"), DSL.literal(1))));
}

@Test
void position() {
FunctionExpression expression = DSL.position(
DSL.literal("world"),
DSL.literal("helloworld"));

Choose a reason for hiding this comment

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

can you change this to "helloworldworld"? Then it will also cover the duplicate case.

assertEquals(INTEGER, expression.type());
assertEquals(6, eval(expression).integerValue());
acarbonetto marked this conversation as resolved.
Show resolved Hide resolved

when(nullRef.type()).thenReturn(STRING);

Choose a reason for hiding this comment

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

Can we split these out into null and missing tests.

assertEquals(nullValue(), eval(DSL.position(nullRef, DSL.literal("hello"))));
when(missingRef.type()).thenReturn(STRING);
assertEquals(missingValue(), eval(DSL.position(missingRef, DSL.literal("hello"))));
}

@Test
void replace() {
FunctionExpression expression = DSL.replace(
Expand Down
25 changes: 25 additions & 0 deletions docs/user/dql/functions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2331,6 +2331,31 @@ Example::
+---------------------+---------------------+


POSITION
------

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

Usage: The syntax POSITION(substr IN str) returns the position of the first occurrence of substring substr in string str. Returns 0 if substr is not in str. Returns NULL if any argument is NULL.

Argument type: STRING, STRING, INTEGER

Return type integer:

(STRING IN STRING) -> INTEGER

Example::

os> SELECT POSITION('world' IN 'helloworld')
acarbonetto marked this conversation as resolved.
Show resolved Hide resolved
fetched rows / total rows = 1/1
+-------------------------------------+
| POSITION('world' IN 'helloworld') |
|-------------------------------------|
| 6 |
+-------------------------------------+


REPLACE
-------

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.sql.sql;

import org.json.JSONObject;
import org.junit.Test;
import org.opensearch.sql.legacy.SQLIntegTestCase;
import org.opensearch.sql.legacy.TestsConstants;

import static org.opensearch.sql.util.MatcherUtils.rows;
import static org.opensearch.sql.util.MatcherUtils.schema;
import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
import static org.opensearch.sql.util.MatcherUtils.verifySchema;

public class PositionFunctionIT extends SQLIntegTestCase {

@Override
protected void init() throws Exception {
loadIndex(Index.PEOPLE2);
loadIndex(Index.CALCS);
}

@Test
public void position_function_test() {
String query = "SELECT firstname, position('a' IN firstname) FROM %s";
JSONObject response = executeJdbcRequest(String.format(query, TestsConstants.TEST_INDEX_PEOPLE2));

verifySchema(response, schema("firstname", null, "keyword"),
schema("position('a' IN firstname)", null, "integer"));
assertEquals(12, response.getInt("total"));

verifyDataRows(response,
rows("Daenerys", 2), rows("Hattie", 2),
rows("Nanette", 2), rows("Dale", 2),
rows("Elinor", 0), rows("Virginia", 8),
rows("Dillard", 5), rows("Mcgee", 0),
rows("Aurelia", 7), rows("Fulton", 0),
rows("Burton", 0), rows("Josie", 0));
}

@Test
public void position_function_with_nulls_test() {
String query = "SELECT str2, position('ee' IN str2) FROM %s";
JSONObject response = executeJdbcRequest(String.format(query, TestsConstants.TEST_INDEX_CALCS));

verifySchema(response, schema("str2", null, "keyword"),
schema("position('ee' IN str2)", null, "integer"));
assertEquals(17, response.getInt("total"));

verifyDataRows(response,
rows("one", 0), rows("two", 0),
rows("three", 4), rows(null, null),
rows("five", 0), rows("six", 0),
rows(null, null), rows("eight", 0),
rows("nine", 0), rows("ten", 0),
rows("eleven", 0), rows("twelve", 0),
rows(null, null), rows("fourteen", 6),
rows("fifteen", 5), rows("sixteen", 5),
rows(null, null));
}

@Test
public void position_function_with_string_literals_test() {
String query = "SELECT position('world' IN 'hello world')";
JSONObject response = executeJdbcRequest(query);

verifySchema(response, schema("position('world' IN 'hello world')", null, "integer"));
assertEquals(1, response.getInt("total"));

verifyDataRows(response, rows(7));
}

@Test
public void position_function_with_only_fields_as_args_test() {
String query = "SELECT position(str3 IN str2) FROM %s WHERE str2 IN ('one', 'two', 'three')";
JSONObject response = executeJdbcRequest(String.format(query, TestsConstants.TEST_INDEX_CALCS));

verifySchema(response, schema("position(str3 IN str2)", null, "integer"));
assertEquals(3, response.getInt("total"));

verifyDataRows(response, rows(3), rows(0), rows(4));
}

@Test
public void position_function_with_function_as_arg_test() {
String query = "SELECT position(upper(str3) IN str1) FROM %s WHERE str1 LIKE 'BINDING SUPPLIES'";
JSONObject response = executeJdbcRequest(String.format(query, TestsConstants.TEST_INDEX_CALCS));

verifySchema(response, schema("position(upper(str3) IN str1)", null, "integer"));
assertEquals(1, response.getInt("total"));

verifyDataRows(response, rows(15));
}
GumpacG marked this conversation as resolved.
Show resolved Hide resolved

@Test
public void position_function_in_where_clause_test() {
String query = "SELECT str2 FROM %s WHERE position(str3 IN str2)=1";
JSONObject response = executeJdbcRequest(String.format(query, TestsConstants.TEST_INDEX_CALCS));

verifySchema(response, schema("str2", null, "keyword"));
assertEquals(2, response.getInt("total"));

verifyDataRows(response, rows("eight"), rows("eleven"));
}
acarbonetto marked this conversation as resolved.
Show resolved Hide resolved
}
1 change: 1 addition & 0 deletions sql/src/main/antlr/OpenSearchSQLLexer.g4
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ NULLIF: 'NULLIF';
PERIOD_ADD: 'PERIOD_ADD';
PERIOD_DIFF: 'PERIOD_DIFF';
PI: 'PI';
POSITION: 'POSITION';
POW: 'POW';
POWER: 'POWER';
RADIANS: 'RADIANS';
Expand Down
5 changes: 5 additions & 0 deletions sql/src/main/antlr/OpenSearchSQLParser.g4
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ functionCall
| relevanceFunction #relevanceFunctionCall
| highlightFunction #highlightFunctionCall
| constantFunction #constantFunctionCall
| positionFunction #positionFunctionCall
;

constantFunction
Expand All @@ -312,6 +313,10 @@ highlightFunction
: HIGHLIGHT LR_BRACKET relevanceField (COMMA highlightArg)* RR_BRACKET
;

positionFunction
: POSITION LR_BRACKET functionArg IN functionArg RR_BRACKET
;

scalarFunctionName
: mathematicalFunctionName
| dateTimeFunctionName
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
import org.opensearch.sql.ast.expression.Literal;
import org.opensearch.sql.ast.expression.Not;
import org.opensearch.sql.ast.expression.Or;
import org.opensearch.sql.ast.expression.PositionFunction;
import org.opensearch.sql.ast.expression.QualifiedName;
import org.opensearch.sql.ast.expression.RelevanceFieldList;
import org.opensearch.sql.ast.expression.UnresolvedArgument;
Expand Down Expand Up @@ -428,6 +429,13 @@ private UnresolvedExpression visitConstantFunction(String functionName,
.collect(Collectors.toList()));
}

@Override
public UnresolvedExpression visitPositionFunction(
OpenSearchSQLParser.PositionFunctionContext ctx) {
return new PositionFunction(visitFunctionArg(ctx.functionArg(0)),
visitFunctionArg(ctx.functionArg(1)));
}

private QualifiedName visitIdentifiers(List<IdentContext> identifiers) {
return new QualifiedName(
identifiers.stream()
Expand Down
Loading