- merge of ticket_3514 None-handling branch

- Fixes to the ORM and to the postgresql JSON type regarding the
``None`` constant in conjunction with the Postgresql :class:`.JSON` type.  When
the :paramref:`.JSON.none_as_null` flag is left at its default
value of ``False``, the ORM will now correctly insert the Json
"'null'" string into the column whenever the value on the ORM
object is set to the value ``None`` or when the value ``None``
is used with :meth:`.Session.bulk_insert_mappings`,
**including** if the column has a default or server default on it.  This
makes use of a new type-level flag "evaluates_none" which is implemented
by the JSON type based on the none_as_null flag. fixes #3514
- Added a new constant :attr:`.postgresql.JSON.NULL`, indicating
that the JSON NULL value should be used for a value
regardless of other settings. part of fixes #3514

8 files changed, 416 insertions, 3 deletions

diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst
index 207a7b5a2..0f974dc8c 100644
--- a/doc/build/changelog/changelog_11.rst
+++ b/doc/build/changelog/changelog_11.rst
@@ -98,6 +98,35 @@
 
 
     .. change::
+        :tags: bug, orm, postgresql
+        :tickets: 3514
+
+        Additional fixes have been made regarding the value of ``None``
+        in conjunction with the Postgresql :class:`.JSON` type.  When
+        the :paramref:`.JSON.none_as_null` flag is left at its default
+        value of ``False``, the ORM will now correctly insert the Json
+        "'null'" string into the column whenever the value on the ORM
+        object is set to the value ``None`` or when the value ``None``
+        is used with :meth:`.Session.bulk_insert_mappings`,
+        **including** if the column has a default or server default on it.
+
+        .. seealso::
+
+            :ref:`change_3514`
+
+    .. change::
+        :tags: feature, postgresql
+        :tickets: 3514
+
+        Added a new constant :attr:`.postgresql.JSON.NULL`, indicating
+        that the JSON NULL value should be used for a value
+        regardless of other settings.
+
+        .. seealso::
+
+            :ref:`change_3514_jsonnull`
+
+    .. change::
         :tags: bug, sql
         :tickets: 2528
 
diff --git a/doc/build/changelog/migration_11.rst b/doc/build/changelog/migration_11.rst
index fc527d8e0..c40d5a9c1 100644
--- a/doc/build/changelog/migration_11.rst
+++ b/doc/build/changelog/migration_11.rst
@@ -271,6 +271,83 @@ Will now need to change to this::
 
 
 
+.. _change_3514:
+
+Postgresql JSON "null" is inserted as expected with ORM operations, regardless of column default present
+-----------------------------------------------------------------------------------------------------------
+
+The :class:`.JSON` type has a flag :paramref:`.JSON.none_as_null` which
+when set to True indicates that the Python value ``None`` should translate
+into a SQL NULL rather than a JSON NULL value.  This flag defaults to False,
+which means that the column should *never* insert SQL NULL or fall back
+to a default unless the :func:`.null` constant were used.  However, this would
+fail in the ORM under two circumstances; one is when the column also contained
+a default or server_default value, a positive value of ``None`` on the mapped
+attribute would still result in the column-level default being triggered,
+replacing the ``None`` value::
+
+    obj = MyObject(json_value=None)
+    session.add(obj)
+    session.commit()   # would fire off default / server_default, not encode "'none'"
+
+The other is when the :meth:`.Session.bulk_insert_mappings`
+method were used, ``None`` would be ignored in all cases::
+
+    session.bulk_insert_mappings(
+        MyObject,
+        [{"json_value": None}])  # would insert SQL NULL and/or trigger defaults
+
+The :class:`.JSON` type now adds a new flag :attr:`.TypeEngine.evaluates_none`
+indicating that ``None`` should not be ignored here; it is configured
+automatically based on the value of :paramref:`.JSON.none_as_null`.
+Thanks to :ticket:`3061`, we can differentiate when the value ``None`` is actively
+set by the user versus when it was never set at all.
+
+If the attribute is not set at all, then column level defaults *will*
+fire off and/or SQL NULL will be inserted as expected, as was the behavior
+previously.  Below, the two variants are illustrated::
+
+    obj = MyObject(json_value=None)
+    session.add(obj)
+    session.commit()   # *will not* fire off column defaults, will insert JSON 'null'
+
+    obj = MyObject()
+    session.add(obj)
+    session.commit()   # *will* fire off column defaults, and/or insert SQL NULL
+
+:ticket:`3514`
+
+.. seealso::
+
+  :ref:`change_3514_jsonnull`
+
+.. _change_3514_jsonnull:
+
+New JSON.NULL Constant Added
+----------------------------
+
+To ensure that an application can always have full control at the value level
+of whether a :class:`.postgresql.JSON` or :class:`.postgresql.JSONB` column
+should receive a SQL NULL or JSON ``"null"`` value, the constant
+:attr:`.postgresql.JSON.NULL` has been added, which in conjunction with
+:func:`.null` can be used to determine fully between SQL NULL and
+JSON ``"null"``, regardless of what :paramref:`.JSON.none_as_null` is set
+to::
+
+    from sqlalchemy import null
+    from sqlalchemy.dialects.postgresql import JSON
+
+    obj1 = MyObject(json_value=null())  # will *always* insert SQL NULL
+    obj2 = MyObject(json_value=JSON.NULL)  # will *always* insert JSON string "null"
+
+    session.add_all([obj1, obj2])
+    session.commit()
+
+.. seealso::
+
+    :ref:`change_3514`
+
+:ticket:`3514`
 
 Dialect Improvements and Changes - MySQL
 =============================================
diff --git a/lib/sqlalchemy/dialects/postgresql/json.py b/lib/sqlalchemy/dialects/postgresql/json.py
index 4716ca970..2e2e71d0c 100644
--- a/lib/sqlalchemy/dialects/postgresql/json.py
+++ b/lib/sqlalchemy/dialects/postgresql/json.py
@@ -115,6 +115,29 @@ class JSON(sqltypes.Indexable, sqltypes.TypeEngine):
     will be detected by the unit of work.  See the example at :class:`.HSTORE`
     for a simple example involving a dictionary.
 
+    When working with NULL values, the :class:`.JSON` type recommends the
+    use of two specific constants in order to differentiate between a column
+    that evaluates to SQL NULL, e.g. no value, vs. the JSON-encoded string
+    of ``"null"``.   To insert or select against a value that is SQL NULL,
+    use the constant :func:`.null`::
+
+        conn.execute(table.insert(), json_value=null())
+
+    To insert or select against a value that is JSON ``"null"``, use the
+    constant :attr:`.JSON.NULL`::
+
+        conn.execute(table.insert(), json_value=JSON.NULL)
+
+    The :class:`.JSON` type supports a flag
+    :paramref:`.JSON.none_as_null` which when set to True will result
+    in the Python constant ``None`` evaluating to the value of SQL
+    NULL, and when set to False results in the Python constant
+    ``None`` evaluating to the value of JSON ``"null"``.    The Python
+    value ``None`` may be used in conjunction with either
+    :attr:`.JSON.NULL` and :func:`.null` in order to indicate NULL
+    values, but care must be taken as to the value of the
+    :paramref:`.JSON.none_as_null` in these cases.
+
     Custom serializers and deserializers are specified at the dialect level,
     that is using :func:`.create_engine`.  The reason for this is that when
     using psycopg2, the DBAPI only allows serializers at the per-cursor
@@ -141,6 +164,30 @@ class JSON(sqltypes.Indexable, sqltypes.TypeEngine):
     hashable = False
     astext_type = sqltypes.Text()
 
+    NULL = util.symbol('JSON_NULL')
+    """Describe the json value of NULL.
+
+    This value is used to force the JSON value of ``"null"`` to be
+    used as the value.   A value of Python ``None`` will be recognized
+    either as SQL NULL or JSON ``"null"``, based on the setting
+    of the :paramref:`.JSON.none_as_null` flag; the :attr:`.JSON.NULL`
+    constant can be used to always resolve to JSON ``"null"`` regardless
+    of this setting.  This is in contrast to the :func:`.sql.null` construct,
+    which always resolves to SQL NULL.  E.g.::
+
+        from sqlalchemy import null
+        from sqlalchemy.dialects.postgresql import JSON
+
+        obj1 = MyObject(json_value=null())  # will *always* insert SQL NULL
+        obj2 = MyObject(json_value=JSON.NULL)  # will *always* insert JSON string "null"
+
+        session.add_all([obj1, obj2])
+        session.commit()
+
+    .. versionadded:: 1.1
+
+    """
+
     def __init__(self, none_as_null=False, astext_type=None):
         """Construct a :class:`.JSON` type.
 
@@ -155,6 +202,10 @@ class JSON(sqltypes.Indexable, sqltypes.TypeEngine):
          .. versionchanged:: 0.9.8 - Added ``none_as_null``, and :func:`.null`
             is now supported in order to persist a NULL value.
 
+         .. seealso::
+
+              :attr:`.JSON.NULL`
+
         :param astext_type: the type to use for the
          :attr:`.JSON.Comparator.astext`
          accessor on indexed attributes.  Defaults to :class:`.types.Text`.
@@ -206,13 +257,19 @@ class JSON(sqltypes.Indexable, sqltypes.TypeEngine):
 
     comparator_factory = Comparator
 
+    @property
+    def evaluates_none(self):
+        return not self.none_as_null
+
     def bind_processor(self, dialect):
         json_serializer = dialect._json_serializer or json.dumps
         if util.py2k:
             encoding = dialect.encoding
 
             def process(value):
-                if isinstance(value, elements.Null) or (
+                if value is self.NULL:
+                    value = None
+                elif isinstance(value, elements.Null) or (
                     value is None and self.none_as_null
                 ):
                     return None
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 48fbaae32..3efaa45ac 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -1915,6 +1915,19 @@ class Mapper(InspectionAttr):
     """
 
     @_memoized_configured_property
+    def _insert_cols_evaluating_none(self):
+        return dict(
+            (
+                table,
+                frozenset(
+                    col.key for col in columns
+                    if col.type.evaluates_none
+                )
+            )
+            for table, columns in self._cols_by_table.items()
+        )
+
+    @_memoized_configured_property
     def _insert_cols_as_none(self):
         return dict(
             (
@@ -1922,7 +1935,8 @@ class Mapper(InspectionAttr):
                 frozenset(
                     col.key for col in columns
                     if not col.primary_key and
-                    not col.server_default and not col.default)
+                    not col.server_default and not col.default
+                    and not col.type.evaluates_none)
             )
             for table, columns in self._cols_by_table.items()
         )
diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index 0bfee2ece..c785a4dee 100644
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -375,10 +375,12 @@ def _collect_insert_commands(
 
         propkey_to_col = mapper._propkey_to_col[table]
 
+        eval_none = mapper._insert_cols_evaluating_none[table]
+
         for propkey in set(propkey_to_col).intersection(state_dict):
             value = state_dict[propkey]
             col = propkey_to_col[propkey]
-            if value is None:
+            if value is None and propkey not in eval_none:
                 continue
             elif not bulk and isinstance(value, sql.ClauseElement):
                 value_params[col.key] = value
diff --git a/lib/sqlalchemy/sql/type_api.py b/lib/sqlalchemy/sql/type_api.py
index 8f502ac02..701e2a44a 100644
--- a/lib/sqlalchemy/sql/type_api.py
+++ b/lib/sqlalchemy/sql/type_api.py
@@ -129,6 +129,19 @@ class TypeEngine(Visitable):
 
     """
 
+    evaluates_none = False
+    """If True, the Python constant ``None`` is considered to be handled
+    explicitly by this type.
+
+    The ORM will use this flag to ensure that a positive value of ``None``
+    is definitely passed to the backend, ignoring whether or not there
+    are Python or server side defaults on this column.
+
+    .. versionadded:: 1.1
+
+
+    """
+
     def compare_against_backend(self, dialect, conn_type):
         """Compare this type against the given backend type.
 
diff --git a/test/dialect/postgresql/test_types.py b/test/dialect/postgresql/test_types.py
index 9e0e5bcc6..00a2de2db 100644
--- a/test/dialect/postgresql/test_types.py
+++ b/test/dialect/postgresql/test_types.py
@@ -2398,6 +2398,15 @@ class JSONRoundTripTest(fixtures.TablesTest):
         ).fetchall()
         eq_([d for d, in data], [None])
 
+    def _assert_column_is_JSON_NULL(self, column='data'):
+        col = self.tables.data_table.c[column]
+
+        data = testing.db.execute(
+            select([col]).
+            where(cast(col, String) == "null")
+        ).fetchall()
+        eq_([d for d, in data], [None])
+
     def _test_insert(self, engine):
         engine.execute(
             self.tables.data_table.insert(),
@@ -2419,6 +2428,13 @@ class JSONRoundTripTest(fixtures.TablesTest):
         )
         self._assert_column_is_NULL(column='nulldata')
 
+    def _test_insert_nulljson_into_none_as_null(self, engine):
+        engine.execute(
+            self.tables.data_table.insert(),
+            {'name': 'r1', 'nulldata': JSON.NULL}
+        )
+        self._assert_column_is_JSON_NULL(column='nulldata')
+
     def _non_native_engine(self, json_serializer=None, json_deserializer=None):
         if json_serializer is not None or json_deserializer is not None:
             options = {
@@ -2467,6 +2483,11 @@ class JSONRoundTripTest(fixtures.TablesTest):
         engine = testing.db
         self._test_insert_none_as_null(engine)
 
+    @testing.requires.psycopg2_native_json
+    def test_insert_native_nulljson_into_none_as_null(self):
+        engine = testing.db
+        self._test_insert_nulljson_into_none_as_null(engine)
+
     def test_insert_python(self):
         engine = self._non_native_engine()
         self._test_insert(engine)
@@ -2479,6 +2500,10 @@ class JSONRoundTripTest(fixtures.TablesTest):
         engine = self._non_native_engine()
         self._test_insert_none_as_null(engine)
 
+    def test_insert_python_nulljson_into_none_as_null(self):
+        engine = self._non_native_engine()
+        self._test_insert_nulljson_into_none_as_null(engine)
+
     def _test_custom_serialize_deserialize(self, native):
         import json
 
@@ -2645,6 +2670,36 @@ class JSONRoundTripTest(fixtures.TablesTest):
         engine = testing.db
         self._test_unicode_round_trip(engine)
 
+    def test_eval_none_flag_orm(self):
+        Base = declarative_base()
+
+        class Data(Base):
+            __table__ = self.tables.data_table
+
+        s = Session(testing.db)
+
+        d1 = Data(name='d1', data=None, nulldata=None)
+        s.add(d1)
+        s.commit()
+
+        s.bulk_insert_mappings(
+            Data, [{"name": "d2", "data": None, "nulldata": None}]
+        )
+        eq_(
+            s.query(
+                cast(self.tables.data_table.c.data, String),
+                cast(self.tables.data_table.c.nulldata, String)
+            ).filter(self.tables.data_table.c.name == 'd1').first(),
+            ("null", None)
+        )
+        eq_(
+            s.query(
+                cast(self.tables.data_table.c.data, String),
+                cast(self.tables.data_table.c.nulldata, String)
+            ).filter(self.tables.data_table.c.name == 'd2').first(),
+            ("null", None)
+        )
+
 
 class JSONBTest(JSONTest):
 
diff --git a/test/orm/test_unitofworkv2.py b/test/orm/test_unitofworkv2.py
index 4ba5d6fbf..d4870adc6 100644
--- a/test/orm/test_unitofworkv2.py
+++ b/test/orm/test_unitofworkv2.py
@@ -1955,3 +1955,169 @@ class TypeWoBoolTest(fixtures.MappedTest, testing.AssertsExecutionResults):
             s.query(Thing.value).scalar().text, "foo"
         )
 
+
+class NullEvaluatingTest(fixtures.MappedTest, testing.AssertsExecutionResults):
+    @classmethod
+    def define_tables(cls, metadata):
+        from sqlalchemy import TypeDecorator
+
+        class EvalsNull(TypeDecorator):
+            impl = String(50)
+
+            evaluates_none = True
+
+            def process_bind_param(self, value, dialect):
+                if value is None:
+                    value = 'nothing'
+                return value
+
+        Table(
+            'test', metadata,
+            Column('id', Integer, primary_key=True,
+                   test_needs_autoincrement=True),
+            Column('evals_null_no_default', EvalsNull()),
+            Column('evals_null_default', EvalsNull(), default='default_val'),
+            Column('no_eval_null_no_default', String(50)),
+            Column('no_eval_null_default', String(50), default='default_val'),
+        )
+
+    @classmethod
+    def setup_classes(cls):
+        class Thing(cls.Basic):
+            pass
+
+    @classmethod
+    def setup_mappers(cls):
+        Thing = cls.classes.Thing
+
+        mapper(Thing, cls.tables.test)
+
+    def _assert_col(self, name, value):
+        Thing = self.classes.Thing
+        s = Session()
+
+        col = getattr(Thing, name)
+        obj = s.query(col).filter(col == value).one()
+        eq_(obj[0], value)
+
+    def _test_insert(self, attr, expected):
+        Thing = self.classes.Thing
+
+        s = Session()
+        t1 = Thing(**{attr: None})
+        s.add(t1)
+        s.commit()
+
+        self._assert_col(attr, expected)
+
+    def _test_bulk_insert(self, attr, expected):
+        Thing = self.classes.Thing
+
+        s = Session()
+        s.bulk_insert_mappings(
+            Thing, [{attr: None}]
+        )
+        s.commit()
+
+        self._assert_col(attr, expected)
+
+    def _test_insert_novalue(self, attr, expected):
+        Thing = self.classes.Thing
+
+        s = Session()
+        t1 = Thing()
+        s.add(t1)
+        s.commit()
+
+        self._assert_col(attr, expected)
+
+    def _test_bulk_insert_novalue(self, attr, expected):
+        Thing = self.classes.Thing
+
+        s = Session()
+        s.bulk_insert_mappings(
+            Thing, [{}]
+        )
+        s.commit()
+
+        self._assert_col(attr, expected)
+
+    def test_evalnull_nodefault_insert(self):
+        self._test_insert(
+            "evals_null_no_default", 'nothing'
+        )
+
+    def test_evalnull_nodefault_bulk_insert(self):
+        self._test_bulk_insert(
+            "evals_null_no_default", 'nothing'
+        )
+
+    def test_evalnull_nodefault_insert_novalue(self):
+        self._test_insert_novalue(
+            "evals_null_no_default", None
+        )
+
+    def test_evalnull_nodefault_bulk_insert_novalue(self):
+        self._test_bulk_insert_novalue(
+            "evals_null_no_default", None
+        )
+
+    def test_evalnull_default_insert(self):
+        self._test_insert(
+            "evals_null_default", 'nothing'
+        )
+
+    def test_evalnull_default_bulk_insert(self):
+        self._test_bulk_insert(
+            "evals_null_default", 'nothing'
+        )
+
+    def test_evalnull_default_insert_novalue(self):
+        self._test_insert_novalue(
+            "evals_null_default", 'default_val'
+        )
+
+    def test_evalnull_default_bulk_insert_novalue(self):
+        self._test_bulk_insert_novalue(
+            "evals_null_default", 'default_val'
+        )
+
+    def test_no_evalnull_nodefault_insert(self):
+        self._test_insert(
+            "no_eval_null_no_default", None
+        )
+
+    def test_no_evalnull_nodefault_bulk_insert(self):
+        self._test_bulk_insert(
+            "no_eval_null_no_default", None
+        )
+
+    def test_no_evalnull_nodefault_insert_novalue(self):
+        self._test_insert_novalue(
+            "no_eval_null_no_default", None
+        )
+
+    def test_no_evalnull_nodefault_bulk_insert_novalue(self):
+        self._test_bulk_insert_novalue(
+            "no_eval_null_no_default", None
+        )
+
+    def test_no_evalnull_default_insert(self):
+        self._test_insert(
+            "no_eval_null_default", 'default_val'
+        )
+
+    def test_no_evalnull_default_bulk_insert(self):
+        self._test_bulk_insert(
+            "no_eval_null_default", 'default_val'
+        )
+
+    def test_no_evalnull_default_insert_novalue(self):
+        self._test_insert_novalue(
+            "no_eval_null_default", 'default_val'
+        )
+
+    def test_no_evalnull_default_bulk_insert_novalue(self):
+        self._test_bulk_insert_novalue(
+            "no_eval_null_default", 'default_val'
+        )