adelapena commented on code in PR #2935:
URL: https://github.com/apache/cassandra/pull/2935#discussion_r1472871418
##########
src/java/org/apache/cassandra/db/filter/RowFilter.java:
##########
@@ -60,32 +83,44 @@
* be handled by a 2ndary index, and the rest is simply filtered out from the
* result set (the later can only happen if the query was using ALLOW
FILTERING).
*/
-public abstract class RowFilter implements Iterable<RowFilter.Expression>
+public class RowFilter implements Iterable<RowFilter.Expression>
{
+ private static RowFilter NONE = new RowFilter(Collections.emptyList(),
false);
private static final Logger logger =
LoggerFactory.getLogger(RowFilter.class);
public static final Serializer serializer = new Serializer();
protected final List<Expression> expressions;
- protected RowFilter(List<Expression> expressions)
- {
- this.expressions = expressions;
- }
+ /**
+ * This flag determines whether intersections (AND queries) are safe or
must be downgraded to unions (OR queries)
+ * to return enough information to the coordinator to produce a correct
query result. Strict filtering is trivially
+ * safe in all cases where there is no coordinator resolution of reads
from multiple replicas. It is also safe where
+ * intersections do not involve multiple non-key (and therefore mutable)
columns. It is also safe when the data
+ * being filtered does not contain partial updates or is fully repaired,
although determining whether or not this is
+ * the case is left to the particular filtering or indexing
implementations.
+ *
+ * @see StatementRestrictions#getRowFilter(IndexRegistry, QueryOptions)
+ *
+ * @see <a
href="https://issues.apache.org/jira/browse/CASSANDRA-19007";>CASSANDRA-19007</a>
+ * @see <a
href="https://issues.apache.org/jira/browse/CASSANDRA-19018";>CASSANDRA-19018</a>
+ */
+ private final boolean isStrict;
- public static RowFilter create()
+ protected RowFilter(List<Expression> expressions, boolean isStrict)
{
- return new CQLFilter(new ArrayList<>());
+ this.expressions = expressions;
+ this.isStrict = isStrict;
Review Comment:
Rather than sending `isStrict` serialized to the replicas, we could instead
send a more generic `needsReconciliation` attribute, the one we get from the
consistency level. Then we could keep the `RowFilter#isStrict()` method but
getting the rest from the expressions. For example:
```java
public boolean isStrict()
{
// With zero or one restricted mutable columns, strict filtering is safe
even at higher consistency levels.
return !needsReconciliation || expressions.stream().filter(e ->
!e.column.isPrimaryKeyColumn()).count() <= 1;
}
```
Alternatively, we could memoize that bit in an attribute that isn't included
in serialization:
```java
protected RowFilter(List<Expression> expressions, boolean
needsReconciliation)
{
this.expressions = expressions;
this.needsReconciliation = needsReconciliation;
this.isStrict = !needsReconciliation || expressions.stream().filter(e ->
!e.column.isPrimaryKeyColumn()).count() <= 1;
}
```
The reason to send `needsReconciliation` to the replicas instead of
`isStrict` is that it has more potential for reutilization.
Specifically, it can be used in `ORDER BY` queries, like the ones in the ANN
vector index. Those queries read the matching rows in the requested order in
the replica, then they order the rows in PK order to prevent problems with
reconciliation, and then order them again by the query order in the
coordinator. If we send `needsReconciliation` to the replicas, they can
consider the lack of reconciliation and send the rows to the coordinator in PK
order, saving us two ordering operations. I think there have been some
experiments around this and the performance improvements are huge.
--
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]
