[
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)
