Skip to content

Commit

Permalink
Fix EqBehavior bug introduced by #1434 (#1614)
Browse files Browse the repository at this point in the history
### What is the issue
The `NON_DAEMON` IndexRegistry returns `true` for all calls to
`supportsExpression`. This leads to an incorrect result from the
`getEqBehavior` method where we get `MATCH` instead of `EQ` because the
index indicates that it can handle `ANALYZER_MATCHES` expressions and
`EQ` expressions.

It is an odd edge case because the javadoc for the `NON_DAEMON` object
is:

```java
    /**
     * An {@code IndexRegistry} intended for use when Cassandra is initialized in client or tool mode.
     * Contains a single stub {@code Index} which possesses no actual indexing or searching capabilities
     * but enables query validation and preparation to succeed. Useful for tools which need to prepare
     * CQL statements without instantiating the whole ColumnFamilyStore infrastructure.
     */
```

This presents a problem for the eq/match logic where we want to find a
nuanced solution to the different solutions. My proposal here is to just
make it use the EQ behavior, but that might have adverse side effects in
untested code.

### What does this PR fix and why was it fixed
The original fix used in #1434 was just to avoid the NPE we hit, but it
allowed for the wrong result in eq behavior. This fix is to say that we
should just return `EQ` in that case. It's possible this fix has
negative consequences that we haven't seen yet, but at the very least,
the CNDB tests pass with it.
  • Loading branch information
michaeljmarshall authored Feb 27, 2025
1 parent 73a5e97 commit 3f00247
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 6 deletions.
8 changes: 3 additions & 5 deletions src/java/org/apache/cassandra/cql3/SingleColumnRelation.java
Original file line number Diff line number Diff line change
Expand Up @@ -201,23 +201,21 @@ protected Restriction newEQRestriction(TableMetadata table, VariableSpecificatio
return new SingleColumnRestriction.EQRestriction(columnDef, term);

// the index is configured to transform EQ into MATCH for backwards compatibility
var matchIndexName = ebi.matchIndex.getIndexMetadata() == null ? "Unknown" : ebi.matchIndex.getIndexMetadata().name;
if (ebi.behavior == IndexRegistry.EqBehavior.MATCH)
{
ClientWarn.instance.warn(String.format(AnalyzerEqOperatorSupport.EQ_RESTRICTION_ON_ANALYZED_WARNING,
columnDef.toString(),
matchIndexName),
ebi.matchIndex.getIndexMetadata().name),
columnDef);
return new SingleColumnRestriction.AnalyzerMatchesRestriction(columnDef, term);
}

// multiple indexes support EQ, this is unsupported
assert ebi.behavior == IndexRegistry.EqBehavior.AMBIGUOUS;
var eqIndexName = ebi.eqIndex.getIndexMetadata() == null ? "Unknown" : ebi.eqIndex.getIndexMetadata().name;
throw invalidRequest(AnalyzerEqOperatorSupport.EQ_AMBIGUOUS_ERROR,
columnDef.toString(),
matchIndexName,
eqIndexName);
ebi.matchIndex.getIndexMetadata().name,
ebi.eqIndex.getIndexMetadata().name);
}
List<? extends ColumnSpecification> receivers = toReceivers(columnDef);
Term entryKey = toTerm(Collections.singletonList(receivers.get(0)), mapKey, table.keyspace, boundNames);
Expand Down
5 changes: 4 additions & 1 deletion src/java/org/apache/cassandra/index/IndexRegistry.java
Original file line number Diff line number Diff line change
Expand Up @@ -408,8 +408,11 @@ default EqBehaviorIndexes getEqBehavior(ColumnMetadata cm)
{
boolean supportsEq = index.supportsExpression(cm, Operator.EQ);
boolean supportsMatches = index.supportsExpression(cm, Operator.ANALYZER_MATCHES);
// This is an edge case due to the NON_DAEMON IndexRegistry, which doesn't have index metadata and
// which uses regular equality by convention.
boolean hasIndexMetadata = index.getIndexMetadata() != null;

if (supportsEq && supportsMatches)
if (supportsEq && supportsMatches && hasIndexMetadata)
bothIndex = index;
else if (supportsEq)
eqOnlyIndex = index;
Expand Down

0 comments on commit 3f00247

Please sign in to comment.