You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "sunil9977 (via GitHub)" <gi...@apache.org> on 2025/03/28 16:54:22 UTC
[PR] sai query with composite type [cassandra]
sunil9977 opened a new pull request, #4017:
URL: https://github.com/apache/cassandra/pull/4017
CASSANDRA-19891
Adding a check for SAI query with non-index column type as composite type.
patch by Sunil Ramchandra Pawar; reviewed by Caleb Rackliffe for CASSANDRA-19891
--
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-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-19891 sai query with composite type [cassandra]
Posted by "pmcfadin (via GitHub)" <gi...@apache.org>.
pmcfadin closed pull request #4017: CASSANDRA-19891 sai query with composite type
URL: https://github.com/apache/cassandra/pull/4017
--
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-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-19891 sai query with composite type [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #4017:
URL: https://github.com/apache/cassandra/pull/4017#discussion_r2044817204
##########
src/java/org/apache/cassandra/index/sai/utils/IndexTermType.java:
##########
@@ -144,7 +144,7 @@ private IndexTermType(ColumnMetadata columnMetadata, List<ColumnMetadata> partit
AbstractType<?> baseType = indexType.unwrap();
- if (baseType.subTypes().isEmpty())
+ if (baseType.subTypes().isEmpty() || indexTargetType == IndexTarget.Type.SIMPLE || indexTargetType == IndexTarget.Type.FULL)
Review Comment:
I'd throw a quick comment above this...
```
// We only need to inspect sub-types when it is possible for them to be queried individually.
```
--
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-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-19891 sai query with composite type [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
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-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-19891 sai query with composite type [cassandra]
Posted by "sunil9977 (via GitHub)" <gi...@apache.org>.
sunil9977 commented on code in PR #4017:
URL: https://github.com/apache/cassandra/pull/4017#discussion_r2044227145
##########
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:
Thank you. I've applied these changes.
--
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-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-19891 sai query with composite type [cassandra]
Posted by "pmcfadin (via GitHub)" <gi...@apache.org>.
pmcfadin commented on PR #4017:
URL: https://github.com/apache/cassandra/pull/4017#issuecomment-4227136775
Closing as [CASSANDRA-19891](https://issues.apache.org/jira/browse/CASSANDRA-19891) has been resolved as Fixed — if this PR contains changes not captured in the merged fix, please reopen.
--
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-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org