Open Stack

On Fri, 2014-01-17 at 06:59 +0000, Gao, Fengqian wrote:
> Hi, Jay,
> 
> Thanks for your reply.
> According to you review comments, I rebase my code, please see my comments for your questions.
> For the issue that ensure there is at least one unique column in the sort keys if pagination is used, I have an idea.
> How about add the primary key at the end of sort keys list passed from users?

Thanks, Fengqian,

Yes, that is exactly what I was thinking... make a helper method that
wraps paginate_query() and looks at the sort_keys and, if there is no
column that is unique, it will append the primary key column(s) to the
sort keys.

I'll review your latest patches later today (currently up in Montreal at
the Neutron/Tempest QA code sprint, so a little delayed...)

Best,
-jay

> --fengqian
> 
> -----Original Message-----
> From: Jay Pipes [mailto:jaypipes at gmail.com] 
> Sent: Monday, January 13, 2014 5:10 AM
> To: Gao, Fengqian
> Cc: Julien Danjou (julien at danjou.info); Doug Hellmann (doug.hellmann at dreamhost.com); Lu, Lianhao; chungg at ca.ibm.com; dperaza at linux.vnet.ibm.com; akornienko at mirantis.com; openstack-dev at lists.openstack.org
> Subject: Re: [openstack-dev][ ceilometer] The Pagination patches
> 
> On Fri, 2014-01-10 at 03:31 +0000, Gao, Fengqian wrote:
> > Since we still have no new conclusion for pagination for now, shall we 
> > still go on with the current pagination solution?
> > 
> > Please help to review patches:
> > 
> > https://review.openstack.org/#/c/41869/
> > https://review.openstack.org/#/c/35454/
> 
> Hi Fengqian,
> 
> Please see my review on the first of the above patches. You need to ensure that there is at least one unique column in the sort keys if pagination is used.
> 
> There is a similar issue in the second patch (reviewing it now) that uses the sqlalchemy_utils (oslo.db) paginate_query() method...
> 
> The sqlalchemy_utils.paginate_query() method **requires** that sort_keys contains a unique column [1]. The docstring for that method says:
> 
> "Pagination works by requiring a unique sort_key, specified by sort_keys. (If sort_keys is not unique, then we risk looping through values.)"
> 
> So, care must be taken to pass sort_keys parameter to paginate_query() that already contains at least one unique column in the sort keys.
> 
> Best,
> -jay
> 
> [1]
> https://github.com/openstack/oslo-incubator/blob/master/openstack/common/db/sqlalchemy/utils.py#L66
> 
>