Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Fix Version/s: 0.7 beta 1
    • Component/s: None
    • Labels:
      None

      Description

      Add column type as enum (in particular in preparation for CASSANDRA-580)

      1. 1058-Add-ColumnType-as-enum.patch
        42 kB
        Sylvain Lebresne
      2. 1058-Add-the-ColumnFamilyType-enum.patch
        37 kB
        Sylvain Lebresne

        Activity

        Hide
        Sylvain Lebresne added a comment -

        Adding first part from CASSANDRA-580 wip patch rebased against trunk.

        Show
        Sylvain Lebresne added a comment - Adding first part from CASSANDRA-580 wip patch rebased against trunk.
        Hide
        Jonathan Ellis added a comment -

        wouldn't RowType be a better description of the enum's purpose?

        Show
        Jonathan Ellis added a comment - wouldn't RowType be a better description of the enum's purpose?
        Hide
        Sylvain Lebresne added a comment -

        My opinion is that RowType is not perfect either as whatever name we choose, it will
        describe the columns of a SuperColumn (damn SuperColumns ...), that are no row.
        Maybe ContainerType or the longer ColumnContainerType ?

        Show
        Sylvain Lebresne added a comment - My opinion is that RowType is not perfect either as whatever name we choose, it will describe the columns of a SuperColumn (damn SuperColumns ...), that are no row. Maybe ContainerType or the longer ColumnContainerType ?
        Hide
        Jonathan Ellis added a comment -

        it describes whether the row contains columns or supercolumns, no?

        Show
        Jonathan Ellis added a comment - it describes whether the row contains columns or supercolumns, no?
        Hide
        Sylvain Lebresne added a comment -

        Not only. That is, SuperColumn have a ColumnType too. Right now, this can only be Standard.
        But 580 will introduce other kind, like a 'Version' ColumnType, that SuperColumn can contain.
        I believe the long way to describe it is "The kind of IColumn a IColumnContainer contains".

        Show
        Sylvain Lebresne added a comment - Not only. That is, SuperColumn have a ColumnType too. Right now, this can only be Standard. But 580 will introduce other kind, like a 'Version' ColumnType, that SuperColumn can contain. I believe the long way to describe it is "The kind of IColumn a IColumnContainer contains".
        Hide
        Todd Blose added a comment -

        SnakeYAML supports java enums. If you're going to do this, you should expect a correct enum in the config instead of doing extra parsing in DatabaseDescriptor.

        http://code.google.com/p/snakeyaml/wiki/Documentation#Enum

        Show
        Todd Blose added a comment - SnakeYAML supports java enums. If you're going to do this, you should expect a correct enum in the config instead of doing extra parsing in DatabaseDescriptor. http://code.google.com/p/snakeyaml/wiki/Documentation#Enum
        Hide
        Jonathan Ellis added a comment -

        I must be missing something, because it seems to me that Version shouldn't be part of ColumnType, it should be part of the timestamp stuff.

        That is, a column and a supercolumn will both have a Version, which will either be a byte[], or a long. As with today, SC would only need that for tombstone-ing.

        Show
        Jonathan Ellis added a comment - I must be missing something, because it seems to me that Version shouldn't be part of ColumnType, it should be part of the timestamp stuff. That is, a column and a supercolumn will both have a Version, which will either be a byte[], or a long. As with today, SC would only need that for tombstone-ing.
        Hide
        Kelvin Kakugawa added a comment -

        Right now, in 580, I use the ColumnType enum to denote whether the Version (actually called Clock) is a byte[] or a long.

        For this patch, it should be sufficient to just support Standard and Super.

        Show
        Kelvin Kakugawa added a comment - Right now, in 580, I use the ColumnType enum to denote whether the Version (actually called Clock) is a byte[] or a long. For this patch, it should be sufficient to just support Standard and Super.
        Hide
        Kelvin Kakugawa added a comment -

        (as discussed on IRC)
        the plans are to name this RowType (since, CFs are restricted to one Column type)

        RowType will only encompass: Super, Standard

        Show
        Kelvin Kakugawa added a comment - (as discussed on IRC) the plans are to name this RowType (since, CFs are restricted to one Column type) RowType will only encompass: Super, Standard
        Hide
        Sylvain Lebresne added a comment -

        What about ColumnFamilyType actually for this one ?
        It seems a bit more precise to me and in the code, the function that will
        return this enum arealready called getColumnFamilyType or
        validateColumnFamily (but just proposing, RowType is shorter, I see
        the advantages in that ).

        Show
        Sylvain Lebresne added a comment - What about ColumnFamilyType actually for this one ? It seems a bit more precise to me and in the code, the function that will return this enum arealready called getColumnFamilyType or validateColumnFamily (but just proposing, RowType is shorter, I see the advantages in that ).
        Hide
        Sylvain Lebresne added a comment -

        Attaching the patch using the ColumnFamilyType name as it feel more natural to me.
        I can switch back to RowType really quickly if there is a consensus for that, so feel free
        to ask.

        Show
        Sylvain Lebresne added a comment - Attaching the patch using the ColumnFamilyType name as it feel more natural to me. I can switch back to RowType really quickly if there is a consensus for that, so feel free to ask.
        Hide
        Jonathan Ellis added a comment -

        Fine

        Show
        Jonathan Ellis added a comment - Fine
        Hide
        Jonathan Ellis added a comment -

        committed, with a couple fixes where we were comparing an enum instance with .equals("Super").

        Show
        Jonathan Ellis added a comment - committed, with a couple fixes where we were comparing an enum instance with .equals("Super").

          People

          • Assignee:
            Sylvain Lebresne
            Reporter:
            Sylvain Lebresne
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development