CNDB-14210: Fix analyzed sai index on compound partition key column (#1763)

- **CNDB-14210: Fix analyzed sai index on compound partition key
column**

### What is the issue
Fixes: riptano/cndb#14210

### What does this PR fix and why was it fixed
Fixes some queries that were broken by the march and may release. In
#1434, we introduced some logic to help make the eq behavior better, and
it incorrectly handled compound partition keys. This fixes that.

The central fix is to use the `EQRestriction` any time we have a primary
key column. This is necessary to ensure we can write and read data. The
tests cover the relevant cases. I also fixed the error message returned
when attempting to use `:` on a clustering column index. Please review
the text of the error message.

Author: michaeljmarshall

src/java/org/apache/cassandra/cql3/SingleColumnRelation.java

@@ -197,7 +197,10 @@ protected Restriction newEQRestriction(TableMetadata table, VariableSpecificatio
             Term term = toTerm(toReceivers(columnDef), value, table.keyspace, boundNames);
             // Leave the restriction as EQ if no analyzed index in backwards compatibility mode is present
             var ebi = IndexRegistry.obtain(table).getEqBehavior(columnDef);
-            if (ebi.behavior == IndexRegistry.EqBehavior.EQ)
+            // The primary key always has ambiguous EQ behavior and we have to defer to later logic to decide
+            // whether the EQ is analayzed or not. This is a legacy behavior that "does the right thing" when
+            // there is a fully restricted partition key or not.
+            if (ebi.behavior == IndexRegistry.EqBehavior.EQ || columnDef.isPrimaryKeyColumn())
                 return new SingleColumnRestriction.EQRestriction(columnDef, term);
 
             // the index is configured to transform EQ into MATCH for backwards compatibility

src/java/org/apache/cassandra/cql3/restrictions/SingleColumnRestriction.java

@@ -1336,17 +1336,24 @@ public boolean isBoundedAnn()
     public static final class AnalyzerMatchesRestriction extends SingleColumnRestriction
     {
         public static final String CANNOT_BE_MERGED_ERROR = "%s cannot be restricted by other operators if it includes analyzer match (:)";
+        public static final String CANNOT_BE_RESTRICTED_BY_CLUSTERING_ERROR =
+        "Cannot restrict column '%s' by analyzer match (:) because it is a clustering column. Equals (=) can be used " +
+        "instead of match, but it will produce incomplete results due to clustering column post filtering.";
+
         private final List<Term> values;
 
         public AnalyzerMatchesRestriction(ColumnMetadata columnDef, Term value)
         {
-            super(columnDef);
-            this.values = Collections.singletonList(value);
+            this(columnDef, Collections.singletonList(value));
         }
 
         public AnalyzerMatchesRestriction(ColumnMetadata columnDef, List<Term> values)
         {
             super(columnDef);
+            // If we don't fail here, we would alternatively fail with the call to this::appendTo, which produces
+            // an unhelpful error message.
+            if (columnDef.isClusteringColumn())
+                throw invalidRequest(CANNOT_BE_RESTRICTED_BY_CLUSTERING_ERROR, columnDef.name);
             this.values = values;
         }
 

test/unit/org/apache/cassandra/index/sai/analyzer/AnalyzerEqOperatorSupportTest.java

@@ -21,7 +21,9 @@
 
 import org.apache.cassandra.cql3.CQLTester;
 import org.apache.cassandra.cql3.conditions.ColumnCondition;
+import org.apache.cassandra.cql3.restrictions.SingleColumnRestriction;
 import org.apache.cassandra.cql3.restrictions.StatementRestrictions;
+import org.apache.cassandra.exceptions.InvalidRequestException;
 import org.apache.cassandra.index.sai.SAITester;
 import org.apache.cassandra.service.ClientWarn;
 import org.assertj.core.api.Assertions;
@@ -30,6 +32,8 @@
 import static java.lang.String.format;
 import static org.apache.cassandra.index.sai.analyzer.AnalyzerEqOperatorSupport.EQ_RESTRICTION_ON_ANALYZED_WARNING;
 import static org.apache.cassandra.index.sai.analyzer.AnalyzerEqOperatorSupport.LWT_CONDITION_ON_ANALYZED_WARNING;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.Assert.assertEquals;
 
 /**
  * Tests for {@link AnalyzerEqOperatorSupport}.
@@ -359,6 +363,103 @@ private void assertTokenizedIndexSupportsMixedEqualityAndMatches()
         assertRowsWithSelectWarning("SELECT k FROM %s WHERE v = 'Quick' OR f = 'Lazy' ALLOW FILTERING", row(1));
     }
 
+    @Test
+    public void testCompoundPrimaryKeyRestrictionsWithAnalyzerEqOperatorSupportUnsupported() throws Throwable
+    {
+        // Note that we test clustering columns, but they don't technically work quite right. I include them
+        // in this test to cover their behavior and prevent unintentional changes.
+        createTable("CREATE TABLE %s (x text, y text, z text, a set<text>, PRIMARY KEY ((x, y), z))");
+
+        // Create case-insensitive index on all columns except y since it is essentially the same as x here
+        createIndex(createCaseInsensitiveIndexString("x", AnalyzerEqOperatorSupport.Value.UNSUPPORTED));
+        createIndex(createCaseInsensitiveIndexString("z", AnalyzerEqOperatorSupport.Value.UNSUPPORTED));
+        createIndex(createCaseInsensitiveIndexString("values(a)", AnalyzerEqOperatorSupport.Value.UNSUPPORTED));
+
+        // Set up two unique rows that will map to the same index terms
+        execute("INSERT INTO %s (x, y, z, a) VALUES (?, ?, ?, ?)", "a", "b", "c", set("d", "e", "f"));
+        execute("INSERT INTO %s (x, y, z, a) VALUES (?, ?, ?, ?)", "A", "B", "C", set("D", "E", "F"));
+        execute("INSERT INTO %s (x, y, z, a) VALUES (?, ?, ?, ?)", "z", "z", "z", set("z", "z", "z"));
+
+        beforeAndAfterFlush(
+        () -> {
+            // Fully qualified partition key and primary key
+            assertRows(execute("SELECT x FROM %s WHERE x = 'a' AND y = 'b'"), row("a"));
+            assertRows(execute("SELECT x FROM %s WHERE x = 'a' AND y = 'b' AND z = 'c'"), row("a"));
+            assertRows(execute("SELECT x FROM %s WHERE x = 'A' AND y = 'B'"), row("A"));
+            assertRows(execute("SELECT x FROM %s WHERE x = 'A' AND y = 'B' AND z = 'C'"), row("A"));
+            assertRows(execute("SELECT * FROM %s WHERE x = 'A' AND y = 'b'"));
+
+            // Index based query for x and a
+            assertEquals(2, execute("SELECT * FROM %s WHERE x : 'a'").size());
+            assertEquals(2, execute("SELECT * FROM %s WHERE a CONTAINS 'd'").size());
+
+            // Setting this as a proper exception so that we get a good error back to the client. It has
+            // been failing for a while, so this at least produces a nice error message.
+            assertThatThrownBy(() -> execute("SELECT * FROM %s WHERE z : 'c'"))
+            .isInstanceOf(InvalidRequestException.class)
+            .hasMessageContaining(String.format(SingleColumnRestriction.AnalyzerMatchesRestriction.CANNOT_BE_RESTRICTED_BY_CLUSTERING_ERROR, "z"));
+
+            // Expect failure for eq on partition key column since it is interpreted as eq and forms an incomplete
+            // partition key restriction.
+            assertThatThrownBy(() -> execute("SELECT * FROM %s WHERE x = 'a'"))
+            .isInstanceOf(InvalidRequestException.class)
+            .hasMessageContaining("Cannot execute this query as it might involve data filtering and thus may have unpredictable performance.");
+
+            assertThatThrownBy(() -> execute("SELECT * FROM %s WHERE z = 'c'"))
+            .isInstanceOf(InvalidRequestException.class)
+            .hasMessageContaining("Column 'z' has an index but does not support the operators specified in the query. " +
+                                  "If you want to execute this query despite the performance unpredictability, use ALLOW FILTERING");
+        });
+    }
+
+    @Test
+    public void testCompoundPrimaryKeyRestrictionsWithAnalyzerEqOperatorSupportMatch() throws Throwable
+    {
+        createTable("CREATE TABLE %s (x text, y text, z text, a set<text>, PRIMARY KEY ((x, y), z))");
+
+        // Create case-insensitive index on all columns except y since it is essentially the same as x here
+        createIndex(createCaseInsensitiveIndexString("x", AnalyzerEqOperatorSupport.Value.MATCH));
+        createIndex(createCaseInsensitiveIndexString("z", AnalyzerEqOperatorSupport.Value.MATCH));
+        createIndex(createCaseInsensitiveIndexString("values(a)", AnalyzerEqOperatorSupport.Value.MATCH));
+
+        // Set up two unique rows that will map to the same index terms
+        execute("INSERT INTO %s (x, y, z, a) VALUES (?, ?, ?, ?)", "a", "b", "c", set("d", "e", "f"));
+        execute("INSERT INTO %s (x, y, z, a) VALUES (?, ?, ?, ?)", "A", "B", "C", set("D", "E", "F"));
+
+        beforeAndAfterFlush(
+        () -> {
+            // Fully qualified partition key
+            assertRows(execute("SELECT x FROM %s WHERE x = 'a' AND y = 'b'"), row("a"));
+            assertRows(execute("SELECT x FROM %s WHERE x = 'a' AND y = 'b' AND z = 'c'"), row("a"));
+            assertRows(execute("SELECT x FROM %s WHERE x = 'A' AND y = 'B'"), row("A"));
+            assertRows(execute("SELECT x FROM %s WHERE x = 'A' AND y = 'B' AND z = 'C'"), row("A"));
+            assertRows(execute("SELECT * FROM %s WHERE x = 'A' AND y = 'b'"));
+
+            // EQ gets interpreted as a match query here
+            assertRows(execute("SELECT x FROM %s WHERE x = 'a'"), row("A"), row("a"));
+            // This only gets 1 row instead of 2 because the index is on a clustering column and that applies post
+            // filtering in surprising and problematic way.
+            assertRows(execute("SELECT x FROM %s WHERE z = 'c'"), row("a"));
+
+            // Index based query for x, z, and a
+            assertRows(execute("SELECT x FROM %s WHERE x : 'a'"), row("A"), row("a"));
+            assertRows(execute("SELECT x FROM %s WHERE a CONTAINS 'd'"), row("A"), row("a"));
+
+            // Setting this as a proper exception so that we get a good error back to the client. It has
+            // been failing for a while, so this at least produces a nice error message.
+            assertThatThrownBy(() -> execute("SELECT * FROM %s WHERE z : 'c'"))
+            .isInstanceOf(InvalidRequestException.class)
+            .hasMessageContaining(String.format(SingleColumnRestriction.AnalyzerMatchesRestriction.CANNOT_BE_RESTRICTED_BY_CLUSTERING_ERROR, "z"));
+        });
+    }
+
+    private String createCaseInsensitiveIndexString(String column, AnalyzerEqOperatorSupport.Value eqBehaviour)
+    {
+        return "CREATE CUSTOM INDEX ON %s(" + column + ") " +
+               "USING 'org.apache.cassandra.index.sai.StorageAttachedIndex' " +
+               "WITH OPTIONS = { 'case_sensitive': false, 'equals_behaviour_when_analyzed': '" + eqBehaviour + "'}";
+    }
+
     private void assertIndexDoesNotSupportEquals()
     {
         // the EQ query should not be supported by the index

test/unit/org/apache/cassandra/index/sai/cql/AllowFilteringTest.java

@@ -19,6 +19,7 @@
 
 import org.junit.Test;
 
+import org.apache.cassandra.cql3.restrictions.SingleColumnRestriction;
 import org.apache.cassandra.cql3.restrictions.StatementRestrictions;
 import org.apache.cassandra.index.IndexBuildInProgressException;
 import org.apache.cassandra.index.sai.SAITester;
@@ -435,7 +436,7 @@ public void testIndexedColumnDoesNotSupportAnalyzerRestriction()
 
         // Analyzer restriction
         assertInvalidMessage(": restriction is only supported on properly indexed columns. a : 'Test' is not valid.", "SELECT * FROM %s WHERE a : 'Test'");
-        assertInvalidMessage(String.format(StatementRestrictions.INDEX_DOES_NOT_SUPPORT_ANALYZER_MATCHES_MESSAGE, 'b'), "SELECT * FROM %s WHERE b : 'Test'");
+        assertInvalidMessage(String.format(SingleColumnRestriction.AnalyzerMatchesRestriction.CANNOT_BE_RESTRICTED_BY_CLUSTERING_ERROR, 'b'), "SELECT * FROM %s WHERE b : 'Test'");
         assertInvalidMessage(String.format(StatementRestrictions.INDEX_DOES_NOT_SUPPORT_ANALYZER_MATCHES_MESSAGE, 'c'), "SELECT * FROM %s WHERE c : 'Test'");
         assertInvalidMessage(String.format(StatementRestrictions.INDEX_DOES_NOT_SUPPORT_ANALYZER_MATCHES_MESSAGE, 'd'), "SELECT * FROM %s WHERE d : 'Test'");
     }