From: Mike Bayer Date: Thu, 23 Jun 2016 23:26:28 +0000 (-0400) Subject: Make boolean processors consistent between Py/C; coerce to 1/0 X-Git-Tag: rel_1_1_0b2~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7c74d702a9632a8c7264d6972e46985de3fb2487;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 --- diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst index 44ae8c2bbf..9ae17fde09 100644 --- a/doc/build/changelog/changelog_11.rst +++ b/doc/build/changelog/changelog_11.rst @@ -21,6 +21,22 @@ .. changelog:: :version: 1.1.0b2 + .. 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 diff --git a/doc/build/changelog/migration_11.rst b/doc/build/changelog/migration_11.rst index 4e54b97e1f..1eea7eaca5 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 5357e34dc7..5b7527c202 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 b57e6740b7..98f8a27598 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 f4df7827c3..47302af979 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 e540f92464..49a1d8f15f 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):