blerer commented on code in PR #2125:
URL: https://github.com/apache/cassandra/pull/2125#discussion_r1127848226
##########
src/java/org/apache/cassandra/cql3/selection/SimpleSelector.java:
##########
@@ -142,19 +142,24 @@ public void addInput(InputRow input) throws
InvalidRequestException
if (!isSet)
{
isSet = true;
- current = input.getValue(idx);
writetimes = input.getWritetimes(idx);
ttls = input.getTtls(idx);
+
+ /*
+ We apply the column mask of the column unless:
+ - The column doesn't have a mask
+ - This selector is for a query with ORDER BY post-ordering,
indicated by this.unmask
+ - The input row is for a user with UNMASK permission, indicated by
input.umask()
+ */
+ ColumnMask mask = unmask || input.unmask() ? null :
column.getMask();
Review Comment:
It is becoming hard to understand the logic. Should we rename `unmask` by
`useForPostOrdering`?
##########
src/java/org/apache/cassandra/cql3/statements/SelectStatement.java:
##########
@@ -256,6 +258,7 @@ public ResultMessage.Rows execute(QueryState state,
QueryOptions options, long q
int userLimit = getLimit(options);
int userPerPartitionLimit = getPerPartitionLimit(options);
int pageSize = options.getPageSize();
+ boolean unmask = state.getClientState().hasUnmaskPermission(table);
Review Comment:
If the query do not hit a table with masked columns (which will probably be
the greatest majority of the cases) does it make sense to waste time looking
for the permissions?
##########
test/unit/org/apache/cassandra/cql3/CQLTester.java:
##########
@@ -1721,6 +1726,24 @@ protected void assertColumnNames(UntypedResultSet
result, String... expectedColu
}
}
+ protected void assertColumnNames(ResultSet result, String...
expectedColumnNames)
Review Comment:
Same question for this one.
##########
src/java/org/apache/cassandra/cql3/statements/SelectStatement.java:
##########
@@ -1401,8 +1411,8 @@ private Comparator<List<ByteBuffer>>
getOrderingComparator(Selection selection,
if (!restrictions.keyIsInRelation())
return null;
- List<Integer> idToSort = new ArrayList<>(orderingColumns.size());
- List<Comparator<ByteBuffer>> sorters = new
ArrayList<>(orderingColumns.size());
+ List<Integer> idToSort = new
ArrayList<Integer>(orderingColumns.size());
Review Comment:
Why did we need to add the type? In this case I imagine that the JVM should
have inferred it.
##########
test/unit/org/apache/cassandra/cql3/CQLTester.java:
##########
@@ -1276,6 +1276,11 @@ protected com.datastax.driver.core.ResultSet
executeNetWithPaging(String query,
return sessionNet().execute(new
SimpleStatement(formatQuery(query)).setFetchSize(pageSize));
}
+ protected com.datastax.driver.core.ResultSet
executeNetWithoutPaging(String query)
+ {
+ return executeNetWithPaging(query, Integer.MAX_VALUE);
+ }
+
Review Comment:
Why did you introduce that method?
--
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]
