boolean literals accepted for BooleanClause in 0.6, not in 0.7

Issue #2117 resolved
Former user created an issue

This code works on 0.6:

from sqlalchemy import *
t = Table('t', MetaData(), Column('id', Integer, primary_key=True))
and_(t.c.id == 3, False)

(although it compiles with a non-standard capitalized {{{False}}} in SQL)

It fails on 0.7 (current tip of default branch) with:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/gthb/extsvn/sqlalchemy/lib/sqlalchemy/sql/expression.py", line 409, in and_
    return BooleanClauseList(operator=operators.and_, *clauses)
  File "/Users/gthb/extsvn/sqlalchemy/lib/sqlalchemy/sql/expression.py", line 2919, in __init__
    super(BooleanClauseList, self).__init__(*clauses, **kwargs)
  File "/Users/gthb/extsvn/sqlalchemy/lib/sqlalchemy/sql/expression.py", line 2848, in __init__
    for clause in clauses if clause is not None]
  File "/Users/gthb/extsvn/sqlalchemy/lib/sqlalchemy/sql/expression.py", line 1189, in _literal_as_text
    "SQL expression object or string expected."
sqlalchemy.exc.ArgumentError: SQL expression object or string expected.

Replacing {{{False}}} with {{{'false'}}} works (and it works in {{{rel_0_6}}} as well).

If it's an intentional change, it is not documented in 07Migration.

Comments (4)

  1. Mike Bayer repo owner
    • changed milestone to 0.7.0

    So one thing to be aware of is that not every database understands the keyword "False", and the capitalized F makes it even less likely. So 0.6's behavior is not exactly a feature.

    If the False is coerced to a literal, which would be the usual routine here, a database like Oracle, and probably others, would still reject it pretty harshly, i.e. x=3 AND :some_bind. True/False are more like NULL anyway in that they are in theory constants.

    So in this case passing False would appear to be a sub-case of the "coerce text to SQL" and the patch below can address this. But its still not DB agnostic, even if we used "0" and "1" for those platforms that don't have a constant, it will probably still reject it.

    diff -r eb4a843318b4fa76d238c43a293d64af64ca1148 lib/sqlalchemy/sql/compiler.py
    --- a/lib/sqlalchemy/sql/compiler.py    Wed Mar 30 14:22:41 2011 -0400
    +++ b/lib/sqlalchemy/sql/compiler.py    Thu Mar 31 12:22:01 2011 -0400
    @@ -426,9 +426,15 @@
                  self.post_process_text(textclause.text))
             )
    
    -    def visit_null(self, null, **kwargs):
    +    def visit_null(self, expr, **kw):
             return 'NULL'
    
    +    def visit_true(self, expr, **kw):
    +        return 'true'
    +
    +    def visit_false(self, expr, **kw):
    +        return 'false'
    +
         def visit_clauselist(self, clauselist, **kwargs):
             sep = clauselist.operator
             if sep is None:
    diff -r eb4a843318b4fa76d238c43a293d64af64ca1148 lib/sqlalchemy/sql/expression.py
    --- a/lib/sqlalchemy/sql/expression.py  Wed Mar 30 14:22:41 2011 -0400
    +++ b/lib/sqlalchemy/sql/expression.py  Thu Mar 31 12:22:01 2011 -0400
    @@ -1031,12 +1031,25 @@
         return _Over(func, partition_by=partition_by, order_by=order_by)
    
     def null():
    -    """Return a :class:`_Null` object, which compiles to ``NULL`` in a sql
    -    statement.
    +    """Return a :class:`_Null` object, which compiles to ``NULL``.
    
         """
         return _Null()
    
    +def true():
    +    """Return a :class:`_True` object, which compiles to ``true``, or the 
    +    boolean equivalent for the target dialect.
    +
    +    """
    +    return _True()
    +
    +def false():
    +    """Return a :class:`_False` object, which compiles to ``false``, or the 
    +    boolean equivalent for the target dialect.
    +
    +    """
    +    return _False()
    +
     class _FunctionGenerator(object):
         """Generate :class:`.Function` objects based on getattr calls."""
    
    @@ -1184,11 +1197,25 @@
             return element.__clause_element__()
         elif isinstance(element, basestring):
             return _TextClause(unicode(element))
    +    elif isinstance(element, (util.NoneType, bool)):
    +        return _const_expr(element)
         else:
             raise exc.ArgumentError(
                 "SQL expression object or string expected."
             )
    
    +def _const_expr(element):
    +    if element is None:
    +        return null()
    +    elif element is False:
    +        return false()
    +    elif element is True:
    +        return true()
    +    else:
    +        raise exc.ArgumentError(
    +            "Expected None, False, or True"
    +        )
    +
     def _clause_element_as_expr(element):
         if hasattr(element, '__clause_element__'):
             return element.__clause_element__()
    @@ -2825,10 +2852,31 @@
         """
    
         __visit_name__ = 'null'
    -
         def __init__(self):
             self.type = sqltypes.NULLTYPE
    
    +class _False(ColumnElement):
    +    """Represent the ``false`` keyword in a SQL statement.
    +
    +    Public constructor is the :func:`false()` function.
    +
    +    """
    +
    +    __visit_name__ = 'false'
    +    def __init__(self):
    +        self.type = sqltypes.BOOLEANTYPE
    +
    +class _True(ColumnElement):
    +    """Represent the ``true`` keyword in a SQL statement.
    +
    +    Public constructor is the :func:`true()` function.
    +
    +    """
    +
    +    __visit_name__ = 'true'
    +    def __init__(self):
    +        self.type = sqltypes.BOOLEANTYPE
    +
    
     class ClauseList(ClauseElement):
         """Describe a list of clauses, separated by an operator.
    
  2. Mike Bayer repo owner

    here's test code from shazow. I want to break it into test_constants() and test_constant_coercion().

    diff -r 5eee7e2eefb754ce998554993e26f8fa3c8428ee test/sql/test_compiler.py
    --- a/test/sql/test_compiler.py Fri Apr 08 16:44:15 2011 -0400
    +++ b/test/sql/test_compiler.py Fri Apr 08 19:51:41 2011 -0700
    @@ -2797,3 +2797,27 @@
                         "INSERT INTO remote_owner.remotetable (rem_id, datatype_id, value) VALUES "
                         "(:rem_id, :datatype_id, :value)")
    
    +
    +class CoercionTest(fixtures.TestBase, AssertsCompiledSQL):
    +    __dialect__ = 'default'
    +
    +    def test_boolean(self):
    +        m = MetaData()
    +        foo =  Table('foo', m,
    +            Column('id', Integer))
    +
    +        self.assert_compile(and_(foo.c.id == 1, False),
    +                            "foo.id = :id_1 AND false")
    +
    +        self.assert_compile(and_(foo.c.id == 1, True),
    +                            "foo.id = :id_1 AND true")
    +
    +        self.assert_compile(and_(foo.c.id == None),
    +                            "foo.id IS NULL")
    +
    +        self.assert_compile(and_(foo.c.id == 1, None),
    +                            "foo.id = :id_1 AND NULL")
    +
    +        self.assert_compile(and_(foo.c.id == 1, null()),
    +                            "foo.id = :id_1 AND NULL")
    +
    
  3. Log in to comment