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] [Comment Edited] (CASSANDRA-10857) Allow dropping COMPACT STORAGE flag from tables in 3.X
Date Wed, 25 Oct 2017 11:30:00 GMT

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

Alex Petrov edited comment on CASSANDRA-10857 at 10/25/17 11:29 AM:
--------------------------------------------------------------------

Thank you for the review. Sorry for such long turnaround.

bq. As we discussed offline, we should test 2ndary indexes.

I've added a 2i test [here|https://github.com/apache/cassandra/compare/cassandra-3.11...ifesdjeen:10857-3.11#diff-1f6aaa9e69365f8c380e0012554dda58R161].
I've added [a dtest|https://github.com/apache/cassandra-dtest/compare/master...ifesdjeen:10857-trunk#diff-d72a3c9b64a76b1f28a592118d41240cR88]
to make sure that keys index would still work in 4.0. 

bq. The first commit has various nits, including some around making language a bit more consistent.

Great. I did have the same idea; however at time {{Schema}} sounded like a more logical place
for it (mostly because {{trunk}} has this method there). But I agree it makes the diff smaller,
so it's good.

bq. In the parser, I was a bit worried about allowing empty strings in QUOTED_NAME directly,

Completely agreed. I had the same idea, but my impression was since it's still possible to
programmatically create empty names, it's worth to have validations anyways, so I went with
the current approach. I agree this change might be safer though...

bq. So I added it in SchemaAlteringStatement directly (and I've let changes on keyspaces/non
compact tables go because no real reason to refuse them). That's the third commit.

You're right, the way it worked was any alterations that were allowed for compact tables were
still working (there are only few though). But for safety it's better to disallow them.


I've extended and moved from {{NEWS.txt}} the section describing which columns get exposed
when; now this information is in the CQL appendix as we discussed. I've changed wording a
bit in both news and CQL appendix and got rid of internal details. The doc changes are only
in 3.11 patch as there's no doc website in 3.0...

|[3.0 patch|https://github.com/apache/cassandra/compare/cassandra-3.0...ifesdjeen:10857-3.0]|[3.11
patch|https://github.com/apache/cassandra/compare/cassandra-3.11...ifesdjeen:10857-3.11]|[Python
Driver Patch|https://github.com/datastax/python-driver/compare/master...ifesdjeen:10857]|[dtests|https://github.com/apache/cassandra-dtest/compare/master...ifesdjeen:10857]|




UPDATE: I've composed a minimal patch for [trunk|https://github.com/apache/cassandra/compare/trunk...ifesdjeen:10857-trunk-minimal],
it removes compact storage grammar and logs a warning when the table has non-cql flags. The
node will start normally, but will ignore flags that make table compact.

Tests are removed or adjusted. In one instance, a schema table had compact storage without
obvious reasons. For [trunk dtest|https://github.com/apache/cassandra-dtest/compare/master...ifesdjeen:10857-trunk],
tests are just split for compact and non-compact. Compact ones are skipped for 4.0.



was (Author: ifesdjeen):
Thank you for the review. Sorry for such long turnaround.

bq. As we discussed offline, we should test 2ndary indexes.

I've added a 2i test [here|https://github.com/apache/cassandra/compare/cassandra-3.11...ifesdjeen:10857-3.11#diff-1f6aaa9e69365f8c380e0012554dda58R161].
I've added [a dtest|https://github.com/apache/cassandra-dtest/compare/master...ifesdjeen:10857-trunk#diff-d72a3c9b64a76b1f28a592118d41240cR88]
to make sure that keys index would still work in 4.0. 

bq. The first commit has various nits, including some around making language a bit more consistent.

Great. I did have the same idea; however at time {{Schema}} sounded like a more logical place
for it (mostly because {{trunk}} has this method there). But I agree it makes the diff smaller,
so it's good.

bq. In the parser, I was a bit worried about allowing empty strings in QUOTED_NAME directly,

Completely agreed. I had the same idea, but my impression was since it's still possible to
programmatically create empty names, it's worth to have validations anyways, so I went with
the current approach. I agree this change might be safer though...

bq. So I added it in SchemaAlteringStatement directly (and I've let changes on keyspaces/non
compact tables go because no real reason to refuse them). That's the third commit.

You're right, the way it worked was any alterations that were allowed for compact tables were
still working (there are only few though). But for safety it's better to disallow them.


I've extended and moved from {{NEWS.txt}} the section describing which columns get exposed
when; now this information is in the CQL appendix as we discussed. I've changed wording a
bit in both news and CQL appendix and got rid of internal details. The doc changes are only
in 3.11 patch as there's no doc website in 3.0...

|[3.0 patch|https://github.com/apache/cassandra/compare/cassandra-3.0...ifesdjeen:10857-3.0]|[3.11
patch|https://github.com/apache/cassandra/compare/cassandra-3.11...ifesdjeen:10857-3.11]|[Python
Driver Patch|https://github.com/datastax/python-driver/compare/master...ifesdjeen:10857]|[dtests|https://github.com/apache/cassandra-dtest/compare/master...ifesdjeen:10857]|




UPDATE: I've composed a patch for [trunk|https://github.com/apache/cassandra/compare/trunk...ifesdjeen:10857-trunk].
It removes {{isDense}}. {{isSuper}}, {{isCompound}} (because it's always true now), {{isCqlTable}}
(because it's always true now), {{isCompactTable}} (because it's always false now). Compact
storage is also removed from grammar copmletely.

{{Keys}} index is left intact, along with the {{DynamicCompositeType}}, since when {{COMPACT
STORAGE}} is dropped, indexes are still accessible, same with composite columns.

4.0 will not start with compact tables on disk and will demand to go back to 3.x and run {{ALTER
... DROP COMPACT STORAGE}}. 

Tests are removed or adjusted. In one instance, a schema table had compact storage without
obvious reasons. For [trunk dtest|https://github.com/apache/cassandra-dtest/compare/master...ifesdjeen:10857-trunk],
tests are just split for compact and non-compact. Compact ones are skipped for 4.0. 


> 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
>            Priority: Blocker
>              Labels: client-impacting
>             Fix For: 3.0.x, 3.11.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.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