adelapena commented on code in PR #2110:
URL: https://github.com/apache/cassandra/pull/2110#discussion_r1100241694
##########
src/java/org/apache/cassandra/cql3/selection/Selection.java:
##########
@@ -187,7 +187,9 @@ public static Selection fromSelectors(TableMetadata table,
factories,
isJson);
- return (processesSelection(selectables) || selectables.size() !=
selectedColumns.size() || hasGroupBy)
+ boolean hasMaskedColumns =
selectedColumns.stream().anyMatch(ColumnMetadata::isMasked);
Review Comment:
I understand that the hot path where we avoid streams consists on that areas
of the code that are repeated for every row or, more probably, column. The
number of times those are hit is several orders of magnitude greater than in
things that are used only to prepare the query, thereof the preference for
performance over readability.
It seems to me that at the moment we are not avoiding streams in query
building. Just taking a quick look at the CQL package there are multiple usages
of streams, for example:
*
[o.a.c.cql3/CQL3Type.java#L840](https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/CQL3Type.java#L840)
*
[o.a.c.cql3/CQL3Type.java#L876](https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/CQL3Type.java#L876)
*
[o.a.c.cql3/Lists.java#L133](https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/Lists.java#L133)
*
[o.a.c.cql3/Sets.java#L123](https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/Sets.java#L123)
*
[o.a.c.cql3/MultiColumnRelation.java#L225](https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/MultiColumnRelation.java#L225)
*
[o.a.c.cql3/QueryProcessor.java#L481](https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/QueryProcessor.java#L481)
*
[o.a.c.cql3/TokenRelation.java#L138](https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/TokenRelation.java#L138)
*
[o.a.c.cql3/conditions/ColumnConditions.java#L75](https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/conditions/ColumnConditions.java#L75)
*
[o.a.c.cql3/functions/AbstractFunction.java#L69](https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/functions/AbstractFunction.java#L69)
*
[o.a.c.cql3/functions/FunctionResolver.java#L203](https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/functions/FunctionResolver.java#L203)
*
[o.a.c.cql3/functions/UDAggregate.java#L97](https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/functions/UDAggregate.java#L97)
*
[o.a.c.cql3/restrictions/StatementRestrictions.java#L380](https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java#L380)
*
[o.a.c.cql3/selection/AbstractFunctionSelector.java#L72](https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/selection/AbstractFunctionSelector.java#L72)
*
[o.a.c.cql3/selection/MapSelector.java#L113](https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/selection/MapSelector.java#L113)
*
[o.a.c.cql3/selection/Selectable.java#L708](https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/selection/Selectable.java#L708)
*
[o.a.c.cql3/selection/Selectable.java#L793](https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/selection/Selectable.java#L793)
*
[o.a.c.cql3/selection/Selectable.java#L886](https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/selection/Selectable.java#L886)
*
[o.a.c.cql3/selection/Selectable.java#L954](https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/selection/Selectable.java#L954)
*
[o.a.c.cql3/selection/Selectable.java#L1024](https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/selection/Selectable.java#L1024)
*
[o.a.c.cql3/statements/SelectStatement.java#L1278](https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java#L1278)
We should also check if queries are calling packages other than `o.a.c.cql3`
that also use streams.
I'd say that getting rid of those streams used on query building leans
towards premature optimization. But, if we are concerned about the performance
of streams on query building we should probably open a followup ticket to audit
where streams are used.
--
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]
