cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tyler Hobbs (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-7859) Extend freezing to collections
Date Thu, 30 Oct 2014 23:09:34 GMT

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

Tyler Hobbs commented on CASSANDRA-7859:
----------------------------------------

My branch has been updated.

bq. I'm not totally convinced about adding new Frozen*Type classes. It adds a bunch of code
duplication and changes

The code duplication was only around extremely simple methods (field accessors, getInstance(),
etc), so I felt it was worth it for the clarity.  But in any case, they have been removed
as suggested, since I think that's reasonable as well.

bq. need to be a tad smarter when dealing with serializing/deserializing types

Good catch.  I've modified toString() for serialization, and added a mostly-fake FrozenType
class that's only used for type parsing to address this (it seems simpler than special-casing
TypeParser).

bq. The commit seems to include changes from CASSANDRA-6904, and some changes in StorageService/StreamCoordinator
that are unrelated to this ticket.

Hmm, not sure how that happened.  I merged in the latest 2.1, so this should be resolved.

bq. I wonder if we shouldn't allow frozen on any type syntax wise. This would simply have
no impact on simple types but that's already what happens for tuples after this patch so this
would be somewhat consistent. I suspect this might simplify validation at least.

With the recent changes, that's what's effectively done internally now.  However, I think
allowing that in the syntax would be more confusing than helpful for users.  I can imagine
a lot of questions like "what's the difference between a frozen int and a non-frozen int?".
 The fact that tuple has this behavior is just an unfortunate oddity, imo.

bq. In CQL3Type.Raw.RawCollection, in the prepare method, we could replace ...

Done

bq. I wonder if we shouldn't make it a bit more generic so that if we want to index full (frozen)
UDT in the future we don't have to introduce a fullUDT and so on. What about generalizing
to wholeValue (which is still a crappy name so better naming ideas welcome)?

We already support indexing complete (frozen) UDTs.  However, just shortening it to {{full()}}
would be relatively clear, succinct, and flexible enough to cover other types if needed.

bq. In Tuples and UserTypes bindInternal methods, not sure using isMultiCell over the existing
isCollection is a good idea.

Fixed, thanks.

bq. In SelectStatement.getRequestedColumns, the changes don't seem related to this issue,
are they?

No, they aren't.  It's a bug that I discovered while working on this, and I intended to break
that out into a separate ticket but forgot to.  I'll open a separate ticket as soon as I can
remember exactly what the bug was :)

bq. In SelectStatement.buildBound, the !r.isContains() test is a no-op since ...

Fixed as suggested.

bq. In SingleColumnRestriction.Contains.canEvaluateWithSlices, I don't understand the comment.

It was just an idea for a potential future optimization.  It wasn't really useful, though,
so it's been removed.

bq. Why do we need the newly added code in SecondaryIndexSearcher.validate?

We don't, removed.

bq. Note 100% sure about the isValueCompatible() for frozen collections...

Fair point.  It now uses {{isCompatible()}} for the name comparator.

> Extend freezing to collections
> ------------------------------
>
>                 Key: CASSANDRA-7859
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-7859
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Sylvain Lebresne
>            Assignee: Tyler Hobbs
>              Labels: cql
>             Fix For: 2.1.2
>
>         Attachments: 7859-v1.txt
>
>
> This is the follow-up to CASSANDRA-7857, to extend {{frozen}} to collections. This will
allow things like {{map<text, frozen<map<int, int>>>}} for instance, as
well as allowing {{frozen}} collections in PK columns.
> Additionally (and that's alsmot a separate ticket but I figured we can start discussing
it here), we could decide that tuple is a frozen type by default. This means that we would
allow {{tuple<int, text>}} without needing to add {{frozen}}, but we would require {{frozen}}
for complex type inside tuple, so {{tuple<int, list<text>>}} would be rejected,
but not {{tuple<int, frozen<list<text>>>}}.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message