Implementation for `simple_query_string`. #61

Yury-Fridlyand · 2022-05-18T00:02:06Z

Signed-off-by: Yury Fridlyand yuryf@bitquilltech.com

Description

Rework on #53 and #55.

Issues Resolved

Check List

New functionality includes testing.
- All tests pass, including unit test, integration test and doctest
New functionality has been documented.
- New functionality has javadoc added
- New functionality has user manual doc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Required renaming analyzeSyntax to parse. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

Added base class that is commont to match_query and match_bool_prefix and others. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

MaxKsyunz

Let's discuss adding field parsing to the parser and propagating the parsed result down to SimpleQueryString.

I believe that will result clearer code.

Also, why is the SQL Java CI workflow failing?

ppl/src/main/antlr/OpenSearchPPLParser.g4

MaxKsyunz · 2022-05-18T07:41:13Z

ppl/src/main/antlr/OpenSearchPPLParser.g4

@@ -308,6 +319,12 @@ relevanceArgName
    | BOOST
    ;

+relevanceField
+    : relevanceArgValue;


This is too permissive and allows for odd queries like: SIMPLE_QUERY_STRING([true, 3.14], ...)

Field list is a list of weighted field name patterns -- ["field_A^3.4", "field*", fie*^3.45].

Wildcard by itself -- "*" -- is a valid field name as well.

See https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-simple-query-string-query.html#simple-query-string-top-level-params

as well as org/opensearch/index/search/SimpleQueryStringQueryParser.java in opensearch code.

Fixed in 94748b5, thank you.
Note it affects other relevance functions as well (match), so I have to update tests too, see d5def08.

MaxKsyunz · 2022-05-18T07:42:29Z

ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java

-    return new Function(
-        ctx.relevanceFunctionName().getText().toLowerCase(),
-        relevanceArguments(ctx));
+    if (ctx.relevanceFunctionType1() != null) {


There should be a visitRelevanceFunctionType1 method that you can override and put the content of this if block.

MaxKsyunz · 2022-05-18T07:43:04Z

ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java

+          ctx.relevanceFunctionType1().relevanceFunctionType1Name().getText().toLowerCase(),
+          relevanceArgumentsType1(ctx.relevanceFunctionType1()));
+    }
+    else {


There should be a visitRelevanceFunctionType2 method that you can override and put the content of this else block.

MaxKsyunz · 2022-05-18T07:43:36Z

ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java

@@ -252,9 +254,16 @@ public UnresolvedExpression visitConvertedDataType(ConvertedDataTypeContext ctx)

  @Override
  public UnresolvedExpression visitRelevanceExpression(RelevanceExpressionContext ctx) {


This can be replaced by two overrides -- see the following comments.

MaxKsyunz · 2022-05-18T07:53:19Z

.../org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SimpleQueryString.java

+import java.util.function.BiFunction;
+import java.util.stream.Collectors;
+
+public class SimpleQueryString extends LuceneQuery {


Should be SimpleQueryStringQuery to follow name convention.

Thank you, fixed in 2b4a99b.

MaxKsyunz · 2022-05-18T08:01:34Z

.../org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SimpleQueryString.java

+          continue;
+        }
+
+        var parts = elem.split("\\^");


What's the return value of split? The should be used instead of var.

Also, why "\\^" instead of ^?

Honestly, I dislike the extensive use of var in strongly-typed languages like Java - because I find it makes the code unreadable.
Anybody reading the code shouldn't have to look up the function to find out what the return type is supposed to be.

Also, why "\\^" instead of ^?

split function receives a regex, so I have to escape ^, otherwise it would be interpreted as a regex control symbol.

MaxKsyunz · 2022-05-18T08:17:16Z

.../org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SimpleQueryString.java

+
+        var parts = elem.split("\\^");
+        var weight = Float.parseFloat(parts[parts.length - 1]);
+        var field = Arrays.stream(parts).limit(parts.length - 1).collect(Collectors.joining("^"));


I'd like to look at making field weight a first-class parser concept. It's not clear if this implementation can differentiate between field named title^4.3 and weight of 4.3 for field title.

Is the \\^ above to allow for escaping ^ that is part of field names?

I proposed the following in the opensearch-project -- opensearch-project#182 (comment)

Is the \\^ above to allow for escaping ^ that is part of field names?

No. split function receives a regex, so I have to escape ^, otherwise it would be interpreted as a regex control symbol.

A user can make a search like

simple_query_string(["some_field^1", "my^favourite^field^ever^100500"], ...)

and it would be parsed into following map

/-------------------------v--------\ | field | weight | >-------------------------+--------< | some_field | 1 | | my^favourite^field^ever | 100500 | \-------------------------^--------/

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

acarbonetto · 2022-05-18T18:27:30Z

core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java

@@ -158,6 +161,20 @@ public Expression visitAggregateFunction(AggregateFunction node, AnalysisContext
    }
  }

+  @Override
+  public Expression visitLiteralList(LiteralList node, AnalysisContext context) {
+    var lst = node


list vs lst? There's really no value to saving a single character in the code.
Code (java especially) should be verbose. list isn't really verbose either, and you could call it literalListCollection or similar.
Along those lines, I would also call node above literalListNode.

lst holds a list of internal AST objects. The list and the objects are consumed on the next line of code and don't exist anymore, so I called it just lst and not veryImportantListOfMagicThingsWhatever.
Re function arguments: have a look on the rest functions in this file. All of them are

public Expression visitSomething(Something node, AnalysisContext context)

So newly added function fits this style. Should I modify all of them?

You don't need to fix everything, but we should name our variables appropriately.

core/src/main/java/org/opensearch/sql/ast/expression/LiteralList.java

acarbonetto · 2022-05-18T18:30:33Z

core/src/main/java/org/opensearch/sql/expression/LiteralListExpression.java

+import org.opensearch.sql.expression.env.Environment;
+
+/**
+ * Literal Expression.


Literal List Expression

Thanks, fixed in 84d84e1.

acarbonetto · 2022-05-18T18:39:09Z

.../org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SimpleQueryString.java

+          continue;
+        }
+
+        var parts = elem.split("\\^");


Honestly, I dislike the extensive use of var in strongly-typed languages like Java - because I find it makes the code unreadable.
Anybody reading the code shouldn't have to look up the function to find out what the return type is supposed to be.

acarbonetto · 2022-05-18T18:41:57Z

.../org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SimpleQueryString.java

+      var res = new HashMap<String, Float>();
+      for (var literal : input) {
+        var elem = literal.stringValue();
+        if (!elem.contains("^")) {


This doesn't consider the case where the field contains a "^" in the name. We should be looking to make sure that the element contains a "^" with a float afterwards (\d+.\d+).
This requires regular expressions or tokenizing the string.

acarbonetto · 2022-05-18T18:42:12Z

.../org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SimpleQueryString.java

+  private Map<String, Float> parseFields(List<ExprValue> input) {
+    try {
+      var res = new HashMap<String, Float>();
+      for (var literal : input) {


We can use a .stream().map() here

acarbonetto · 2022-05-18T18:44:04Z

.../org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SimpleQueryString.java

+      }
+      return res;
+    }
+    catch (Exception e) {


Can we be more specific what we're catching? This will catch everything that turn it into a SemanticCheckException without much explanation.

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

- Two new AST entities: `RelevanceField` and `RelevanceFieldList` (instead of `LiteralList`). - Update in `AstExpressionBuilder`x2. - Rework on `ExpressionAnalyzer` and `SimpleQueryStringQuery`. - Update in `PPL` ANTLR parser. Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Removed `RelevanceField` as not needed anymore. Since parsing result is stored in `ExprTupleValue` which is actually a map, so we can keep data there. Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

acarbonetto · 2022-05-30T19:29:02Z

core/src/main/java/org/opensearch/sql/ast/expression/RelevanceFieldList.java

+
+  @Override
+  public List<UnresolvedExpression> getChild() {
+    return List.of();


the parent function returns null. What's the benefit of returning a list?

acarbonetto · 2022-05-30T19:46:16Z

ppl/src/main/antlr/OpenSearchPPLLexer.g4

+// Operators. Bit
+
+BIT_NOT_OP:                         '~';
+//BIT_OR_OP:                          '|';


commented code - can we remove?

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

…ount check. Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

codecov · 2022-05-31T03:28:01Z

Codecov Report

Merging #61 (cba5749) into dev-simple_query_string-#192 (26058b8) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@                        Coverage Diff                         @@
##             dev-simple_query_string-#192      #61      +/-   ##
==================================================================
+ Coverage                           97.66%   97.70%   +0.04%     
- Complexity                           2743     2767      +24     
==================================================================
  Files                                 268      267       -1     
  Lines                                6772     6891     +119     
  Branches                              435      436       +1     
==================================================================
+ Hits                                 6614     6733     +119     
  Misses                                157      157              
  Partials                                1        1

Flag	Coverage Δ
sql-engine	`97.70% <100.00%> (+0.04%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ensearch/sql/expression/ExpressionNodeVisitor.java	`100.00% <ø> (ø)`
...arch/storage/script/filter/FilterQueryBuilder.java	`100.00% <ø> (ø)`
...rg/opensearch/sql/analysis/ExpressionAnalyzer.java	`100.00% <100.00%> (ø)`
...opensearch/sql/data/model/ExprCollectionValue.java	`100.00% <100.00%> (ø)`
...c/main/java/org/opensearch/sql/expression/DSL.java	`100.00% <100.00%> (ø)`
...h/sql/expression/function/BuiltinFunctionName.java	`100.00% <100.00%> (ø)`
...h/sql/expression/function/OpenSearchFunctions.java	`100.00% <100.00%> (ø)`
...age/script/filter/lucene/relevance/MatchQuery.java	`100.00% <100.00%> (ø)`
...ilter/lucene/relevance/SimpleQueryStringQuery.java	`100.00% <100.00%> (ø)`
...pensearch/sql/ppl/parser/AstExpressionBuilder.java	`100.00% <100.00%> (ø)`
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 26058b8...cba5749. Read the comment docs.

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

forestmvey · 2022-05-31T16:09:09Z

docs/user/dql/functions.rst

+Another example to show how to set custom values for the optional parameters::
+
+    os> select firstname, lastname, city, address from accounts where simple_query_string(['firstname', city ^ 2], 'Amber Nogal', analyzer=keyword, default_operator='AND');
+    fetched rows / total rows = 0/0


Maybe we should consider adding some additional test data for these tests. Perhaps out of the scope of this ticket but it would be nice having clear statements that show usage of these tests by returning exemplary data. It seems hard to do this with the optional parameters and the small amount of queryable data we currently have.

I consider using more complicated samples as I did in integration test. Prior to that we need to reduce the data set to have it short and simple. See b2639e3.

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

…v-simple_query_string-#192-impl2 Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

…ava`. Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

MaxKsyunz · 2022-06-01T00:09:21Z

core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java

+        .entrySet()
+        .stream()
+        .collect(Collectors.toMap(
+            n -> (String) ((Literal) n.getKey()).getValue(),


Would be great if we could do this without the casts.

I have to convert there Map<UnresolvedExpression, UnresolvedExpression> into a Map<String, ExprValue>.

First cast can be simplified to n.getKey().toString(), but I don't see a way to simplify the second one.

MaxKsyunz · 2022-06-01T00:16:17Z

core/src/main/java/org/opensearch/sql/ast/expression/RelevanceFieldList.java

+    return fieldList
+        .entrySet()
+        .stream()
+        .map(e -> String.format("\"%s\" ^ %s", e.getKey().toString(), e.getValue().toString()))


I believe toString is unnecessary here -- String.format takes care of it.

Thank you, fixed in 71bf20c.

MaxKsyunz · 2022-06-01T00:18:28Z

core/src/main/java/org/opensearch/sql/data/model/ExprCollectionValue.java

@@ -45,7 +45,7 @@ public List<ExprValue> collectionValue() {
  public String toString() {
    return valueList.stream()
        .map(Object::toString)
-        .collect(Collectors.joining(",", "[", "]"));
+        .collect(Collectors.joining(", ", "[", "]"));


Was this change necessary to make a test pass?

No, it just makes the string representation a bit more readable. I was the first user of it.

MaxKsyunz · 2022-06-01T04:55:31Z

legacy/src/test/java/org/opensearch/sql/legacy/antlr/semantic/SemanticAnalyzerBasicTest.java

@@ -59,7 +61,7 @@ public void contextShouldIncludeAllFieldsAfterVisitingIndexNameInFromClause() {
        Map<String, Type> typeByName = context.peek().resolveAll(Namespace.FIELD_NAME);
        assertThat(
            typeByName,
-            allOf(
+            Matchers.<Map<String, Type>>allOf(
                aMapWithSize(21),
                hasEntry("semantics", new OpenSearchIndex("semantics", INDEX)),


This line is different in upstream/main.

Compare to https://github.com/opensearch-project/sql/blob/26058b8436b47687bb6039a6431c17a6d5d59f35/legacy/src/test/java/org/opensearch/sql/legacy/antlr/semantic/SemanticAnalyzerBasicTest.java#L64

The (Type) cast is removed by IntelliJ's Code Cleanup which then required addition of th

I don't know how you ran into this but that's what I experienced.

Thank you, fixed in 65ec516.

MaxKsyunz · 2022-06-01T05:00:28Z

core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java

         numParameters <= MIN_NUM_PARAMETERS + numOptionalParameters;
         numParameters++) {
-      List<ExprType> args = new ArrayList<>(Collections.nCopies(numParameters, STRING));
+      List<ExprType> args = new ArrayList<>(Collections.nCopies(numParameters - 1, STRING));


Why is -1 necessary?

upstream/main does not have -1 here nor on line 84 above.

I add one more parameter on the next line. It may be of another type, so I introduced additional function argument.
I agree that it is an ugly implementation.

MaxKsyunz · 2022-06-01T05:01:08Z

legacy/src/test/java/org/opensearch/sql/legacy/antlr/semantic/scope/EnvironmentTest.java

@@ -116,7 +119,7 @@ public void defineFieldSymbolShouldBeAbleToResolveAll() {
        Map<String, Type> typeByName = environment().resolveAll(Namespace.FIELD_NAME);
        assertThat(
            typeByName,
-            allOf(
+            Matchers.<Map<String, Type>>allOf(


Same as SemanticAnalyzerBasicTest.java this file is different in upstream/main.

Thank you, fixed in 65ec516.

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

…ace. Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

acarbonetto · 2022-06-01T21:54:28Z

core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java

+
+  private static FunctionResolver match_bool_prefix() {
+    FunctionName name = BuiltinFunctionName.MATCH_BOOL_PREFIX.getName();
+    // TODO: Create different resolver more suited for relevance functions.


Should we do this?

It is caused by the merge. Fixed in cba5749, the merge was reverted.

acarbonetto · 2022-06-01T21:55:35Z

build.gradle

@@ -6,9 +6,9 @@

 buildscript {
    ext {
-        opensearch_version = System.getProperty("opensearch.version", "2.0.0-rc1-SNAPSHOT")
+        opensearch_version = System.getProperty("opensearch.version", "2.0.0-SNAPSHOT")


not sure why this is in your PR - is this due to a downmerge from main?

Yes, it is caused by the merge. Fixed in cba5749, the merge was reverted.

acarbonetto · 2022-06-01T21:56:29Z

core/src/test/java/org/opensearch/sql/analysis/ExpressionAnalyzerTest.java

-  // Test is disabled because DSL and AstDSL expressions always has fields in the different order
-  // regardless of how they are specified
+  // Test is disabled because DSL and AstDSL expressions sometimes has fields in the different order
+  // regardless of how they are specified. Test passes and fails in a flaky behavior.


This is unfortunate.
Can we raise a follow-up ticket to resolve this in a timely manner?

Fixed in 048984c.

…utableMap` from Guava. Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

acarbonetto · 2022-06-02T20:13:49Z

build.gradle

@@ -6,9 +6,9 @@

 buildscript {
    ext {
-        opensearch_version = System.getProperty("opensearch.version", "2.0.0-SNAPSHOT")
+        opensearch_version = System.getProperty("opensearch.version", "2.0.0-rc1-SNAPSHOT")


This will revert changes that are in the upstream/main branch.
Let's be careful that we aren't undoing Joshua's work.

Yury-Fridlyand · 2022-06-06T21:43:24Z

Reimplemented in #66.

MaxKsyunz and others added 3 commits May 17, 2022 15:16

Support MATCH_BOOL_PREFIX in SQL parser.

a422774

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

Add an interface for PPL and SQL parsers.

838da0f

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

Implementation for simple_query_string.

5b346ea

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

This was referenced May 18, 2022

Implementation of simple_query_string #54

Closed

Update for SQL and PPL parser to accept simple_query_string #53

Closed

MaxKsyunz added 6 commits May 17, 2022 17:16

Implement Parser interface on PPLSyntaxParser.

f57429c

Required renaming analyzeSyntax to parse. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

Add base class for all SQL tests.

e80a334

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

Add match_bool_prefix to list of known functions.

531f31c

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

Add match_bool_prefix to DSL.

1b85a9e

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

Add MatchBoolPrefixQuery query builder.

dd4f77b

Added base class that is commont to match_query and match_bool_prefix and others. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

Add MatchBoolPrefixQuery to FilterQueryBuilder.

1ba94d9

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

MaxKsyunz requested changes May 18, 2022

View reviewed changes

Fix code style.

a51c0ca

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

acarbonetto requested changes May 18, 2022

View reviewed changes

MaxKsyunz and others added 15 commits May 18, 2022 17:50

Address PR comments.

1855e39

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

100% unit test coverage for RelevanceQuery class.

4321fc1

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

Add integration test for match_bool_prefix in SQL.

92087d9

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

Renaming to make code clearer.

c8a5da2

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Fix parser to accept strings only for field.

94748b5

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Fix tests according to the parser update.

d5def08

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Add DSL test.

3a36a0f

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Remaning to fit code standard.

2b4a99b

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Typo fix.

84d84e1

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Typo fix for c8a5da2.

d379af2

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Enable MatchBoolPrefixParserTest tests since they now pass.

56ead1b

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

Update parser syntax. Rework on field list as on 2D array.

bea74fe

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Simplify processing of RelevanceFieldList.

ebf563c

Removed `RelevanceField` as not needed anymore. Since parsing result is stored in `ExprTupleValue` which is actually a map, so we can keep data there. Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Add user documentation for match_bool_prefix.

a149dd7

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

acarbonetto approved these changes May 30, 2022

View reviewed changes

MaxKsyunz and others added 6 commits May 30, 2022 13:45

Merge from upstream/main and resolve conflicts.

5ccadd5

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>

ANTLR: Extend list of relenvace function arguments.

b879b03

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Small optimization in AST processing. Code clean-up. Added argument c…

a7b0a8c

…ount check. Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Add parser tests.

4e43ccd

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Add rest unit and DSL tests.

a641212

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Add integration tests.

b2639e3

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Add doctest.

f7923ee

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

forestmvey reviewed May 31, 2022

View reviewed changes

forestmvey approved these changes May 31, 2022

View reviewed changes

Yury-Fridlyand added 4 commits May 31, 2022 15:37

Update test notes.

33edff4

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Merge remote-tracking branch 'origin/opensearch-project-main' into de…

7c3ff71

…v-simple_query_string-#192-impl2 Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Fix legacy test compilation errors, gradle task `:legacy:compileTestJ…

b3bf2c8

…ava`. Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Fix merge errors.

928faf5

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

MaxKsyunz requested changes Jun 1, 2022

View reviewed changes

Yury-Fridlyand added 5 commits May 31, 2022 22:47

Code style.

5541d95

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Rework SimpleQueryStringQuery to use the RelevanceFunction interf…

de179c3

…ace. Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Add more unit tests and test notes.

a9d5c19

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Fix merge errors in legacy test. Revert b3bf2c8.

65ec516

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Minor optimizations according to PR feedback.

71bf20c

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

acarbonetto requested changes Jun 1, 2022

View reviewed changes

Yury-Fridlyand added 2 commits June 1, 2022 18:07

Fix flakyness of regular Map (HashMap), by replacing it with `Imm…

048984c

…utableMap` from Guava. Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Revert merge with opensearch-project-main, commit 7c3ff71.

cba5749

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

acarbonetto reviewed Jun 2, 2022

View reviewed changes

Yury-Fridlyand mentioned this pull request Jun 2, 2022

simple_query_string implementation. #66

Merged

6 tasks

Yury-Fridlyand closed this Jun 6, 2022

Yury-Fridlyand deleted the dev-simple_query_string-#192-impl2 branch June 6, 2022 21:43

		@@ -252,9 +254,16 @@ public UnresolvedExpression visitConvertedDataType(ConvertedDataTypeContext ctx)

		@Override
		public UnresolvedExpression visitRelevanceExpression(RelevanceExpressionContext ctx) {

Implementation for simple_query_string. #61

Implementation for simple_query_string. #61

Conversation

Yury-Fridlyand commented May 18, 2022 • edited Loading

Description

Issues Resolved

Check List

MaxKsyunz left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yury-Fridlyand May 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaxKsyunz May 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yury-Fridlyand May 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented May 31, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yury-Fridlyand Jun 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yury-Fridlyand commented Jun 6, 2022

Implementation for `simple_query_string`. #61

Implementation for `simple_query_string`. #61

Yury-Fridlyand commented May 18, 2022 •

edited

Loading

MaxKsyunz left a comment •

edited

Loading

Yury-Fridlyand May 19, 2022 •

edited

Loading

MaxKsyunz May 18, 2022 •

edited

Loading

Yury-Fridlyand May 19, 2022 •

edited

Loading

codecov bot commented May 31, 2022 •

edited

Loading

Yury-Fridlyand Jun 1, 2022 •

edited

Loading