[ https://issues.apache.org/jira/browse/CASSANDRA-6783?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13939664#comment-13939664 ]
Tyler Hobbs commented on CASSANDRA-6783: ---------------------------------------- The v2 patch still doesn't use the correct condition. Specifically, you need {{!o2.hasRemaining}}. Perhaps add a null/empty case to the tests? > Collections should have a proper compare() method for UDT > --------------------------------------------------------- > > Key: CASSANDRA-6783 > URL: https://issues.apache.org/jira/browse/CASSANDRA-6783 > Project: Cassandra > Issue Type: Bug > Reporter: Sylvain Lebresne > Assignee: Sylvain Lebresne > Fix For: 2.1 beta2 > > Attachments: 6783-2.txt, 6783.txt > > > So far, ListType, SetType and MapType don't have a proper implementation of > compare() (they throw UnsupportedOperationException) because we haven't need > one since as far as the cell comparator is concenred, only parts of a > collection ends up in the comparator and need to be compared, but the full > collection itself does not. > But with UDT can nest a collection and that sometimes require to be able to > compare them. Typically, I pushed a dtest > [here|https://github.com/riptano/cassandra-dtest/commit/290e9496d1b2c45158c7d7f5487d09ba48897a7f] > that ends up throwing: > {noformat} > java.lang.UnsupportedOperationException: CollectionType should not be use > directly as a comparator > at > org.apache.cassandra.db.marshal.CollectionType.compare(CollectionType.java:72) > ~[main/:na] > at > org.apache.cassandra.db.marshal.CollectionType.compare(CollectionType.java:37) > ~[main/:na] > at > org.apache.cassandra.db.marshal.AbstractType.compareCollectionMembers(AbstractType.java:174) > ~[main/:na] > at > org.apache.cassandra.db.marshal.AbstractCompositeType.compare(AbstractCompositeType.java:101) > ~[main/:na] > at > org.apache.cassandra.db.marshal.AbstractCompositeType.compare(AbstractCompositeType.java:35) > ~[main/:na] > at java.util.TreeMap.compare(TreeMap.java:1188) ~[na:1.7.0_45] > at java.util.TreeMap.put(TreeMap.java:531) ~[na:1.7.0_45] > at java.util.TreeSet.add(TreeSet.java:255) ~[na:1.7.0_45] > at org.apache.cassandra.cql3.Sets$DelayedValue.bind(Sets.java:205) > ~[main/:na] > at org.apache.cassandra.cql3.Sets$Literal.prepare(Sets.java:91) > ~[main/:na] > at > org.apache.cassandra.cql3.UserTypes$Literal.prepare(UserTypes.java:60) > ~[main/:na] > at > org.apache.cassandra.cql3.Operation$SetElement.prepare(Operation.java:221) > ~[main/:na] > at > org.apache.cassandra.cql3.statements.UpdateStatement$ParsedUpdate.prepareInternal(UpdateStatement.java:201) > ~[main/:na] > ... > {noformat} > Note that this stack doesn't involve cell name comparison at all, it's just > that CQL3 sometimes uses a SortedSet underneath to deal with set literals > (since internal sets are sorted by their value), and so when a set contains > UDT that has set themselves, we need the collection comparison. That being > said, for some cases like having a UDT as a map key, we do would need > collections to be comparable for the purpose of cell name comparison. > Attaching relatively simple patch. The patch is a bit bigger than it should > be because while adding the 3 simple compare() method, I realized that we had > methods to read a short length (2 unsigned short) from a ByteBuffer > duplicated all over the place and that it was time to consolidate that in > ByteBufferUtil where it should have been from day one (thus removing the > duplication). I can separate that trivial refactor in a separate patch if we > really need to, but really, the new stuff is the compare() method > implementation in ListType, SetType and MapType and the rest is a bit of > trivial cleanup. -- This message was sent by Atlassian JIRA (v6.2#6252)