[
https://issues.apache.org/jira/browse/CASSANDRA-9057?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14519641#comment-14519641
]
Sam Tunnicliffe commented on CASSANDRA-9057:
--------------------------------------------
It seems that the ByteBuffer generation in {{IndexedValuesValidationTest}} is
wrong. After writing the bytes, {{remaining()}} returns {{capacity -
position}}, which == {{MAX_UNSIGNED_SHORT}} (also writing 1024 0s into a buffer
initialized with all 0s is a bit odd). This is my fault as it's been that way
since I added the test and in looking into this I found that a number of the
passing tests are not doing what quite what we (or I at least) expect.
* {{testIndexOnClusteringValueInsertPartitionKeyOver64k}} *should* reject the
partition key before it gets to index validation, but it doesn't because the
buffer is too small. Fixing the buffer generation to actually create one where
{{remaining()}} > 64k makes it do so. Because of this, I think we ought to drop
this test.
* {{testIndexOnPartitionKeyOver64k}} does actually IRE because the key exceeds
the maximum size, before hitting the index validation (but we don't check the
IRE in the test). This is due to the key here being a compound, so the entire
key comprises more than just the generated buffer. Either way, I think we
should drop this test too.
* {{testIndexOnClusteringValueOver64k}} The presence of a clustering column
over 64k should cause the primary update to be rejected before index validation
as the max total clustering length would be exceeded. What this test should be
doing is asserting the index validation kicks in when the partition key + the
remaining non-indexed clustering columns > 64k.
* A similar thing applies for
{{testIndexOnCollectionInsertCollectionKeyOver64k}}. A collection key > 64k
should be rejected regardless of indexing, so the test should use a collection
key < 64k, but ensure that partition key + collection key > 64k
* I'm afraid I was a bit off with the map/key value comment earlier. Only
non-frozen maps support indexes on their keys and they also have a size
restriction on map entries, so
{{testIndexOnCollectionKeyInsertCollectionValueOver64k}} needs to use a frozen
map and a full entry index. That also means we can't use a bind marker for the
map entry, as frozen collections must be set in toto.
I've pushed a [branch|https://github.com/beobal/cassandra/tree/9057-v3] which
(hopefully) addresses each of these points. The problem now is that the message
the IRE returns is not always super-useful as it's not always the size of the
cell value itself that is the problem - add a printStackTrace in {{failInsert}}
and you'll see what I mean.
Once we're ok with {{IVVT}}, how do you feel about removing
{{SecondaryIndexCellSizeTest}}? The only {{PCSI}} implementation in the
codebase which doesn't extend {{ASPCSI}} is the stub in {{SSICST}}. That, plus
{{PRSI#validate}} being hardcoded to return true means that that test isn't
actually giving us very much at all.
FTR, I'd rather remove the {{validate}} implementations from both {{PRSI}} and
{{PCSI}} and force subclasses to provide
their own implementations, but we can't really do that in a minor (I guess
changing {{SecondaryIndex}} is less bad because we explicitly say not to extend
it directly).
Nits:
*Unused import in {{UpdateStatement}}
*{{@Override}} annotation on {{validate}} in
{{AbstractSimplePerColumnSecondaryIndex}} should be removed
> index validation fails for non-indexed column
> ---------------------------------------------
>
> Key: CASSANDRA-9057
> URL: https://issues.apache.org/jira/browse/CASSANDRA-9057
> Project: Cassandra
> Issue Type: Bug
> Reporter: Eric Evans
> Assignee: Carl Yeksigian
> Fix For: 2.1.x
>
> Attachments: 9057-2.1-v2.txt, 9057-2.1.txt
>
>
> On 2.1.3, updates are failing with an InvalidRequestException when an
> unindexed column is greater than the maximum allowed for indexed entries.
> {noformat}
> ResponseError: Can't index column value of size 1483409 for index null on
> local_group_default_T_parsoid_html.data
> {noformat}
> In this case, the update _does_ include a 1483409 byte column value, but it
> is for a column that is not indexed, (the single indexed column is < 32
> bytes), presumably this is why
> {{cfm.getColumnDefinition(cell.name()).getIndexName()}} returns {{null}}.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
