[
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)
