[jira] [Commented] (CASSANDRA-10857) Allow dropping COMPACT STORAGE flag from tables in 3.X

["Sylvain Lebresne (JIRA)" <ji...@apache.org>]

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

Sylvain Lebresne commented on CASSANDRA-10857:
----------------------------------------------

With the caveat that I haven't read the patch at all, some remarks:

bq. just by using a property called force_non_compact that can only be set to true and on compact tables

Given that compact storage is declared using {{WITH COMPACT STORAGE}}, syntactically I'd support this removal with:
{noformat}
ALTER TABLE foo DROP COMPACT STORAGE
{noformat}
or something along that line, rather than "random" table property. It's of course mainly a minor syntactic detail.

bq. (1) When the table is created without value columns {{CREATE TABLE %s (pkey ascii, ckey ascii, PRIMARY KEY (pkey, ckey)) WITH COMPACT STORAGE}}, the {{value}} column has {{EmptyType}}, which might not be very useful.

That's an issue. And not so much because {{value}} is an {{EmptyType}} over another type (we don't currently for some reason, but could allow the conversion of {{EmptyType}} to most type, letting the user pick what he wants), but because the {{value}} column will be pretty unintuitive for the user.

Such a table (compact with only PK columns) has likely been created from CQL (I don't think many people have been using {{EmptyType}} from thrift since the type has been introduced with CQL and was never really "publicized"). And so the user clearly intended to only make use of {{pkey}} and {{ckey}}. If a user drops compactness on that table, he'll get that new {{value}} column. First, he'll be surprised as it's useless to him and he's not even upgrading from thrift. But more importantly, I fear that because it is useless to him, his first reaction will be to drop that column (which he can). Problem is, if he does that, he'll basically have remove *all* its data, essentially dropping the table.

But with that said, I don't have a great solution. I could have dealt with that better in CASSANDRA-8099 during the format migration through some special casing but I honestly didn't tough of it and that's too late. I think we'll just have to carefully document this kind of caveat.

bq. (2) When the {{comparatorType}} is set to anything but string, returned column names will be converted to their byte representation in {{AbstractType#toString}}, which, depending on the datatype is unintuitive to represent.

I don't think that's a problem as it's not new to this patch (it's annoying today if you try to access the table from CQL). And in a way, it's what user asked for. I'll note for the record that this only happen in the weird case where in thrift the comparator was not string based *and* the user had declared at least one {{column_metadata}}, which should be pretty rare.

The only think we should do here however, is probably to convert those column name to string so we don't have to rely on the special casing we currently do with {{CFMetaData.thriftColumnNameType}}.

bq. (3) Generally, any unset type would default to BytesType, whether the table was created from thrift or via CQL with {{COMPACT STORAGE}}

The defaulting to {{BytesType}} isn't really a problem imo. The problem is more than in those cases, dropping compactness will make new columns appear, columns that the user likely don't care about (and since they don't care about it, the fact it's {{BytesType}} doesn't really matter). I'm not sure there is an easy way around that though, unless we make this ticket quite a bit more complex.

bq. (4) If key was composite, it would expand to multiple clustering columns

Unless I misunderstand the case you are refering to, there will be no "expansion", (compact) table with composite comparator are already exposed with multiple clustering columns today. And that really is a feature.

bq. (5) With super column families, we'll get the name of the supercolumn as an empty string.

That's actually a genuine problem because the "supercolumn map" column used internally is a regular column, and we don't support renaming regular column (as in, we just don't have the facilities to do this even internally). We might have to accept (and document) that upgrading a super column will left you with a weird column named {{""}} (though it's worth checking that such name is actually allowed in queries). Longer term, we could possibly support renames for regular columns (it's a lot easier post-CASSANDRA-8099, though still not trivial), but I really don't think we should tackle that here.


> Allow dropping COMPACT STORAGE flag from tables in 3.X
> ------------------------------------------------------
>
>                 Key: CASSANDRA-10857
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-10857
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: CQL, Distributed Metadata
>            Reporter: Aleksey Yeschenko
>            Assignee: Alex Petrov
>             Fix For: 3.x
>
>
> Thrift allows users to define flexible mixed column families - where certain columns would have explicitly pre-defined names, potentially non-default validation types, and be indexed.
> Example:
> {code}
> create column family foo
>     and default_validation_class = UTF8Type
>     and column_metadata = [
>         {column_name: bar, validation_class: Int32Type, index_type: KEYS},
>         {column_name: baz, validation_class: UUIDType, index_type: KEYS}
>     ];
> {code}
> Columns named {{bar}} and {{baz}} will be validated as {{Int32Type}} and {{UUIDType}}, respectively, and be indexed. Columns with any other name will be validated by {{UTF8Type}} and will not be indexed.
> With CASSANDRA-8099, {{bar}} and {{baz}} would be mapped to static columns internally. However, being {{WITH COMPACT STORAGE}}, the table will only expose {{bar}} and {{baz}} columns. Accessing any dynamic columns (any column not named {{bar}} and {{baz}}) right now requires going through Thrift.
> This is blocking Thrift -> CQL migration for users who have mixed dynamic/static column families. That said, it *shouldn't* be hard to allow users to drop the {{compact}} flag to expose the table as it is internally now, and be able to access all columns.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)