Code review comment for lp:~adeuring/launchpad/bug-739052

Hi Robert,

thanks for the review and many goos suggestions.

On 29.07.2011 10:42, Robert Collins wrote:
> Review: Approve
> have a functional suggestion for you
[...]
>
> You may need
> 300 + if isinstance(expression, Desc):
> 301 + expression = expression.expr
> 302 + return expression < memo
> 303 + else:
> 304 + return expression > memo
>
> to have >= instead of > - I'm not sure about this, but a collection
sorted on two keys - say name, someint with rows ('foo', 1) ('foo', 2)
('foo', 3) ... would be a way to test this - make sure that pages work
properly.

Good catch!

Ordering by more that one column makes the approach indeed a bit more
complicated....

We don't want the row with the memo values itself in the result, so, for
a sort expression like "ORDER BY col1, col2, col3" we can use

  SELECT ... FROM ...
  WHERE (original_clause AND col1 >= memo1 AND col2 >= memo2
         AND col3 >= memo3) OFFSET 1

An ugly alternative:

   WHERE (original_clause) AND col1 > memo1
         OR (col1 = memo1 AND col2 > memo2)
         OR (col1 = memo1 AND col2 = memo2 AND col3 > memo3)

Or, since the three WHERE clauses do not overlap:

   (SELECT ... FROM ... WHERE original_clause
       AND col1 = memo1 AND col2 = memo2 AND col3 > memo3 ORDER BY col3)
   UNION ALL
   (SELECT ... FROM ... WHERE original_clause
       AND col1 = memo1 AND col2 > memo2 ORDER BY col2, col3)
   UNION ALL
   (SELECT ... FROM ... WHERE original_clause
   AND col1 > memo1 ORDER BY col1, col2, col3)

that's even more ugly...