order by _score must be defined explicitly now

CHANGES.txt

@@ -5,6 +5,10 @@ Changes for Crate
 Unreleased
 ==========
 
+ - Ordering by `_score` must be defined explictly now even when using a
+   `match` predicate.
+   Note: This is a backward incompatibility change.
+
  - Fix: Filtering by `_score` wasn't recognized on group-by queries.
 
  - Fix: Do not shutdown the BulkShardProcessor while there are pending requests

docs/sql/fulltext.txt

@@ -37,7 +37,7 @@ To get the relevance of a matching row, an internal system column
 It is a numeric value which is not absolute but relative to the other rows.
 The higher the score value, the more relevant the row::
 
-    cr> select name, _score from locations where match(name_description_ft, 'time');
+    cr> select name, _score from locations where match(name_description_ft, 'time') order by _score desc;
     +-----------+------------+
     | name      |     _score |
     +-----------+------------+
@@ -198,7 +198,7 @@ Usage
 
 A fulltext search is done using the :ref:`predicates_match` predicate::
 
-    cr> select name from locations where match(name_description_ft, 'time');
+    cr> select name from locations where match(name_description_ft, 'time') order by _score desc;
     +-----------+
     | name      |
     +-----------+
@@ -212,7 +212,7 @@ information about the quality of a match, the relevance of the row,
 the :ref:`_score <sql_ddl_system_column_score>` can be selected::
 
     cr> select name, _score
-    ... from locations where match(name_description_ft, 'time');
+    ... from locations where match(name_description_ft, 'time') order by _score desc;
     +-----------+------------+
     | name      |     _score |
     +-----------+------------+
@@ -221,24 +221,11 @@ the :ref:`_score <sql_ddl_system_column_score>` can be selected::
     +-----------+------------+
     SELECT 2 rows in set (... sec)
 
-The results of a query using MATCH are sorted by :ref:`_score <sql_ddl_system_column_score>`
-in a descending order by default. Of course it is possible to change it to use an ascending order instead::
-
-    cr> select name, _score
-    ... from locations where match(name_description_ft, 'time')
-    ... order by _score asc;
-    +-----------+------------+
-    | name      |     _score |
-    +-----------+------------+
-    | Bartledan | 0.4590714  |
-    | Altair    | 0.56319076 |
-    +-----------+------------+
-    SELECT 2 rows in set (... sec)
-
 .. note::
 
-    The `_score` is a relative and not an absolute value. It just sets a row
-    in relation to the other ones.
+    The `_score` is a relative and not an absolute value. It just sets
+    a row in relation to the other ones. By default, results are not
+    ordered. Ordering by `_score` must be defined explicitly.
 
 
 Searching On Multiple Columns
@@ -266,7 +253,7 @@ use ``phrase`` or ``phrase_prefix``::
     ... where match(
     ...     (name_description_ft, race['name'] 1.5, kind 0.75),
     ...     'end of the galaxy'
-    ... );
+    ... ) order by _score desc;
     +-------------------+-------------+
     | name              |      _score |
     +-------------------+-------------+

sql/src/main/java/io/crate/planner/consumer/QueryThenFetchConsumer.java

@@ -53,7 +53,6 @@
 import io.crate.types.DataTypes;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.List;
 
 public class QueryThenFetchConsumer implements Consumer {
@@ -110,6 +109,7 @@ public PlannedAnalyzedRelation visitQueriedTable(QueriedTable table, ConsumerCon
                 outputSymbols.add(DocReferenceConverter.convertIfPossible(symbol, tableInfo));
             }
 
+            List<Projection> collectProjections = new ArrayList<>();
             OrderBy orderBy = table.querySpec().orderBy();
             if (orderBy != null) {
                 table.tableRelation().validateOrderBy(orderBy);
@@ -119,6 +119,13 @@ public PlannedAnalyzedRelation visitQueriedTable(QueriedTable table, ConsumerCon
                         collectSymbols.add(symbol);
                     }
                 }
+
+                MergeProjection mergeProjection = new MergeProjection(
+                        collectSymbols,
+                        orderBy.orderBySymbols(),
+                        orderBy.reverseFlags(),
+                        orderBy.nullsFirst());
+                collectProjections.add(mergeProjection);
             }
 
             CollectNode collectNode = PlanNodeBuilder.collect(
@@ -131,31 +138,23 @@ public PlannedAnalyzedRelation visitQueriedTable(QueriedTable table, ConsumerCon
                     MoreObjects.firstNonNull(table.querySpec().limit(), Constants.DEFAULT_SELECT_LIMIT) + table.querySpec().offset()
             );
             collectNode.keepContextForFetcher(true);
-
-            if (orderBy != null) {
-                MergeProjection mergeProjection = new MergeProjection(
-                        collectSymbols,
-                        orderBy.orderBySymbols(),
-                        orderBy.reverseFlags(),
-                        orderBy.nullsFirst());
-                collectNode.projections(ImmutableList.<Projection>of(mergeProjection));
-            }
+            collectNode.projections(collectProjections);
 
             List<Projection> mergeProjections = new ArrayList<>();
-            TopNProjection topNProjection = new TopNProjection(
-                    MoreObjects.firstNonNull(table.querySpec().limit(), Constants.DEFAULT_SELECT_LIMIT),
-                    table.querySpec().offset(),
-                    scoreSelected ? Arrays.<Symbol>asList(DEFAULT_SCORE_INPUT_COLUMN) : ImmutableList.<Symbol>of(),
-                    scoreSelected ? new boolean[]{true} : new boolean[0],
-                    scoreSelected ? new Boolean[]{false} : new Boolean[0]
-            );
-            List<Symbol> outputs = new ArrayList<>(2);
-            outputs.add(DEFAULT_DOC_ID_INPUT_COLUMN);
-            if (scoreSelected) {
-                outputs.add(DEFAULT_SCORE_INPUT_COLUMN);
+
+            if (table.querySpec().isLimited()) {
+                TopNProjection topNProjection = new TopNProjection(
+                        MoreObjects.firstNonNull(table.querySpec().limit(), Constants.DEFAULT_SELECT_LIMIT),
+                        table.querySpec().offset()
+                );
+                List<Symbol> outputs = new ArrayList<>(2);
+                outputs.add(DEFAULT_DOC_ID_INPUT_COLUMN);
+                if (scoreSelected) {
+                    outputs.add(DEFAULT_SCORE_INPUT_COLUMN);
+                }
+                topNProjection.outputs(outputs);
+                mergeProjections.add(topNProjection);
             }
-            topNProjection.outputs(outputs);
-            mergeProjections.add(topNProjection);
 
             FetchProjection fetchProjection = new FetchProjection(
                     DEFAULT_DOC_ID_INPUT_COLUMN, collectSymbols, outputSymbols,

sql/src/main/java/io/crate/planner/projection/TopNProjection.java

@@ -48,8 +48,11 @@ public TopNProjection newInstance() {
 
     List<Symbol> outputs = ImmutableList.of();
 
+    @Nullable
     List<Symbol> orderBy;
+    @Nullable
     boolean[] reverseFlags;
+    @Nullable
     private Boolean[] nullsFirst;
 
     public TopNProjection() {
@@ -87,14 +90,17 @@ public int offset() {
         return offset;
     }
 
+    @Nullable
     public List<Symbol> orderBy() {
         return orderBy;
     }
 
+    @Nullable
     public boolean[] reverseFlags() {
         return reverseFlags;
     }
 
+    @Nullable
     public Boolean[] nullsFirst() {
         return nullsFirst;
     }

sql/src/test/java/io/crate/integrationtests/TransportSQLActionTest.java

@@ -1904,7 +1904,7 @@ public void testSelectWhereMultiColumnMatchDifferentTypesDifferentScore() throws
         this.setup.setUpLocations();
         refresh();
         execute("select name, description, kind, _score from locations " +
-                "where match((kind, name_description_ft 0.5), 'Planet earth') using most_fields with (analyzer='english')");
+                "where match((kind, name_description_ft 0.5), 'Planet earth') using most_fields with (analyzer='english') order by _score desc");
         assertThat(response.rowCount(), is(5L));
         assertThat(TestingHelpers.printedTable(response.rows()),
                 is("Alpha Centauri| 4.1 light-years northwest of earth| Star System| 0.049483635\n" +
@@ -1913,7 +1913,7 @@ public void testSelectWhereMultiColumnMatchDifferentTypesDifferentScore() throws
                         "Galactic Sector QQ7 Active J Gamma| Galactic Sector QQ7 Active J Gamma contains the Sun Zarss, the planet Preliumtarn of the famed Sevorbeupstry and Quentulus Quazgar Mountains.| Galaxy| 0.017716927\n"));
 
         execute("select name, description, kind, _score from locations " +
-                "where match((kind, name_description_ft 0.5), 'Planet earth') using cross_fields");
+                "where match((kind, name_description_ft 0.5), 'Planet earth') using cross_fields order by _score desc");
         assertThat(response.rowCount(), is(5L));
         assertThat(TestingHelpers.printedTable(response.rows()),
                 is("Alpha Centauri| 4.1 light-years northwest of earth| Star System| 0.06658964\n" +
@@ -1941,7 +1941,7 @@ public void testSimpleMatchWithBoost() throws Exception {
         refresh();
         execute("select characters.name AS characters_name, _score " +
                 "from characters " +
-                "where match(characters.quote_ft 1.0, 'country')");
+                "where match(characters.quote_ft 1.0, 'country') order by _score desc");
         assertThat(response.rows().length, is(2));
         assertThat((String) response.rows()[0][0], is("Trillian"));
         assertThat((float) response.rows()[0][1], is(0.15342641f));
@@ -1953,15 +1953,15 @@ public void testSimpleMatchWithBoost() throws Exception {
     public void testMatchTypes() throws Exception {
         this.setup.setUpLocations();
         refresh();
-        execute("select name, _score from locations where match((kind 0.8, name_description_ft 0.6), 'planet earth') using best_fields");
+        execute("select name, _score from locations where match((kind 0.8, name_description_ft 0.6), 'planet earth') using best_fields order by _score desc");
         assertThat(TestingHelpers.printedTable(response.rows()),
                 is("Alpha Centauri| 0.22184466\n| 0.21719791\nAllosimanius Syneca| 0.09626817\nBartledan| 0.08423465\nGalactic Sector QQ7 Active J Gamma| 0.08144922\n"));
 
-        execute("select name, _score from locations where match((kind 0.6, name_description_ft 0.8), 'planet earth') using most_fields");
+        execute("select name, _score from locations where match((kind 0.6, name_description_ft 0.8), 'planet earth') using most_fields order by _score desc");
         assertThat(TestingHelpers.printedTable(response.rows()),
                 is("Alpha Centauri| 0.12094267\n| 0.1035407\nAllosimanius Syneca| 0.05248235\nBartledan| 0.045922056\nGalactic Sector QQ7 Active J Gamma| 0.038827762\n"));
 
-        execute("select name, _score from locations where match((kind 0.4, name_description_ft 1.0), 'planet earth') using cross_fields");
+        execute("select name, _score from locations where match((kind 0.4, name_description_ft 1.0), 'planet earth') using cross_fields order by _score desc");
         assertThat(TestingHelpers.printedTable(response.rows()),
                 is("Alpha Centauri| 0.14147125\n| 0.116184436\nAllosimanius Syneca| 0.061390605\nBartledan| 0.05371678\nGalactic Sector QQ7 Active J Gamma| 0.043569162\n"));
 
@@ -1987,7 +1987,7 @@ public void testMatchOptions() throws Exception {
         execute("select name, _score from locations where match((kind, name_description_ft), 'galaxy') " +
                 "using best_fields with (fuzziness=0.5) order by _score desc");
         assertThat(TestingHelpers.printedTable(response.rows()),
-                is("End of the Galaxy| 1.4109559\nOuter Eastern Rim| 1.4109559\nNorth West Ripple| 1.2808706\nGalactic Sector QQ7 Active J Gamma| 1.2808706\nAltair| 0.3842612\nAlgol| 0.25617412\n"));
+                is("Outer Eastern Rim| 1.4109559\nEnd of the Galaxy| 1.4109559\nNorth West Ripple| 1.2808706\nGalactic Sector QQ7 Active J Gamma| 1.2808706\nAltair| 0.3842612\nAlgol| 0.25617412\n"));
 
         execute("select name, _score from locations where match((kind, name_description_ft), 'gala') " +
                 "using best_fields with (operator='or', minimum_should_match=2) order by _score desc");

sql/src/test/java/io/crate/planner/PlannerTest.java

@@ -503,21 +503,21 @@ public void testQueryThenFetchPlanDefaultLimit() throws Exception {
         assertThat(collectNode.limit(), is(Constants.DEFAULT_SELECT_LIMIT));
 
         MergeNode mergeNode = plan.mergeNode();
-        TopNProjection topN = (TopNProjection)mergeNode.projections().get(0);
-        assertThat(topN.limit(), is(Constants.DEFAULT_SELECT_LIMIT));
-        assertThat(topN.offset(), is(0));
-        assertThat(topN.orderBy().size(), is(0));
+        assertThat(mergeNode.projections().size(), is(1));
+        assertThat(mergeNode.finalProjection().get(), instanceOf(FetchProjection.class));
 
         // with offset
         plan = (QueryThenFetch)plan("select name from users offset 20");
         collectNode = plan.collectNode();
         assertThat(collectNode.limit(), is(Constants.DEFAULT_SELECT_LIMIT + 20));
 
         mergeNode = plan.mergeNode();
-        topN = (TopNProjection)mergeNode.projections().get(0);
+        assertThat(mergeNode.projections().size(), is(2));
+        assertThat(mergeNode.finalProjection().get(), instanceOf(FetchProjection.class));
+        TopNProjection topN = (TopNProjection)mergeNode.projections().get(0);
         assertThat(topN.limit(), is(Constants.DEFAULT_SELECT_LIMIT));
         assertThat(topN.offset(), is(20));
-        assertThat(topN.orderBy().size(), is(0));
+        assertNull(topN.orderBy());
     }
 
     @Test