cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sylvain Lebresne (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-7017) allow per-partition LIMIT clause in cql
Date Wed, 06 Apr 2016 10:05:25 GMT

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

Mime
View raw message