adelapena commented on code in PR #2125:
URL: https://github.com/apache/cassandra/pull/2125#discussion_r1117321986


##########
src/java/org/apache/cassandra/cql3/selection/Selector.java:
##########
@@ -443,14 +447,21 @@ private <V> ByteBuffer value(Cell<V> c)
         }
 
         /**
-         * Return the value of the column with the specified index.
+         * Return the value of the column with the specified index. If the 
column the value belongs to is masked with a
+         * {@link ColumnMask} and {@link #unmask} hasn't been specified, such 
mask will be applied to the value.
          *
          * @param index the column index
-         * @return the value of the column with the specified index
+         * @return the value of the column with the specified index, masked if 
its column is masked
          */
         public ByteBuffer getValue(int index)
         {
-            return values[index];
+            ByteBuffer value = values[index];

Review Comment:
   I have changed [the patch for 
CASSANDRA-18068](https://github.com/apache/cassandra/pull/2110) to store the 
masked values on `InputRow` and thus reverted the changes on `SelectStatement`. 
I'll rebase this PR if that looks good.
   
   Regarding ordering, DDM usually preserves the order of the unmasked columns, 
mostly because trying to order based on the masked value of the column would 
require a kind of replica-side ordering that we want to avoid for performance 
reasons. 
   
   However, if the user explicitly uses `ORDER BY` to order selected partitions 
in the coordinator the rows will be ordered by the masked value of the 
clustering key. That's so because when the sorting is done the columns are 
already masked. I'm not sure whether that is acceptable. If it's not, I guess 
we could try to either:
   
   - Collect both masked and unmasked values, which adds complexity and memory 
pressure.
   - Mask after ordering, although that will probably cause problems with 
chained selectors.
   - Disable `ORDER BY` if there are masked clustering columns
   
   What do you think?



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to