Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cassandra.apache.org
Date: Fri, 30 Mar 2012 17:00:31 +0000 (UTC)
From: "Sylvain Lebresne (Commented) (JIRA)" <jira@apache.org>
To: commits@cassandra.apache.org
Message-ID: 
 <220483583.38671.1333126831432.JavaMail.tomcat@hel.zones.apache.org>
In-Reply-To: 
 <247935902.25680.1332882026652.JavaMail.tomcat@hel.zones.apache.org>
Subject: [jira] [Commented] (CASSANDRA-4093) schema_* CFs do not respect
 column comparator which leads to CLI commands failure.
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


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

Sylvain Lebresne commented on CASSANDRA-4093:
---------------------------------------------

bq. The thing about backwards-compatibility hacks is you tend to get stuck with them.

I suppose it depends of ones own definition of a hack, but I really don't consider this patch as a hack. As soon as we consider CT to be first class citizen, adding the ability to specify to which component a ColumnDefinition applies (which is all the patch does) feels like a rather natural extension to me. And as said, if we decide to allow secondary indexes on any component of a CT, which I though was something we wanted ultimately, we'll have to add that componentIndex information. So breaking compatibility to avoid adding a simple info to ColumnDefinition that we'll very likely end up adding anyway later feels like the wrong choice to me.

bq. We've made huge strides towards simplifying this in CQL3 and composite PKs. I don't think we can afford to dilute this with footnotes about how you can escape hatch back to the old world

I don't understand. As of the attached patch, there is 0 footnotes to add to CQL3. Currently it's pretty much *exclusively* a thrift backward-compatibility patch. If you're referring to me saying that we could later consider allowing index on a full composite in CQL3, then I'm sorry, let's forget about that for now. This patch does not force us at all to do that.

bq. dragging in all our old legacy baggage again. Put another way: it's time to bury "wtf is a supercolumn" once and for all.

I don't understand what you're referring to. Again, what this patch does is recognizing that CT are now first class citizen and thus that it makes sense that a ColumnDefinition may refer to any of the component. And while it is true that we don't *yet* use that generalization, it *trivially* allows us to support backward compatibility for thrift, hence the idea of doing that generalization now. I don't see where this patch drag *any* old legacy baggage so I'd love to get more precision on what you're referring to exactly. 

bq. Of our hundreds of deployments of 0.8 and 1.0, I can live with one or two needing to wait for 1.1.2 or whatever to upgrade.

But that is *not* what will happen. Anyone that use an index on a CT currently won't be able to upgrade *ever*. Because their index will just be not be supported anymore. If they upgrade, C* will crash. Seriously, they will be completely screwed. Their only option will be some complex manual data model migration which will be impossible to do without any downtime for the application (since no version will both support their existing index *and* the new possibly-introduced-in-1.2 indexing of the the last component of a CT).

bq. TLDR: backwards compatibility in this specific case is high cost, low benefit.

I couldn't disagree more. I still don't see what this high cost is. I wrote the patch and I just don't see at all the high cost. The patch is fairly trivial (it's not small in size but for a good part because it include thrift generated code) and has barely any user side visibility. More precisely, the only visibility it has is on the thrift side, and even then it's an optional field whose default is the right one for the thrift side. On the other side, the benefit is huge imo because it means we don't screw up the people that use C* in original ways.
                
> schema_* CFs do not respect column comparator which leads to CLI commands failure.
> ----------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-4093
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4093
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Tools
>    Affects Versions: 1.1.0
>            Reporter: Dave Brosius
>            Assignee: Sylvain Lebresne
>             Fix For: 1.1.0
>
>         Attachments: 4093.txt, CASSANDRA-4093-CD-changes.patch
>
>
> ColumnDefinition.{ascii, utf8, bool, ...} static methods used to initialize schema_* CFs column_metadata do not respect CF comparator and use ByteBufferUtil.bytes(...) for column names which creates problems in CLI and probably in other places.
> The CompositeType validator throws exception on first column
> String columnName = columnNameValidator.getString(columnDef.name);
> Because it appears the composite type length header is wrong (25455)
> AbstractCompositeType.getWithShortLength
> java.lang.IllegalArgumentException
> 	at java.nio.Buffer.limit(Buffer.java:247)
> 	at org.apache.cassandra.db.marshal.AbstractCompositeType.getBytes(AbstractCompositeType.java:50)
> 	at org.apache.cassandra.db.marshal.AbstractCompositeType.getWithShortLength(AbstractCompositeType.java:59)
> 	at org.apache.cassandra.db.marshal.AbstractCompositeType.getString(AbstractCompositeType.java:139)
> 	at org.apache.cassandra.cli.CliClient.describeColumnFamily(CliClient.java:2046)
> 	at org.apache.cassandra.cli.CliClient.describeKeySpace(CliClient.java:1969)
> 	at org.apache.cassandra.cli.CliClient.executeShowKeySpaces(CliClient.java:1574)
> (seen in trunk)

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira