Merge lp://staging/~cjwatson/storm/is-and-is-not-operators into lp://staging/storm

Proposed by Colin Watson
Status: Merged
Merged at revision: 577
Proposed branch: lp://staging/~cjwatson/storm/is-and-is-not-operators
Merge into: lp://staging/storm
Diff against target: 233 lines (+173/-0)
3 files modified
NEWS (+1/-0)
storm/expr.py (+70/-0)
storm/tests/expr.py (+102/-0)
To merge this branch: bzr merge lp://staging/~cjwatson/storm/is-and-is-not-operators
Reviewer Review Type Date Requested Status
Andrey Fedoseev (community) Approve
Review via email: mp+431754@code.staging.launchpad.net

Commit message

Add storm.expr.Is and storm.expr.IsNot operators.

Description of the change

Launchpad has had custom `IsTrue` and `IsFalse` operators for a while, because there are a few places where `expr` or `expr == True` (or the equivalents for False) don't quite do what we need: for example, if you're working with a nullable boolean column, then SQL NULL semantics mean that `expr = TRUE` or `expr = FALSE` return NULL when `expr` is NULL, which can be undesirable at times.

In addition, `==` and `!=` comparisons with True/False/None are typically picked up as errors by modern linters, since in normal Python code they should use `is` or `is not` or simple boolean tests instead. Having to override this is an annoyance.

Compare https://docs.sqlalchemy.org/en/20/core/operators.html#identity-comparisons, although SQLAlchemy is more permissive; it seems to be happy to compile anything on the right-hand-side of `IS` or `IS NOT` and leave it up to the database. Since we already have to take some care with how booleans are rendered, and there's no guarantee that the spelling of `IS TRUE` etc. is going to match the way that boolean values themselves are spelled, I thought it better to limit the set of right-hand-side expressions that we will compile.

I've checked that current versions of all the databases currently supported by Storm (PostgreSQL, MySQL, and SQLite) support all the resulting expressions, although in the case of SQLite it does need at least version 3.23.0 (released in 2018).

To post a comment you must log in.
Revision history for this message
Andrey Fedoseev (andrey-fedoseev) :
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: