cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tyler Hobbs (JIRA)" <>
Subject [jira] [Commented] (CASSANDRA-11354) PrimaryKeyRestrictionSet should be refactored
Date Thu, 14 Apr 2016 22:51:25 GMT


Tyler Hobbs commented on CASSANDRA-11354:

The patch looks good to me overall.  The new class structure is much more understandable.
I think the only remaining confusing part is the difference between {{PartitionKeyRestrictions}}
and {{PartitionKeyRestrictionSet}}, which is not obvious at first.  I think perhaps renaming
{{PartitionKeyRestrictionSet}} to {{PartitionKeySingleRestrictionSet}} may make this clearer
(although the new name is long).  Editing the javadocs to clarify that {{PartitionKeyRestrictionSet}}
specifically doesn't include token restrictions would also help.

Other than that, I only have a couple of nitpicks:
* In {{ClusteringColumnRestrictions}}, most of the checks in the private constructor can be
moved to {{mergeWith()}} to make it more clear what is going on.
* {{Restrictions}} has two redundant methods that are also declared in {{Restriction}}: {{getColumnDefs()}}
and {{getFunctions()}}

With those fixed, +1

> PrimaryKeyRestrictionSet should be refactored
> ---------------------------------------------
>                 Key: CASSANDRA-11354
>                 URL:
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: CQL
>            Reporter: Benjamin Lerer
>            Assignee: Benjamin Lerer
> While reviewing CASSANDRA-11310 I realized that the code of {{PrimaryKeyRestrictionSet}}
was really confusing.
> The main 2 issues are:
> * the fact that it is used for partition keys and clustering columns restrictions whereas
those types of column required different processing
> * the {{isEQ}}, {{isSlice}}, {{isIN}} and {{isContains}} methods should not be there
as the set of restrictions might not match any of those categories when secondary indexes
are used.

This message was sent by Atlassian JIRA

View raw message