ifesdjeen commented on code in PR #2935:
URL: https://github.com/apache/cassandra/pull/2935#discussion_r1471004446
##########
src/java/org/apache/cassandra/db/filter/RowFilter.java:
##########
@@ -68,19 +93,35 @@ public abstract class RowFilter implements
Iterable<RowFilter.Expression>
protected final List<Expression> expressions;
- protected RowFilter(List<Expression> 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;
+
+ protected RowFilter(List<Expression> expressions, boolean isStrict)
{
this.expressions = expressions;
+ this.isStrict = isStrict;
}
- public static RowFilter create()
+ public static RowFilter create(boolean isStrict)
Review Comment:
I know it might be beyond the scope of this patch, but there is only one
implementation of `RowFilter` with a removal of `ThriftFilter`. Maybe we should
bite a bullet and remove this indirection?
##########
src/java/org/apache/cassandra/index/sai/plan/FilterTree.java:
##########
@@ -43,14 +44,16 @@
*/
public class FilterTree
{
- protected final BooleanOperator op;
+ protected final BooleanOperator baseOperator;
protected final ListMultimap<ColumnMetadata, Expression> expressions;
protected final List<FilterTree> children = new ArrayList<>();
+ private final boolean strict;
Review Comment:
we call it `isStrict` throughout the patch, maybe rename it to the same for
consistency?
##########
src/java/org/apache/cassandra/db/filter/RowFilter.java:
##########
@@ -68,19 +93,35 @@ public abstract class RowFilter implements
Iterable<RowFilter.Expression>
protected final List<Expression> expressions;
- protected RowFilter(List<Expression> 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;
+
+ protected RowFilter(List<Expression> expressions, boolean isStrict)
{
this.expressions = expressions;
+ this.isStrict = isStrict;
}
- public static RowFilter create()
+ public static RowFilter create(boolean isStrict)
{
- return new CQLFilter(new ArrayList<>());
+ return new CQLFilter(new ArrayList<>(), isStrict);
}
public static RowFilter create(int capacity)
{
- return new CQLFilter(new ArrayList<>(capacity));
+ return new CQLFilter(new ArrayList<>(capacity), true);
Review Comment:
Is there any downside to _not_ use strict filtering and use union by
default, and optimise for all other cases?
--
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]
