Merge lp://staging/~jkakar/storm/any-strips-order-by into lp://staging/storm

Proposed by Jamu Kakar
Status: Merged
Approved by: Muharem Hrnjadovic
Approved revision: 365
Merged at revision: 365
Proposed branch: lp://staging/~jkakar/storm/any-strips-order-by
Merge into: lp://staging/storm
Diff against target: 319 lines (+107/-61)
4 files modified
storm/sqlobject.py (+3/-3)
storm/store.py (+39/-14)
tests/store/base.py (+64/-43)
tests/tutorial.txt (+1/-1)
To merge this branch: bzr merge lp://staging/~jkakar/storm/any-strips-order-by
Reviewer Review Type Date Requested Status
Muharem Hrnjadovic (community) Approve
Thomas Herve (community) Approve
Review via email: mp+30686@code.staging.launchpad.net

Description of the change

This branch introduces the following changes:

- An ORDER BY clause, if present, is removed when ResultSet.any is
  invoked.

- The test for ResultSet.is_empty has been updated to use the debug
  tracer, instead of monkey patching Store._connection.execute.

The other functions that could benefit from this kind of change,
ResultSet.__contains__ and Result._aggregate, don't need to be
modified because replace_columns already includes this optimization.

To post a comment you must log in.
Revision history for this message
Thomas Herve (therve) wrote :

+1!

review: Approve
Revision history for this message
Jamu Kakar (jkakar) wrote :

I had to make some changes to fix test failures, but they all pass
now.

365. By Jamu Kakar

- Fix the implementation of ResultSet.first to not use
  ResultSet.any, which doesn't work as a standin anymore because it
  strips ORDER BY clauses.
- Fixed the behaviour of ResultSet.__getitem__ to use a new custom
  version of any() that doesn't alter the ORDER BY clause. This is
  needed to maintain compatibility with prejoin-related logic.
- Improved docstrings.

Revision history for this message
Muharem Hrnjadovic (al-maisan) wrote :

Looks good, +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to status/vote changes: