[
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-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
