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

Sam Tunnicliffe edited comment on CASSANDRA-21046 at 12/4/25 1:40 PM:
----------------------------------------------------------------------

I don't agree that relocating the new fields is the right approach. 
{{KeyspaceParams/TableParams}} are the correct places for the comment and 
security label fields, alongside other "non-essential" attributes and the 
precedent is clear with the pre-existence of {{TableParams.comment}}.

Ultimately, all that's required to fix this issue is to disallow use of {{with 
security_label}} and {{with comment}} from {{CREATE/ALTER KEYSPACE...}} and 
{{with security_label}} from {{CREATE/ALTER TABLE...}} (including {{CREATE 
TABLE ... LIKE ...}}). 

This can be done by just removing them from the {{KeyspaceParams.Option}} and 
{{TableParams.Option}} enums like [~smiklosovic] suggested. The only other side 
effect would be on the respective {{toString}} methods, which could easily use 
simple strings for "SECURITY LABEL" and "COMMENT". After doing this, the new 
tests you added to {{CommentAndSecurityLabelTest}} should still pass. 

One other minor change is required to make {{CopyTableStatement::validate}} 
call {{attrs.validate()}}. Without that, {{create table ... like ... with 
security_label='xxx';}} is still allowed, but correctly does not add a security 
label. Adding a call to {{attrs.validate()}} would cause it to be rejected. I 
don't think you added a test for that scenario yet, which would be good.

One last thing, the CI summary you attached has no results for any tests aside 
from the {{cqlshlib}} suite.



was (Author: beobal):
I don't agree that relocating the new fields is the right approach. 
{{KeyspaceParams/TableParams}} are the correct places for the comment and 
security label fields, alongside other "non-essential" attributes and the 
precedent is clear with the pre-existence of {{TableParams.comment}}.

Ultimately, all that's required to fix this issue is to disallow use of {{with 
security_label}} and {{with comment}} from {{CREATE/ALTER KEYSPACE...}} and 
{{with security_label}} from {{CREATE/ALTER TABLE...}} (including {{CREATE 
TABLE ... LIKE ...}}). 

This can be done by just removing them from the {{KeyspaceParams.Option}} and 
{{TableParams.Option}} enums like [~smiklosovic] suggested. The only other side 
effect would be on the respective {{toString}} methods, which could easily use 
simple strings for "SECURITY LABEL" and "COMMENT". After doing this, the new 
tests you added to {{CommentAndSecurityLabelTest}} should still pass. 

One other minor change is required to make {{CopyTableStatement::validate}} 
call {{attrs.validate()}}. Without that, {{create table ... like ... with 
security_label='xxx';}} is still allowed, but does not add a security label. 
Adding a call to {{attrs.validate()}} would cause it to be rejected. I don't 
think you added a test for that scenario yet, which would be good.

One last thing, the CI summary you attached has no results for any tests aside 
from the {{cqlshlib}} suite.


> Schema annotations escape validation on CREATE and ALTER DDL statements
> -----------------------------------------------------------------------
>
>                 Key: CASSANDRA-21046
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-21046
>             Project: Apache Cassandra
>          Issue Type: Bug
>          Components: Cluster/Schema
>            Reporter: Sam Tunnicliffe
>            Assignee: Jyothsna Konisa
>            Priority: Normal
>             Fix For: 6.x
>
>         Attachments: ci_summary-2.html
>
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> {{CREATE KEYSPACE}} & {{CREATE TABLE}} statements silently accept 
> {{security_label}} and {{comment}} as options, but only {{CREATE TABLE ... 
> WITH comment ...}} has any effect. The same applies to {{ALTER KEYSPACE/ALTER 
> TABLE}}.
> {code}
> cqlsh> create KEYSPACE ks1 with replication = {'class': 'SimpleStrategy', 
> 'replication_factor': 1} AND security_label = 'label1' AND comment = 
> 'comment1';
> cqlsh> describe ks1;
> CREATE KEYSPACE ks1 WITH replication = {'class': 'SimpleStrategy', 
> 'replication_factor': '1'}  AND durable_writes = true  AND fast_path = 
> 'simple';
> {code}
> Supplying an unsupported/unimplemented option to either a {{CREATE}} or 
> {{ALTER}} statement should raise an error like:
> {code}
> cqlsh> alter table ks.t1 with blabla='foooo';
> SyntaxException: Unknown property 'blabla'
> {code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

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

Reply via email to