cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Petrov (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-12373) 3.0 breaks CQL compatibility with super columns families
Date Wed, 20 Sep 2017 11:05:00 GMT

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

Alex Petrov commented on CASSANDRA-12373:
-----------------------------------------

bq. Thanks for the changes and you patience on this. My main remaining remark is that I don't
think we should include the SC key and value columns to partitionColumns (in CFMetaData.rebuild).

This was surprisingly simple to do.

bq. In CFMetaData.renameColumn, in the case of updating the SC key or value column, I believe
we should be updating columnMetadata as well since those columns are listed in it, but that
doesn't seem to be the case (not sure how important it is, it might be a following call to
rebuild fixes that in practice, but since the method doesn't call rebuild itself, probably
better to make sure we handle it).

I can't see how this can be helpful because of the subsequent {{rebuild}} call, but this also
doesn't break anything, so I went ahead and changed it.

bq. In CFMetaData.makeLegacyDefaultValidator, compact tables with counter will now return
BytesType instead of CounterColumnType, which is kind of technically incorrect. To be entirely
honest, this doesn't matter currently because that method isn't ever called for non-compact
tables (and at this point, probably never will), but if we're going to rely on this, I'd rather
make it an assertion than returning something somewhat wrong. Personally, I'd just keep the
counter special case and move on, as this has nothing to do with this ticket, but if you prefer
transforming it to a assert !isCompactTable(), no complain.

I've added the {{isCounter}} back, no strong opinion here, too.

bq. Nit: in CFMetaData.renameColumn, the comment "SuperColumn tables allow renaming all columns"
doesn't match the code entirely anymore.

Yeah, I was implying dense ones, but I don't think this comment is of much use here anyways.

bq. Nit: in SuperColumnCompatibility.getSuperCfKeyColumn, I don't think the "3.x created supercolumn
family" comment is accurate anymore since in ThriftConversion you now add the 2nd clustering
column (which, in itself, lgtm).

It's still true for pre-12373 3.x thrift-created supercolumn family tables. We've discussed
this offline shortly: there was no good way to force the table update to make all the table
look completely the same, so this is the only place we still have to special-case. I've added
the {{pre 12373}} remark and hope it's clearer now.

I've committed the change only to [3.0|https://github.com/apache/cassandra/compare/cassandra-3.0...ifesdjeen:12373-3.0]
for now, will rebase and update the rest of the branches later today.

> 3.0 breaks CQL compatibility with super columns families
> --------------------------------------------------------
>
>                 Key: CASSANDRA-12373
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-12373
>             Project: Cassandra
>          Issue Type: Bug
>          Components: CQL
>            Reporter: Sylvain Lebresne
>            Assignee: Alex Petrov
>             Fix For: 3.0.x, 3.11.x
>
>
> This is a follow-up to CASSANDRA-12335 to fix the CQL side of super column compatibility.
> The details and a proposed solution can be found in the comments of CASSANDRA-12335 but
the crux of the issue is that super column famillies show up differently in CQL in 3.0.x/3.x
compared to 2.x, hence breaking backward compatibilty.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org


Mime
View raw message