binary 'and' and 'or' operators support

Bug #187047 reported by Thomas Herve on 2008-01-29
2
Affects Status Importance Assigned to Milestone
Storm
Undecided
Unassigned

Bug Description

I have a need for binary 'and' operator to match IP addresses stored in a database. The attached branch implements both operators, with tests. Please review.

Related branches

James Henstridge (jamesh) wrote :

Overall, the patch looks okay to me. A few comments:

 * I wonder if Comparable.__and__() and __or__() doing boolean and/or will be confusing?
 * Would BitwiseAnd and BitwiseOr be better names? That seems to be the names for the operators used here:
    http://docs.python.org/ref/bitwise.html
 * Would it be worth rounding out the remaining bitwise operators? While and/or seem fairly consistently implemented, the others don't so might require per-backend compilation (e.g. xor is "^" in mysql and "#" in postgresql).

Thomas Herve (therve) wrote :

Thanks for the comments. I renamed to BitWiseAnd and BitWiseOr, that's indeed more clear. I also added BitWiseXor and BitWiseNot, with support for the 3 databases.

James Henstridge (jamesh) wrote :

As bitwise is one word, the names should be "Bitwise" rather than "BitWise".

Also, I am not sure about the precedences you've given to the new operators. You don't seem to set precedence for all operators, and the Python precedences do not match http://docs.python.org/ref/summary.html, which needs to be fixed. I'm not sure where to find precedence information for the SQL side of things (or whether it is the same for each DB).

Thomas Herve (therve) wrote :

OK, I had to workaround a bug of bitwisenot under sqlite, but that seems ok now. Thanks again.

James Henstridge (jamesh) wrote :

Looks a lot better. My tests on hardy indicate that the "~" operator has equal or lower precedence to "+" (but higher than boolean and/or), so the precedence value you've used seems plausible. Did you check the sqlite docs/source to find what it was set to?

Once someone else checks over your branch, we can get it merged.

Thomas Herve (therve) wrote :

Looking at SQLite source code, it's hard to tell what was actually fixed. I'll have to test further.

Thomas Herve (therve) wrote :

OK I looked and my first thoughts were good. I added some more tests to check precedence between operators.

Gustavo Niemeyer (niemeyer) wrote :

Nice stuff. Three minor comments which we discussed:

[1]

We have to check the SQL precedence rules, as PostgreSQL seems
to have different rules than the one used in the branch.

[2]

    def compile_bitwise_xor_postgres(compile, expr, state):
        expr.oper = "#"

This changes the expression passed by the user, which may be unexpected.
It'd be nice to just return the compiled statement from this handler.

[3]

    BitwiseXor

BitwiseXOr is probably the most expected spelling, as we discussed live.

Thanks for the branch! +1!

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers