[
https://issues.apache.org/jira/browse/CASSANDRA-7017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15228067#comment-15228067
]
Sylvain Lebresne commented on CASSANDRA-7017:
---------------------------------------------
Patch generally looks good, I don't think you missed anything obvious, but a
few small remarks:
* Not sure why you made {{SelectStatement.getLimit}} {{private}} and {{static}}
but ideally we would want to keep it {{public}} and not {{static}} as it can be
overriden by custom {{QueryHandler}} as the comment mention (basically it's a
small extension point where advanced users can have some fun, not terribly
important outside of the fact that we want to keep this method and few others
public).
* I guess same question about making {{SelectStatement.prepareLimit}}
{{static}}: if it's necessary for the patch I think I'm missing why. To be
clear, it's absolutely not a big deal in that case since it's private in the
first place, but that does make things more verbose for no clear reasons so ...
* I'm nitpicking here but for the test, I'd make the
{{perPartitionLimitTest()}} just take a {{useCompactStorage}} boolean and
create the statement using that in the method. Making the create statement the
parameter make it sound like there is more flexibility than there is since
{{perPartitionLimitTest()}} strongly depends on the schema. Again, super small
nitpick on this one.
* I'd prefer re-using {{CQLTester.assertRowsIgnoringOrder()}} over re-creating
a {{groupResults}} for that test only. Granted, {{assertRowsIgnoringOrder()}}
is a little bit less precise since it completely ignore ordering, but that's
fine for this test and we can always improve it later. But recreating that kind
of generic facility in every test class that needs it will make it a
maintenance pain long term.
> allow per-partition LIMIT clause in cql
> ---------------------------------------
>
> Key: CASSANDRA-7017
> URL: https://issues.apache.org/jira/browse/CASSANDRA-7017
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Jonathan Halliday
> Assignee: Alex Petrov
> Labels: cql
> Fix For: 3.x
>
> Attachments: 0001-Allow-per-partition-limit-in-SELECT-queries.patch,
> 0001-CASSANDRA-7017.patch
>
>
> somewhat related to static columns (#6561) and slicing (#4851), it is
> desirable to apply a LIMIT on a per-partition rather than per-query basis,
> such as to retrieve the top (most recent, etc) N clustered values for each
> partition key, e.g.
> -- for each league, keep a ranked list of users
> create table scores (league text, score int, player text, primary key(league,
> score, player) );
> -- get the top 3 teams in each league:
> select * from scores staticlimit 3;
> this currently requires issuing one query per partition key, which is tedious
> if all the key partition key values are known and impossible if they aren't.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
