dcapwell commented on code in PR #4561:
URL: https://github.com/apache/cassandra/pull/4561#discussion_r2748482665
##########
src/java/org/apache/cassandra/index/sai/utils/IndexTermType.java:
##########
@@ -453,6 +459,70 @@ public Iterator<ByteBuffer> valuesOf(Row row, long
nowInSecs)
}
}
+ public Iterator<ByteBuffer> valuesOfFrozenCollection (Row row, long
nowInSecs)
Review Comment:
The `valuesOfFrozenCollection` method only handles `REGULAR` and `STATIC`
columns (via `row.getCell()`), but doesn't check `columnMetadata.kind`. **When
this method is called for a clustering column or composite partition key column
that is a frozen collection, it returns null, causing queries to return 0
results.**
### Confirmed by Testing
I created a test that demonstrates the bug:
```java
/**
* Tests VALUES index on a frozen list used as a CLUSTERING column.
* <p>
* CASSANDRA-18492: This test demonstrates a bug where
VALUES/KEYS/ENTRIES indexes
* on frozen collections that are clustering columns return 0 results.
* </p>
* <p>
* Root cause: {@code IndexTermType.valuesOfFrozenCollection()} uses
{@code row.getCell(columnMetadata)}
* which returns null for clustering columns (their data is in {@code
row.clustering()}, not cells).
* </p>
*/
@Test
public void testFrozenListClusteringKeyWithValuesIndex()
{
createTable("CREATE TABLE %s (pk int, ck frozen<list<int>>, v int,
PRIMARY KEY (pk, ck))");
createIndex("CREATE INDEX ON %s(VALUES(ck)) USING 'sai'");
execute("INSERT INTO %s (pk, ck, v) VALUES (1, [1, 2, 3], 100)");
execute("INSERT INTO %s (pk, ck, v) VALUES (1, [4, 5, 6], 200)");
execute("INSERT INTO %s (pk, ck, v) VALUES (2, [1, 7, 8], 300)");
// Test memtable path
ResultSet rows = executeNet("SELECT pk, v FROM %s WHERE ck CONTAINS
1");
// BUG: Currently returns 0 rows instead of 2
assertEquals("Memtable query should find 2 rows containing value 1",
2, rows.all().size());
flush();
// Test SSTable path
rows = executeNet("SELECT pk, v FROM %s WHERE ck CONTAINS 1");
// BUG: Currently returns 0 rows instead of 2
assertEquals("SSTable query should find 2 rows containing value 1",
2, rows.all().size());
}
/**
* Tests VALUES index on a frozen set used as a CLUSTERING column.
* <p>
* CASSANDRA-18492: Same bug as {@link
#testFrozenListClusteringKeyWithValuesIndex()}.
* </p>
*/
@Test
public void testFrozenSetClusteringKeyWithValuesIndex()
{
createTable("CREATE TABLE %s (pk int, ck frozen<set<text>>, v int,
PRIMARY KEY (pk, ck))");
createIndex("CREATE INDEX ON %s(VALUES(ck)) USING 'sai'");
execute("INSERT INTO %s (pk, ck, v) VALUES (1, {'a', 'b'}, 100)");
execute("INSERT INTO %s (pk, ck, v) VALUES (1, {'c', 'd'}, 200)");
execute("INSERT INTO %s (pk, ck, v) VALUES (2, {'a', 'e'}, 300)");
// Test memtable path
ResultSet rows = executeNet("SELECT pk, v FROM %s WHERE ck CONTAINS
'a'");
// BUG: Currently returns 0 rows instead of 2
assertEquals("Memtable query should find 2 rows containing 'a'", 2,
rows.all().size());
flush();
// Test SSTable path
rows = executeNet("SELECT pk, v FROM %s WHERE ck CONTAINS 'a'");
// BUG: Currently returns 0 rows instead of 2
assertEquals("SSTable query should find 2 rows containing 'a'", 2,
rows.all().size());
}
/**
* Tests KEYS index on a frozen map used as a CLUSTERING column.
* <p>
* CASSANDRA-18492: Same bug as {@link
#testFrozenListClusteringKeyWithValuesIndex()}.
* </p>
*/
@Test
public void testFrozenMapClusteringKeyWithKeysIndex()
{
createTable("CREATE TABLE %s (pk int, ck frozen<map<text, int>>, v
int, PRIMARY KEY (pk, ck))");
createIndex("CREATE INDEX ON %s(KEYS(ck)) USING 'sai'");
execute("INSERT INTO %s (pk, ck, v) VALUES (1, {'x': 1, 'y': 2},
100)");
execute("INSERT INTO %s (pk, ck, v) VALUES (1, {'z': 3}, 200)");
execute("INSERT INTO %s (pk, ck, v) VALUES (2, {'x': 4}, 300)");
// Test memtable path
ResultSet rows = executeNet("SELECT pk, v FROM %s WHERE ck CONTAINS
KEY 'x'");
// BUG: Currently returns 0 rows instead of 2
assertEquals("Memtable query should find 2 rows with key 'x'", 2,
rows.all().size());
flush();
// Test SSTable path
rows = executeNet("SELECT pk, v FROM %s WHERE ck CONTAINS KEY 'x'");
// BUG: Currently returns 0 rows instead of 2
assertEquals("SSTable query should find 2 rows with key 'x'", 2,
rows.all().size());
}
/**
* Tests VALUES index on a frozen map used as a CLUSTERING column.
* <p>
* CASSANDRA-18492: Same bug as {@link
#testFrozenListClusteringKeyWithValuesIndex()}.
* </p>
*/
@Test
public void testFrozenMapClusteringKeyWithValuesIndex()
{
createTable("CREATE TABLE %s (pk int, ck frozen<map<text, int>>, v
int, PRIMARY KEY (pk, ck))");
createIndex("CREATE INDEX ON %s(VALUES(ck)) USING 'sai'");
execute("INSERT INTO %s (pk, ck, v) VALUES (1, {'x': 1, 'y': 2},
100)");
execute("INSERT INTO %s (pk, ck, v) VALUES (1, {'z': 3}, 200)");
execute("INSERT INTO %s (pk, ck, v) VALUES (2, {'x': 1}, 300)");
// Test memtable path
ResultSet rows = executeNet("SELECT pk, v FROM %s WHERE ck CONTAINS
1");
// BUG: Currently returns 0 rows instead of 2
assertEquals("Memtable query should find 2 rows with value 1", 2,
rows.all().size());
flush();
// Test SSTable path
rows = executeNet("SELECT pk, v FROM %s WHERE ck CONTAINS 1");
// BUG: Currently returns 0 rows instead of 2
assertEquals("SSTable query should find 2 rows with value 1", 2,
rows.all().size());
}
```
**Test result**: `Got less rows than expected. Expected 2 but got 0`
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
