maedhroz commented on code in PR #4017:
URL: https://github.com/apache/cassandra/pull/4017#discussion_r2042963421
##########
src/java/org/apache/cassandra/index/sai/plan/Operation.java:
##########
@@ -287,6 +288,9 @@ private static IndexTarget.Type
determineIndexTargetType(RowFilter.Expression ex
}
}
}
+ else if (type instanceof CompositeType)
+ indexTargetType = IndexTarget.Type.KEYS_AND_VALUES;
+
Review Comment:
I think we want to avoid assigning a target at all for composite types,
given we actually **can't** create a "keys and values" index for them. I would
take the following patch and apply it here:
```
Subject: [PATCH] Avoid creating sub-types unless we're indexing a map
---
Index: test/unit/org/apache/cassandra/index/sai/cql/ComplexQueryTest.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git
a/test/unit/org/apache/cassandra/index/sai/cql/ComplexQueryTest.java
b/test/unit/org/apache/cassandra/index/sai/cql/ComplexQueryTest.java
--- a/test/unit/org/apache/cassandra/index/sai/cql/ComplexQueryTest.java
(revision 4be2fc803102563c8d22dce9e7b0af62448a39d5)
+++ b/test/unit/org/apache/cassandra/index/sai/cql/ComplexQueryTest.java
(date 1744665356150)
@@ -67,7 +67,7 @@
}
@Test
- public void CompositeTypeWithMapInsideQuery()
+ public void compositeTypeWithMapInsideQuery()
{
createTable(KEYSPACE, "CREATE TABLE %s (" +
"pk1
frozen<map<'CompositeType(IntegerType,SimpleDateType)',
'DynamicCompositeType(Q=>LongType,I=>ByteType,6=>LexicalUUIDType)'>>," +
@@ -80,8 +80,7 @@
"r4 frozen<list<frozen<list<time>>>>," +
"r5
'CompositeType(CompositeType(ShortType,SimpleDateType,BooleanType),CompositeType(FloatType),MapType(ByteType,TimeType))',"
+
"r6 set<smallint>," +
- "PRIMARY KEY ((pk1, pk2), ck1, ck2)) " +
- "WITH CLUSTERING ORDER BY (ck1 ASC, ck2
DESC);");
+ "PRIMARY KEY ((pk1, pk2), ck1, ck2))");
Index: src/java/org/apache/cassandra/index/sai/utils/IndexTermType.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git
a/src/java/org/apache/cassandra/index/sai/utils/IndexTermType.java
b/src/java/org/apache/cassandra/index/sai/utils/IndexTermType.java
--- a/src/java/org/apache/cassandra/index/sai/utils/IndexTermType.java
(revision 4be2fc803102563c8d22dce9e7b0af62448a39d5)
+++ b/src/java/org/apache/cassandra/index/sai/utils/IndexTermType.java
(date 1744665435697)
@@ -144,7 +144,7 @@
AbstractType<?> baseType = indexType.unwrap();
- if (baseType.subTypes().isEmpty())
+ if (baseType.subTypes().isEmpty() || indexTargetType ==
IndexTarget.Type.SIMPLE || indexTargetType == IndexTarget.Type.FULL)
{
this.subTypes = Collections.emptyList();
}
Index: src/java/org/apache/cassandra/index/sai/plan/Operation.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/java/org/apache/cassandra/index/sai/plan/Operation.java
b/src/java/org/apache/cassandra/index/sai/plan/Operation.java
--- a/src/java/org/apache/cassandra/index/sai/plan/Operation.java
(revision 4be2fc803102563c8d22dce9e7b0af62448a39d5)
+++ b/src/java/org/apache/cassandra/index/sai/plan/Operation.java (date
1744663914147)
@@ -38,7 +38,6 @@
import org.apache.cassandra.db.filter.RowFilter;
import org.apache.cassandra.db.marshal.AbstractType;
import org.apache.cassandra.db.marshal.CollectionType;
-import org.apache.cassandra.db.marshal.CompositeType;
import org.apache.cassandra.exceptions.InvalidRequestException;
import org.apache.cassandra.index.sai.QueryContext;
import org.apache.cassandra.index.sai.StorageAttachedIndex;
@@ -288,8 +287,6 @@
}
}
}
- else if (type instanceof CompositeType)
- indexTargetType = IndexTarget.Type.KEYS_AND_VALUES;
return indexTargetType;
}
```
This passes our test by simply avoiding sub-type evaluation in
`IndexTermType` when we clearly don't care about them. (Again, the only reason
we ever look at them is during validation in CREATE INDEX when we've indexed a
non-frozen map.)
--
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: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
