adelapena commented on a change in pull request #844:
URL: https://github.com/apache/cassandra/pull/844#discussion_r538556889



##########
File path: src/java/org/apache/cassandra/index/Index.java
##########
@@ -426,6 +426,16 @@ public Indexer indexerFor(DecoratedKey key,
                               WriteContext ctx,
                               IndexTransaction.Type transactionType);
 
+    /**
+     * Tells whether this index supports replica fitering protection or not.
+     *
+     * @return true if this index supports replica filtering protection, false 
otherwise
+     */
+    default boolean supportsReplicaFilteringProtection()

Review comment:
       I'd move down the new method from the [`Update 
processing`](https://github.com/apache/cassandra/blob/bc9a554c29248ed7733608b995cc26f410e2ba78/src/java/org/apache/cassandra/index/Index.java#L405)
 section to the 
[`Querying`](https://github.com/apache/cassandra/blob/bc9a554c29248ed7733608b995cc26f410e2ba78/src/java/org/apache/cassandra/index/Index.java#L537)
 section, probably right below `validate(ReadCommand)`.

##########
File path: src/java/org/apache/cassandra/service/reads/DataResolver.java
##########
@@ -121,11 +121,15 @@ private boolean needsReplicaFilteringProtection()
         if (command.rowFilter().isEmpty())
             return false;
 
-        IndexMetadata indexDef = command.indexMetadata();
-        if (indexDef != null && indexDef.isCustom())
+        ColumnFamilyStore cfs = 
ColumnFamilyStore.getIfExists(command.metadata().keyspace, 
command.metadata().name);

Review comment:
       I think that for better performance we should check whether the 
`IndexMetadata` is null or not-custom before opening `ColumnFamilyStore`. Also, 
we can open the store with the shorter 
`ColumnFamilyStore.getIfExists(command.metadata().id)`.

##########
File path: src/java/org/apache/cassandra/index/Index.java
##########
@@ -426,6 +426,16 @@ public Indexer indexerFor(DecoratedKey key,
                               WriteContext ctx,
                               IndexTransaction.Type transactionType);
 
+    /**
+     * Tells whether this index supports replica fitering protection or not.
+     *
+     * @return true if this index supports replica filtering protection, false 
otherwise
+     */
+    default boolean supportsReplicaFilteringProtection()
+    {
+        return true;

Review comment:
       Not sure what should be the default argument, I think that for 
compatibility we should return `false`.

##########
File path: src/java/org/apache/cassandra/index/Index.java
##########
@@ -426,6 +426,16 @@ public Indexer indexerFor(DecoratedKey key,
                               WriteContext ctx,
                               IndexTransaction.Type transactionType);
 
+    /**
+     * Tells whether this index supports replica fitering protection or not.
+     *
+     * @return true if this index supports replica filtering protection, false 
otherwise
+     */
+    default boolean supportsReplicaFilteringProtection()

Review comment:
       I wonder if it would make sense to use a more generic name for this 
method, something like `supportsFiltering`. @maedhroz wdyt?

##########
File path: src/java/org/apache/cassandra/index/Index.java
##########
@@ -426,6 +426,16 @@ public Indexer indexerFor(DecoratedKey key,
                               WriteContext ctx,
                               IndexTransaction.Type transactionType);
 
+    /**
+     * Tells whether this index supports replica fitering protection or not.
+     *
+     * @return true if this index supports replica filtering protection, false 
otherwise
+     */
+    default boolean supportsReplicaFilteringProtection()

Review comment:
       We could pass the `RowFilter` as an argument, so implementations can 
change their response depending on the specific query. For example, SASI could 
use RFP if no analyzed columns are involved. Not saying that we should change 
SASI behaviour in this ticket, just having the argument in the interface.

##########
File path: src/java/org/apache/cassandra/index/sasi/SASIIndex.java
##########
@@ -249,6 +249,11 @@ public long getEstimatedResultRows()
     public void validate(PartitionUpdate update) throws InvalidRequestException
     {}
 
+    public boolean supportsReplicaFilteringProtection()

Review comment:
       ```suggestion
       @Override
       public boolean supportsReplicaFilteringProtection()
   ```




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

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