Uploaded image for project: 'Cassandra'
  1. Cassandra
  2. CASSANDRA-3559

CFMetaData conversions to Thrift/Native schema should be inverse one of the other

    Details

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

      Description

      In other word, it would probably be a good idea to have:

        cfm == CFMetadata.fromThrift(cfm.toThrift())
        cfm == CFMetadata.fromSchema(cfm.toSchema())
      

      In particular, we could have unit tests to check that, which would avoid things like CASSANDRA-3558.

      It is not the case today for thrift because of the keyAlias. For some reason, if the keyAlias is not set, we return with toThrift() the default alias. I don't think this serves any purpose though.

      1. 3559-v3.patch
        13 kB
        Sylvain Lebresne
      2. CASSANDRA-3559-v2.patch
        3 kB
        Pavel Yaskevich
      3. 3559.patch
        4 kB
        Sylvain Lebresne

        Activity

        Hide
        slebresne Sylvain Lebresne added a comment -

        Note that this actually tests CASSANDRA-3558 and hence the uni test won't pass without it.

        Show
        slebresne Sylvain Lebresne added a comment - Note that this actually tests CASSANDRA-3558 and hence the uni test won't pass without it.
        Hide
        xedin Pavel Yaskevich added a comment -

        CASSANDRA-1391 is also scheduled for 1.1 and it will remove all

        {to/from}

        Avro() methods

        Show
        xedin Pavel Yaskevich added a comment - CASSANDRA-1391 is also scheduled for 1.1 and it will remove all {to/from} Avro() methods
        Hide
        slebresne Sylvain Lebresne added a comment -

        True. I'm fine waiting to see if it makes it for 1.1 and close that one if so.

        Show
        slebresne Sylvain Lebresne added a comment - True. I'm fine waiting to see if it makes it for 1.1 and close that one if so.
        Hide
        xedin Pavel Yaskevich added a comment - - edited

        Now when CASSANDRA-1391 is committed we can skip Avro validation and concentrate on thrift part because toAvro() methods were removed and fromAvro() methods were marked as @Deprecated so there is no requirement to change them ever again.

        Show
        xedin Pavel Yaskevich added a comment - - edited Now when CASSANDRA-1391 is committed we can skip Avro validation and concentrate on thrift part because toAvro() methods were removed and fromAvro() methods were marked as @Deprecated so there is no requirement to change them ever again.
        Hide
        xedin Pavel Yaskevich added a comment -

        rebased and removed avro test.

        Show
        xedin Pavel Yaskevich added a comment - rebased and removed avro test.
        Hide
        slebresne Sylvain Lebresne added a comment -

        Attaching v3 that replaces the avro tests by the test of the new native format. The latter had two problem:

        • It was throwing a NPE when deserializing a null ByteBuffer value; this is currently a blocker for CASSANDRA-3761.
        • It wasn't respecting null values for primitive types. Typically the fromSchema(toSchema()) cycle was replacing a non-set bloomFilterChance by one being set to 0.0.

        v3 fixes both problem.

        Show
        slebresne Sylvain Lebresne added a comment - Attaching v3 that replaces the avro tests by the test of the new native format. The latter had two problem: It was throwing a NPE when deserializing a null ByteBuffer value; this is currently a blocker for CASSANDRA-3761 . It wasn't respecting null values for primitive types. Typically the fromSchema(toSchema()) cycle was replacing a non-set bloomFilterChance by one being set to 0.0. v3 fixes both problem.
        Hide
        xedin Pavel Yaskevich added a comment -

        +1

        Show
        xedin Pavel Yaskevich added a comment - +1
        Hide
        slebresne Sylvain Lebresne added a comment -

        Committed, thanks

        Show
        slebresne Sylvain Lebresne added a comment - Committed, thanks

          People

          • Assignee:
            slebresne Sylvain Lebresne
            Reporter:
            slebresne Sylvain Lebresne
            Reviewer:
            Pavel Yaskevich
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development