Make boolean processors consistent between Py/C; coerce to 1/0

The processing performed by the :class:`.Boolean` datatype for backends
that only feature integer types has been made consistent between the
pure Python and C-extension versions, in that the C-extension version
will accept any integer value from the database as a boolean, not just
zero and one; additionally, non-boolean integer values being sent to
the database are coerced to exactly zero or one, instead of being
passed as the original integer value.

Change-Id: I01e647547fd7047bd549dd70e1fa202c51e8328b
Fixes: #3730

6 files changed, 152 insertions, 14 deletions

diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst
index 44ae8c2bb..9ae17fde0 100644
--- a/doc/build/changelog/changelog_11.rst
+++ b/doc/build/changelog/changelog_11.rst
@@ -23,6 +23,22 @@
 
     .. change::
         :tags: bug, sql
+        :tickets: 3730
+
+        The processing performed by the :class:`.Boolean` datatype for backends
+        that only feature integer types has been made consistent between the
+        pure Python and C-extension versions, in that the C-extension version
+        will accept any integer value from the database as a boolean, not just
+        zero and one; additionally, non-boolean integer values being sent to
+        the database are coerced to exactly zero or one, instead of being
+        passed as the original integer value.
+
+        .. seealso::
+
+            :ref:`change_3730`
+
+    .. change::
+        :tags: bug, sql
         :tickets: 3725
 
         Rolled back the validation rules a bit in :class:`.Enum` to allow
diff --git a/doc/build/changelog/migration_11.rst b/doc/build/changelog/migration_11.rst
index 4e54b97e1..1eea7eaca 100644
--- a/doc/build/changelog/migration_11.rst
+++ b/doc/build/changelog/migration_11.rst
@@ -1507,6 +1507,28 @@ this CHECK constraint can now be disabled using the new
 
 :ticket:`3095`
 
+.. _change_3730:
+
+Non-native boolean integer values coerced to zero/one/None in all cases
+-----------------------------------------------------------------------
+
+The :class:`.Boolean` datatype coerces Python booleans to integer values
+for backends that don't have a native boolean type, such as SQLite and
+MySQL.  On these backends, a CHECK constraint is normally set up which
+ensures the values in the database are in fact one of these two values.
+However, MySQL ignores CHECK constraints, the constraint is optional, and
+an existing database might not have this constraint.  The :class:`.Boolean`
+datatype has been repaired such that an incoming Python-side value that is
+already an integer value is coerced to zero or one, not just passed as-is;
+additionally, the C-extension version of the int-to-boolean processor for
+results now uses the same Python boolean interpretation of the value,
+rather than asserting an exact one or zero value.  This is now consistent
+with the pure-Python int-to-boolean processor and is more forgiving of
+existing data already within the database.   Values of None/NULL are as before
+retained as None/NULL.
+
+:ticket:`3730`
+
 .. _change_2837:
 
 Large parameter and row values are now truncated in logging and exception displays
diff --git a/lib/sqlalchemy/cextension/processors.c b/lib/sqlalchemy/cextension/processors.c
index 5357e34dc..5b7527c20 100644
--- a/lib/sqlalchemy/cextension/processors.c
+++ b/lib/sqlalchemy/cextension/processors.c
@@ -22,28 +22,18 @@ typedef int Py_ssize_t;
 static PyObject *
 int_to_boolean(PyObject *self, PyObject *arg)
 {
-    long l = 0;
+    int l = 0;
     PyObject *res;
 
     if (arg == Py_None)
         Py_RETURN_NONE;
 
-
-#if PY_MAJOR_VERSION >= 3
-    l = PyLong_AsLong(arg);
-#else
-    l = PyInt_AsLong(arg);
-#endif
+    l = PyObject_IsTrue(arg);
     if (l == 0) {
         res = Py_False;
     } else if (l == 1) {
         res = Py_True;
-    } else if ((l == -1) && PyErr_Occurred()) {
-        /* -1 can be either the actual value, or an error flag. */
-        return NULL;
     } else {
-        PyErr_SetString(PyExc_ValueError,
-                        "int_to_boolean only accepts None, 0 or 1");
         return NULL;
     }
 
diff --git a/lib/sqlalchemy/processors.py b/lib/sqlalchemy/processors.py
index b57e6740b..98f8a2759 100644
--- a/lib/sqlalchemy/processors.py
+++ b/lib/sqlalchemy/processors.py
@@ -53,7 +53,7 @@ def boolean_to_int(value):
     if value is None:
         return None
     else:
-        return int(value)
+        return int(bool(value))
 
 
 def py_fallback():
@@ -111,7 +111,7 @@ def py_fallback():
         if value is None:
             return None
         else:
-            return value and True or False
+            return bool(value)
 
     DATETIME_RE = re.compile(
         "(\d+)-(\d+)-(\d+) (\d+):(\d+):(\d+)(?:\.(\d+))?")
diff --git a/test/engine/test_processors.py b/test/engine/test_processors.py
index f4df7827c..47302af97 100644
--- a/test/engine/test_processors.py
+++ b/test/engine/test_processors.py
@@ -2,6 +2,85 @@ from sqlalchemy.testing import fixtures
 from sqlalchemy.testing import assert_raises_message, eq_
 
 
+class _BooleanProcessorTest(fixtures.TestBase):
+    def test_int_to_bool_none(self):
+        eq_(
+            self.module.int_to_boolean(None),
+            None
+        )
+
+    def test_int_to_bool_zero(self):
+        eq_(
+            self.module.int_to_boolean(0),
+            False
+        )
+
+    def test_int_to_bool_one(self):
+        eq_(
+            self.module.int_to_boolean(1),
+            True
+        )
+
+    def test_int_to_bool_positive_int(self):
+        eq_(
+            self.module.int_to_boolean(12),
+            True
+        )
+
+    def test_int_to_bool_negative_int(self):
+        eq_(
+            self.module.int_to_boolean(-4),
+            True
+        )
+
+
+
+class PyBooleanProcessorTest(_BooleanProcessorTest):
+    @classmethod
+    def setup_class(cls):
+        from sqlalchemy import processors
+        cls.module = type(
+            "util", (object,),
+            dict(
+                (k, staticmethod(v))
+                for k, v in list(processors.py_fallback().items())
+            )
+        )
+
+    def test_bool_to_int_false(self):
+        from sqlalchemy import processors
+        eq_(processors.boolean_to_int(False), 0)
+
+    def test_bool_to_int_true(self):
+        from sqlalchemy import processors
+        eq_(processors.boolean_to_int(True), 1)
+
+    def test_bool_to_int_positive_int(self):
+        from sqlalchemy import processors
+        eq_(processors.boolean_to_int(5), 1)
+
+    def test_bool_to_int_negative_int(self):
+        from sqlalchemy import processors
+        eq_(processors.boolean_to_int(-10), 1)
+
+    def test_bool_to_int_zero(self):
+        from sqlalchemy import processors
+        eq_(processors.boolean_to_int(0), 0)
+
+    def test_bool_to_int_one(self):
+        from sqlalchemy import processors
+        eq_(processors.boolean_to_int(1), 1)
+
+
+class CBooleanProcessorTest(_BooleanProcessorTest):
+    __requires__ = ('cextensions',)
+
+    @classmethod
+    def setup_class(cls):
+        from sqlalchemy import cprocessors
+        cls.module = cprocessors
+
+
 class _DateProcessorTest(fixtures.TestBase):
     def test_date_no_string(self):
         assert_raises_message(
diff --git a/test/sql/test_types.py b/test/sql/test_types.py
index e540f9246..49a1d8f15 100644
--- a/test/sql/test_types.py
+++ b/test/sql/test_types.py
@@ -2314,6 +2314,37 @@ class BooleanTest(
             dialect="sqlite"
         )
 
+    @testing.skip_if(lambda: testing.db.dialect.supports_native_boolean)
+    def test_nonnative_processor_coerces_to_onezero(self):
+        boolean_table = self.tables.boolean_table
+        with testing.db.connect() as conn:
+            conn.execute(
+                boolean_table.insert(),
+                {"id": 1, "unconstrained_value": 5}
+            )
+
+            eq_(
+                conn.scalar("select unconstrained_value from boolean_table"),
+                1
+            )
+
+    @testing.skip_if(lambda: testing.db.dialect.supports_native_boolean)
+    def test_nonnative_processor_coerces_integer_to_boolean(self):
+        boolean_table = self.tables.boolean_table
+        with testing.db.connect() as conn:
+            conn.execute(
+                "insert into boolean_table (id, unconstrained_value) values (1, 5)"
+            )
+
+            eq_(
+                conn.scalar("select unconstrained_value from boolean_table"),
+                5
+            )
+
+            eq_(
+                conn.scalar(select([boolean_table.c.unconstrained_value])),
+                True
+            )
 
 class PickleTest(fixtures.TestBase):