[GitHub] [metamodel] arjansh commented on a change in pull request #248: METAMODEL-1233: Elasticsearch uses 'match' query for text based columns conditions in WHERE clause

[GitBox]

arjansh commented on a change in pull request #248:
URL: https://github.com/apache/metamodel/pull/248#discussion_r576699028



##########
File path: 
elasticsearch/common/src/test/java/org/apache/metamodel/elasticsearch/common/ElasticSearchUtilsTest.java
##########
@@ -60,4 +69,40 @@ public void testCreateRowWithParsableDates() throws 
Exception {
         assertTrue(stringValue instanceof String);
         assertTrue(dateValue instanceof Date);
     }
+
+    /**
+     * For text-based conditions a 'match' query is recommended (instead of 
'term' query).
+     */
+    public void testMatchQueryIsCreatedForTextEqualTo() {
+        final SelectItem selectItem = new SelectItem(new 
MutableColumn("column_name", ColumnType.STRING));
+        final FilterItem filterItem = new FilterItem(selectItem, 
OperatorType.EQUALS_TO, "text-value");
+        final QueryBuilder queryBuilder =
+                
ElasticSearchUtils.createQueryBuilderForSimpleWhere(Collections.singletonList(filterItem),
 null);
+        assertEquals("match", queryBuilder.getName());
+    }
+
+    /**
+     * For text-based conditions a 'match' query is recommended (instead of 
'term' query).
+     * In case of 'DIFFERENT_FROM', we need a 'bool' query with 'must not' and 
'match' query.
+     */
+    public void testBoolQueryIsCreatedForTextDifferentFrom() {
+        final SelectItem selectItem = new SelectItem(new 
MutableColumn("column_name", ColumnType.STRING));
+        final FilterItem filterItem = new FilterItem(selectItem, 
OperatorType.DIFFERENT_FROM, "text-value");
+        final QueryBuilder queryBuilder =
+                
ElasticSearchUtils.createQueryBuilderForSimpleWhere(Collections.singletonList(filterItem),
 null);
+        assertEquals("bool", queryBuilder.getName());

Review comment:
       Can you also add assertions to validate that the queryBuilder contains 
the "must not" and "match" parts in the query?
   
   Technically, you can add something like this:
   
   ```
           final Map<String, Object> queryMap = XContentHelper
                   .convertToMap(XContentType.JSON.xContent(), 
queryBuilder.toString(), false);
   ```
   
   That converts the queryBuilder object to a Map containing the values you're 
interested in, so then you can add some assertions to check if it contains what 
you're expecting. The same applies to the other test case (below this one)




----------------------------------------------------------------
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.

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