Fix JSON queries with IN restrictions and ORDER BY clause

patch by Francisco Fernandez; reviewed by Benjamin Lerer for CASSANDRA-14286


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/594cde79
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/594cde79
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/594cde79

Branch: refs/heads/trunk
Commit: 594cde7937de6f848998bac35d35591f8a18aa10
Parents: b3ac793
Author: Francisco Fernandez <francisco.fernandez.cast...@gmail.com>
Authored: Tue Apr 17 12:02:25 2018 +0200
Committer: Benjamin Lerer <b.le...@gmail.com>
Committed: Tue Apr 17 12:04:33 2018 +0200

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 .../org/apache/cassandra/cql3/ResultSet.java    |  5 ++
 .../cassandra/cql3/selection/Selection.java     | 69 +++++++++++++++++---
 .../cql3/statements/SelectStatement.java        | 19 +++---
 .../cql3/validation/entities/JsonTest.java      | 21 ++++++
 5 files changed, 96 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/594cde79/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 5221b1e..967ee05 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 2.2.13
+ * Fix JSON queries with IN restrictions and ORDER BY clause (CASSANDRA-14286)
  * CQL fromJson(null) throws NullPointerException (CASSANDRA-13891)
  * Fix query pager DEBUG log leak causing hit in paged reads throughput 
(CASSANDRA-14318)
  * Backport circleci yaml (CASSANDRA-14240)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/594cde79/src/java/org/apache/cassandra/cql3/ResultSet.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/ResultSet.java 
b/src/java/org/apache/cassandra/cql3/ResultSet.java
index 028691f..16f0d1b 100644
--- a/src/java/org/apache/cassandra/cql3/ResultSet.java
+++ b/src/java/org/apache/cassandra/cql3/ResultSet.java
@@ -254,6 +254,11 @@ public class ResultSet
             return new ResultMetadata(EnumSet.copyOf(flags), names, 
columnCount, pagingState);
         }
 
+        public int getColumnCount()
+        {
+            return columnCount;
+        }
+
         /**
          * Return only the column names requested by the user, excluding those 
added for post-query re-orderings,
          * see definition of names and columnCount.

http://git-wip-us.apache.org/repos/asf/cassandra/blob/594cde79/src/java/org/apache/cassandra/cql3/selection/Selection.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/selection/Selection.java 
b/src/java/org/apache/cassandra/cql3/selection/Selection.java
index dc4e240..5385fc6 100644
--- a/src/java/org/apache/cassandra/cql3/selection/Selection.java
+++ b/src/java/org/apache/cassandra/cql3/selection/Selection.java
@@ -24,6 +24,7 @@ import com.google.common.base.Objects;
 import com.google.common.base.Predicate;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Iterators;
+import com.google.common.collect.Lists;
 
 import org.apache.cassandra.config.CFMetaData;
 import org.apache.cassandra.config.ColumnDefinition;
@@ -56,6 +57,8 @@ public abstract class Selection
     private final ResultSet.ResultMetadata metadata;
     private final boolean collectTimestamps;
     private final boolean collectTTLs;
+    // Columns used to order the result set for multi-partition queries
+    private Map<ColumnDefinition, Integer> orderingIndex;
 
     protected Selection(CFMetaData cfm,
                         List<ColumnDefinition> columns,
@@ -130,6 +133,24 @@ public abstract class Selection
         return false;
     }
 
+    public Map<ColumnDefinition, Integer> getOrderingIndex(boolean isJson)
+    {
+        if (!isJson)
+            return orderingIndex;
+
+        // If we order post-query in json, the first and only column that we 
ship to the client is the json column.
+        // In that case, we should keep ordering columns around to perform the 
ordering, then these columns will
+        // be placed after the json column. As a consequence of where the 
colums are placed, we should give the
+        // ordering index a value based on their position in the json encoding 
and discard the original index.
+        // (CASSANDRA-14286)
+        int columnIndex = 1;
+        Map<ColumnDefinition, Integer> jsonOrderingIndex = new 
LinkedHashMap<>(orderingIndex.size());
+        for (ColumnDefinition column : orderingIndex.keySet())
+            jsonOrderingIndex.put(column, columnIndex++);
+
+        return jsonOrderingIndex;
+    }
+
     public ResultSet.ResultMetadata getResultMetadata(boolean isJson)
     {
         if (!isJson)
@@ -137,7 +158,13 @@ public abstract class Selection
 
         ColumnSpecification firstColumn = metadata.names.get(0);
         ColumnSpecification jsonSpec = new 
ColumnSpecification(firstColumn.ksName, firstColumn.cfName, 
Json.JSON_COLUMN_ID, UTF8Type.instance);
-        return new ResultSet.ResultMetadata(Arrays.asList(jsonSpec));
+        ResultSet.ResultMetadata resultMetadata = new 
ResultSet.ResultMetadata(Lists.newArrayList(jsonSpec));
+        if (orderingIndex != null)
+        {
+            for (ColumnDefinition orderingColumn : orderingIndex.keySet())
+                resultMetadata.addNonSerializedColumn(orderingColumn);
+        }
+        return resultMetadata;
     }
 
     public static Selection wildcard(CFMetaData cfm)
@@ -152,7 +179,20 @@ public abstract class Selection
         return new SimpleSelection(cfm, columns, false);
     }
 
-    public int addColumnForOrdering(ColumnDefinition c)
+    public void addColumnForOrdering(ColumnDefinition c)
+    {
+        if (orderingIndex == null)
+            orderingIndex = new LinkedHashMap<>();
+
+        int index = getResultSetIndex(c);
+
+        if (index < 0)
+            index = addOrderingColumn(c);
+
+        orderingIndex.put(c, index);
+    }
+
+    protected int addOrderingColumn(ColumnDefinition c)
     {
         columns.add(c);
         metadata.addNonSerializedColumn(c);
@@ -356,14 +396,25 @@ public abstract class Selection
         private List<ByteBuffer> getOutputRow(int protocolVersion)
         {
             List<ByteBuffer> outputRow = 
selectors.getOutputRow(protocolVersion);
-            return isJson ? rowToJson(outputRow, protocolVersion)
-                          : outputRow;
+            if (isJson)
+            {
+                List<ByteBuffer> jsonRow = rowToJson(outputRow, 
protocolVersion);
+
+                // Keep ordering columns around for possible post-query 
ordering. (CASSANDRA-14286)
+                if (orderingIndex != null)
+                {
+                    for (Integer orderingColumnIndex : orderingIndex.values())
+                        jsonRow.add(outputRow.get(orderingColumnIndex));
+                }
+                outputRow = jsonRow;
+            }
+            return outputRow;
         }
 
         private List<ByteBuffer> rowToJson(List<ByteBuffer> row, int 
protocolVersion)
         {
             StringBuilder sb = new StringBuilder("{");
-            for (int i = 0; i < metadata.names.size(); i++)
+            for (int i = 0; i < metadata.getColumnCount(); i++)
             {
                 if (i > 0)
                     sb.append(", ");
@@ -383,7 +434,9 @@ public abstract class Selection
                     sb.append(spec.type.toJSONString(buffer, protocolVersion));
             }
             sb.append("}");
-            return 
Collections.singletonList(UTF8Type.instance.getSerializer().serialize(sb.toString()));
+            List<ByteBuffer> jsonRow = new ArrayList<>();
+            
jsonRow.add(UTF8Type.instance.getSerializer().serialize(sb.toString()));
+            return jsonRow;
         }
 
         private ByteBuffer value(Cell c)
@@ -515,9 +568,9 @@ public abstract class Selection
         }
 
         @Override
-        public int addColumnForOrdering(ColumnDefinition c)
+        protected int addOrderingColumn(ColumnDefinition c)
         {
-            int index = super.addColumnForOrdering(c);
+            int index = super.addOrderingColumn(c);
             factories.addSelectorForOrdering(c, index);
             return factories.size() - 1;
         }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/594cde79/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java 
b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
index 13276c7..729cf83 100644
--- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
@@ -842,7 +842,7 @@ public class SelectStatement implements CQLStatement
             if (!parameters.orderings.isEmpty())
             {
                 verifyOrderingIsAllowed(restrictions);
-                orderingComparator = getOrderingComparator(cfm, selection, 
restrictions);
+                orderingComparator = getOrderingComparator(cfm, selection, 
restrictions, parameters.isJson);
                 isReversed = isReversed(cfm);
             }
 
@@ -942,13 +942,14 @@ public class SelectStatement implements CQLStatement
 
         private Comparator<List<ByteBuffer>> getOrderingComparator(CFMetaData 
cfm,
                                                                    Selection 
selection,
-                                                                   
StatementRestrictions restrictions)
+                                                                   
StatementRestrictions restrictions,
+                                                                   boolean 
isJson)
                                                                    throws 
InvalidRequestException
         {
             if (!restrictions.keyIsInRelation())
                 return null;
 
-            Map<ColumnIdentifier, Integer> orderingIndexes = 
getOrderingIndex(cfm, selection);
+            Map<ColumnDefinition, Integer> orderingIndexes = 
getOrderingIndex(cfm, selection, isJson);
 
             List<Integer> idToSort = new ArrayList<Integer>();
             List<Comparator<ByteBuffer>> sorters = new 
ArrayList<Comparator<ByteBuffer>>();
@@ -957,32 +958,28 @@ public class SelectStatement implements CQLStatement
             {
                 ColumnIdentifier identifier = raw.prepare(cfm);
                 ColumnDefinition orderingColumn = 
cfm.getColumnDefinition(identifier);
-                idToSort.add(orderingIndexes.get(orderingColumn.name));
+                idToSort.add(orderingIndexes.get(orderingColumn));
                 sorters.add(orderingColumn.type);
             }
             return idToSort.size() == 1 ? new 
SingleColumnComparator(idToSort.get(0), sorters.get(0))
                     : new CompositeComparator(sorters, idToSort);
         }
 
-        private Map<ColumnIdentifier, Integer> getOrderingIndex(CFMetaData 
cfm, Selection selection)
+        private Map<ColumnDefinition, Integer> getOrderingIndex(CFMetaData 
cfm, Selection selection, boolean isJson)
                 throws InvalidRequestException
         {
             // If we order post-query (see orderResults), the sorted column 
needs to be in the ResultSet for sorting,
             // even if we don't
             // ultimately ship them to the client (CASSANDRA-4911).
-            Map<ColumnIdentifier, Integer> orderingIndexes = new HashMap<>();
             for (ColumnIdentifier.Raw raw : parameters.orderings.keySet())
             {
                 ColumnIdentifier column = raw.prepare(cfm);
                 final ColumnDefinition def = cfm.getColumnDefinition(column);
                 if (def == null)
                     handleUnrecognizedOrderingColumn(column);
-                int index = selection.getResultSetIndex(def);
-                if (index < 0)
-                    index = selection.addColumnForOrdering(def);
-                orderingIndexes.put(def.name, index);
+                selection.addColumnForOrdering(def);
             }
-            return orderingIndexes;
+            return selection.getOrderingIndex(isJson);
         }
 
         private boolean isReversed(CFMetaData cfm) throws 
InvalidRequestException

http://git-wip-us.apache.org/repos/asf/cassandra/blob/594cde79/test/unit/org/apache/cassandra/cql3/validation/entities/JsonTest.java
----------------------------------------------------------------------
diff --git 
a/test/unit/org/apache/cassandra/cql3/validation/entities/JsonTest.java 
b/test/unit/org/apache/cassandra/cql3/validation/entities/JsonTest.java
index 46cdd52..9d40038 100644
--- a/test/unit/org/apache/cassandra/cql3/validation/entities/JsonTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/entities/JsonTest.java
@@ -1264,4 +1264,25 @@ public class JsonTest extends CQLTester
         executor.shutdown();
         Assert.assertTrue(executor.awaitTermination(30, TimeUnit.SECONDS));
     }
+
+    // CASSANDRA-14286
+    @Test
+    public void testJsonOrdering() throws Throwable
+    {
+        createTable("CREATE TABLE %s( PRIMARY KEY (a, b), a INT, b INT);");
+        execute("INSERT INTO %s(a, b) VALUES (20, 30);");
+        execute("INSERT INTO %s(a, b) VALUES (100, 200);");
+
+        assertRows(execute("SELECT JSON a, b FROM %s WHERE a IN (20, 100) 
ORDER BY b"),
+                   row("{\"a\": 20, \"b\": 30}"),
+                   row("{\"a\": 100, \"b\": 200}"));
+
+        assertRows(execute("SELECT JSON a, b FROM %s WHERE a IN (20, 100) 
ORDER BY b DESC"),
+                   row("{\"a\": 100, \"b\": 200}"),
+                   row("{\"a\": 20, \"b\": 30}"));
+
+        assertRows(execute("SELECT JSON a FROM %s WHERE a IN (20, 100) ORDER 
BY b DESC"),
+                   row("{\"a\": 100}"),
+                   row("{\"a\": 20}"));
+    }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to