fabriziofortino commented on a change in pull request #494:
URL: https://github.com/apache/jackrabbit-oak/pull/494#discussion_r806623002
##########
File path:
oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticIndexHelper.java
##########
@@ -276,6 +276,11 @@ private static void mapIndexRules(ElasticIndexDefinition
indexDefinition, XConte
mappingBuilder.endObject();
}
+ mappingBuilder.startObject(ElasticIndexDefinition.DYNAMIC_BOOST_TAGS)
+ .field("type", "text")
+ .field("analyzer", "oak_analyzer")
+ .endObject();
+
Review comment:
I would change this. We currently support one or more dynamicBoost
properties. Setting this to a fixed field won't work in case more than one
field is configured. The same mapping can be moved in the dynamicBoost for-loop
after line 276. I would also rename the field name using the `:fulltext` suffix:
```java
mappingBuilder.startObject(pd.nodeName + FieldNames.FULLTEXT)
.field("type", "text")
.field("analyzer", "oak_analyzer")
.endObject();
```
This would require changes in `ElasticDocument` and `ElasticRequestHandler`
##########
File path:
oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/ElasticRequestHandler.java
##########
@@ -568,7 +568,7 @@ private boolean visitTerm(String propertyName, String text,
String boost, boolea
if (boost != null) {
fullTextQuery.boost(Float.parseFloat(boost));
}
- BoolQueryBuilder shouldBoolQueryWrapper =
boolQuery().should(fullTextQuery);
+ BoolQueryBuilder shouldBoolQueryWrapper =
boolQuery().must(fullTextQuery);
Review comment:
We could simplify this by removing a level of nesting: `fullTextQuery`
can be passed as `must` clause at line 575, the rest (dynamic score queries)
should go in the `should` clause.
##########
File path:
oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticDynamicBoostQueryTest.java
##########
@@ -152,15 +152,53 @@ public void testQueryDynamicBoostOrder() throws Exception
{
});
}
- // dynamic boost: space is explained as OR instead of AND, this should be
documented
@Test
- public void testQueryDynamicBoostSpace() throws Exception {
+ public void testQueryDynamicBoostWildcard() throws Exception {
configureIndex();
prepareTestAssets();
assertEventually(() -> {
- assertQuery("select [jcr:path] from [dam:Asset] where
contains(@title, 'blue flower')", SQL2,
+ assertQuery("select [jcr:path] from [dam:Asset] where
contains(@title, 'blu?')", SQL2, Arrays.asList("/test/asset3"));
+ assertQuery("select [jcr:path] from [dam:Asset] where
contains(@title, 'bl?e')", SQL2, Arrays.asList("/test/asset3"));
+ assertQuery("select [jcr:path] from [dam:Asset] where
contains(@title, '?lue')", SQL2, Arrays.asList("/test/asset3"));
+ assertQuery("select [jcr:path] from [dam:Asset] where
contains(@title, 'coff*')", SQL2, Arrays.asList("/test/asset2"));
+ assertQuery("select [jcr:path] from [dam:Asset] where
contains(@title, 'co*ee')", SQL2, Arrays.asList("/test/asset2"));
+ assertQuery("select [jcr:path] from [dam:Asset] where
contains(@title, '*ffee')", SQL2, Arrays.asList("/test/asset2"));
+ });
+ }
+
+ @Test
+ public void testQueryDynamicBoosSpace() throws Exception {
Review comment:
typo
```suggestion
public void testQueryDynamicBoostSpace() throws Exception {
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
