CommonTermsQuery fix for ignored coordination factor

CommonTermsQueryParser does not check for disable_coords, only for disable_coord. Yet the builder only outputs disable_coords, leading to disabling the coordination factor to be ignored in the Java API. Closes elastic#11730 Closes elastic#11780
rjernst · Jun 19, 2015 · 9b4a6e8 · 9b4a6e8
1 parent 7d79796
commit 9b4a6e8
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 7 deletions.
diff --git a/core/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java
@@ -64,7 +64,7 @@ public static enum Operator {
 
     private String highFreqMinimumShouldMatch = null;
 
-    private Boolean disableCoords = null;
+    private Boolean disableCoord = null;
 
     private Float cutoffFrequency = null;
 
@@ -150,6 +150,11 @@ public CommonTermsQueryBuilder lowFreqMinimumShouldMatch(String lowFreqMinimumSh
         this.lowFreqMinimumShouldMatch = lowFreqMinimumShouldMatch;
         return this;
     }
+
+    public CommonTermsQueryBuilder disableCoord(boolean disableCoord) {
+        this.disableCoord = disableCoord;
+        return this;
+    }
 
     /**
      * Sets the query name for the filter that can be used when searching for matched_filters per hit.
@@ -165,8 +170,8 @@ public void doXContent(XContentBuilder builder, Params params) throws IOExceptio
         builder.startObject(name);
 
         builder.field("query", text);
-        if (disableCoords != null) {
-            builder.field("disable_coords", disableCoords);
+        if (disableCoord != null) {
+            builder.field("disable_coord", disableCoord);
         }
         if (highFreqOperator != null) {
             builder.field("high_freq_operator", highFreqOperator.toString());

diff --git a/core/src/main/java/org/elasticsearch/index/query/CommonTermsQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/CommonTermsQueryParser.java
@@ -47,7 +47,7 @@ public class CommonTermsQueryParser implements QueryParser {
 
     static final Occur DEFAULT_LOW_FREQ_OCCUR = Occur.SHOULD;
 
-    static final boolean DEFAULT_DISABLE_COORDS = true;
+    static final boolean DEFAULT_DISABLE_COORD = true;
 
 
     @Inject
@@ -72,7 +72,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
         String queryAnalyzer = null;
         String lowFreqMinimumShouldMatch = null;
         String highFreqMinimumShouldMatch = null;
-        boolean disableCoords = DEFAULT_DISABLE_COORDS;
+        boolean disableCoord = DEFAULT_DISABLE_COORD;
         Occur highFreqOccur = DEFAULT_HIGH_FREQ_OCCUR;
         Occur lowFreqOccur = DEFAULT_LOW_FREQ_OCCUR;
         float maxTermFrequency = DEFAULT_MAX_TERM_DOC_FREQ;
@@ -113,7 +113,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
                         }
                         queryAnalyzer = analyzer;
                     } else if ("disable_coord".equals(currentFieldName) || "disableCoord".equals(currentFieldName)) {
-                        disableCoords = parser.booleanValue();
+                        disableCoord = parser.booleanValue();
                     } else if ("boost".equals(currentFieldName)) {
                         boost = parser.floatValue();
                     } else if ("high_freq_operator".equals(currentFieldName) || "highFreqOperator".equals(currentFieldName)) {
@@ -188,7 +188,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
             }
         }
 
-        ExtendedCommonTermsQuery commonsQuery = new ExtendedCommonTermsQuery(highFreqOccur, lowFreqOccur, maxTermFrequency, disableCoords, fieldType);
+        ExtendedCommonTermsQuery commonsQuery = new ExtendedCommonTermsQuery(highFreqOccur, lowFreqOccur, maxTermFrequency, disableCoord, fieldType);
         commonsQuery.setBoost(boost);
         Query query = parseQueryString(commonsQuery, value.toString(), field, parseContext, analyzer, lowFreqMinimumShouldMatch, highFreqMinimumShouldMatch);
         if (queryName != null) {

diff --git a/core/src/test/java/org/elasticsearch/index/query/SimpleIndexQueryParserTests.java b/core/src/test/java/org/elasticsearch/index/query/SimpleIndexQueryParserTests.java
@@ -2170,6 +2170,19 @@ public void testCommonTermsQuery3() throws IOException {
         assertThat(ectQuery.getLowFreqMinimumNumberShouldMatchSpec(), equalTo("2"));
     }
 
+    @Test // see #11730
+    public void testCommonTermsQuery4() throws IOException {
+        IndexQueryParserService queryParser = queryParser();
+        Query parsedQuery = queryParser.parse(commonTermsQuery("field", "text").disableCoord(false)).query();
+        assertThat(parsedQuery, instanceOf(ExtendedCommonTermsQuery.class));
+        ExtendedCommonTermsQuery ectQuery = (ExtendedCommonTermsQuery) parsedQuery;
+        assertFalse(ectQuery.isCoordDisabled());
+        parsedQuery = queryParser.parse(commonTermsQuery("field", "text").disableCoord(true)).query();
+        assertThat(parsedQuery, instanceOf(ExtendedCommonTermsQuery.class));
+        ectQuery = (ExtendedCommonTermsQuery) parsedQuery;
+        assertTrue(ectQuery.isCoordDisabled());
+    }
+
     @Test(expected = QueryParsingException.class)
     public void assureMalformedThrowsException() throws IOException {
         IndexQueryParserService queryParser;