[ 
https://issues.apache.org/jira/browse/CASSANDRA-13917?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16984366#comment-16984366
 ] 

Alex Petrov commented on CASSANDRA-13917:
-----------------------------------------

Thank you for the patch [~Gerrrr]. I think the patch is overall good, but I was 
a bit skeptical about {{hiddenColumns}} set and its creation depending on 
{{isStaticCompactTable}}, which has lead me to consider other cases, such as:

{code}
        createTable("CREATE TABLE %s (a int, b int, PRIMARY KEY(a, b)) WITH 
COMPACT STORAGE");
        execute("INSERT INTO %s (a, b, value) VALUES (?, ?, ?)", 1, 1, null); 
// this should not work
{code}

I did check some thrift-created tables, but in the most other things work the 
same way with and without compact storage. You can get some more information 
and (hopefully) an exhaustive set of compact storage table examples in 
{{DropCompactStorageThriftTest}}. 

In short, we should make sure that non-static compact tables also work as 
expected while not breaking dense tables that actually have the value column 
specified. This means that we should probably take a slightly different 
approach for validation and check for hidden columns depending on the table 
configuration.

Supercolumn families seem to work fine; but I also think we probably can skip 
adding tests for those.

> COMPACT STORAGE queries on dense static tables accept hidden column1 and 
> value columns
> --------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-13917
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-13917
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Legacy/Core
>            Reporter: Alex Petrov
>            Assignee: Aleksandr Sorokoumov
>            Priority: Low
>              Labels: lhf
>             Fix For: 3.0.x, 3.11.x
>
>         Attachments: 13917-3.0-testall-2.png, 
> 13917-3.0-testall-20.11.2019.png, 13917-3.0.png, 13917-3.11-testall-2.png, 
> 13917-3.11-testall-20.11.2019.png, 13917-3.11.png
>
>
> Test for the issue:
> {code}
>     @Test
>     public void testCompactStorage() throws Throwable
>     {
>         createTable("CREATE TABLE %s (a int PRIMARY KEY, b int, c int) WITH 
> COMPACT STORAGE");
>         assertInvalid("INSERT INTO %s (a, b, c, column1) VALUES (?, ?, ?, 
> ?)", 1, 1, 1, ByteBufferUtil.bytes('a'));
>         // This one fails with Some clustering keys are missing: column1, 
> which is still wrong
>         assertInvalid("INSERT INTO %s (a, b, c, value) VALUES (?, ?, ?, ?)", 
> 1, 1, 1, ByteBufferUtil.bytes('a'));       
>         assertInvalid("INSERT INTO %s (a, b, c, column1, value) VALUES (?, ?, 
> ?, ?, ?)", 1, 1, 1, ByteBufferUtil.bytes('a'), ByteBufferUtil.bytes('b'));
>         assertEmpty(execute("SELECT * FROM %s"));
>     }
> {code}
> Gladly, these writes are no-op, even though they succeed.
> {{value}} and {{column1}} should be completely hidden. Fixing this one should 
> be as easy as just adding validations.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to