[GitHub] [jackrabbit-oak] thomasmueller commented on a change in pull request #424: Support allowLeadingWildcard for elastic

[GitBox <gi...@apache.org>]

thomasmueller commented on a change in pull request #424:
URL: https://github.com/apache/jackrabbit-oak/pull/424#discussion_r772138018



##########
File path: oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/ElasticRequestHandler.java
##########
@@ -787,7 +787,25 @@ private static QueryBuilder fullTextQuery(String text, String fieldName, PlanRes
             // and could contain other parts like renditions, node name, etc
             return multiMatchQuery.field(fieldName);
         } else {
-            return simpleQueryStringQuery(text).field(fieldName).defaultOperator(Operator.AND);
+            // https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-query-string-query.html
+            // simpleQueryStringQuery does not support leading wildcards whereas it's supported by default in queryStringQuery
+            // Not using queryStringQuery by default , since this can have performance impact, also some functional cases break.

Review comment:
       It would be good to know the details (as much as you know them or have a link to documentation) for "this can have performance impact" and "some functional cases break". It is fine if this is documented in the jira issue I think (but then either the commit message or the documentation here needs a link to the jira issue), but it's probably even better if this is a comment in the source code.  I think I know why it would break some tests: simpleQueryString is less strict. But performance impact I'm not sure. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org