Column(JSON, default=None) defaults to SQL NULL in 1.1 instead of JSON null

[Adrian]

from datetime import datetime, date

from sqlalchemy import *
from sqlalchemy.orm import *
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.dialects.postgresql import JSON


Base = declarative_base()


class Foo(Base):
    __tablename__ = 'foo'
    id = Column(Integer, primary_key=True)
    json = Column(JSON, nullable=False, default=None)

e = create_engine('postgresql:///test', echo=True)
Base.metadata.create_all(e)
s = Session(e)


s.add(Foo())
s.flush()

This fails with the NOT NULL being violated. Using default=JSON.NULL works but unless I misunderstand the docs I shouldn't get a SQL NULL there.

[Mike Bayer]

can you clarify if behavior has changed since 1.0.x ?

[Mike Bayer]

also....."default=None" means, "there's no default". So....this is not looking like a bug. The migration notes you refer to don't really say the default would have this behavior, only the use of the term "never" I can see is misleading, can clarify that. later down it says "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. "

[Adrian]

Yes, in 1.0.x it was defaulting to a JSON null.

[Mike Bayer]

OK, this is different behavior from 1.0.x but it is correct. migration notes need to be clarified. if you want json null, you need to set Foo(json=None).

[Adrian]

OK, so I'll switch to default=JSON.NULL...

[Mike Bayer]

OK, now I'm not sure. I have to totally crack open the whole change, tests, changelog to see what happened.

[Mike Bayer]

yeah so, the thing is Column(... default=None) means you didn't set "default" at all, basically.

So if one makes a column like:

Column('x', Integer, nullable=False)

then you don't specify it in your flush. you'd want an IntegrityError raised. The change is that now if you have:

Column('x', JSON, nullable=False)

that also raises now. Not sure how the migration notes totally missed this and I need to see if the tests cover it.

[Adrian]

So the new behavior is correct? Just want to avoid changing tons of JSON columns to default=JSON.NULL unless needed

[Mike Bayer]

@thiefmaster this whole change came from your bug report in the first place, in #3514. Did you not test any of the three beta releases which included this change ?

[Mike Bayer]

So in particular, this line in the migration notes is wrong: "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. " because this is not the previous behavior for a missing default.

[Mike Bayer]

The ORM has always had the behavior that if a column has no defaults on it, an INSERT will render None for a column's value if it was omitted from the object - the new "evaulates_none" flag changes this.. I'm not sure at this point why we need that very old behavior, but it's not coming out in 1.1 at least. You can make your older behavior work if you do this:

class MyJSON(JSON):
    should_evaluate_none = False


class Foo(Base):
    __tablename__ = 'foo'
    id = Column(Integer, primary_key=True)
    json = Column(MyJSON(), nullable=False)

this is of course totally non-intuitive so I don't think I'm going to push this as a real workaround.

[Mike Bayer]

Rewrite migration notes for [ticket:3514]

The change to "evaluates none" datatypes in the ORM was not fully described in the migration notes, missing the key behavioral change that a column which is missing a default entirely will not receive a value for a missing JSON column now. The issue here touched upon a revisit of the assumptions in [ticket:3514], but overall the old behavior "worked" mostly because the ORM wants to explicitly render NULL into an INSERT for column values that are missing, which itself is a legacy behavior which should be considered for possible removal in a future major release. Given that "missing ORM value + no column default set up == dont put it in the INSERT" would be the most intuitive behavior, the move in [ticket:3514] represents a step in this direction.

Change-Id: I454d5bb0773bd73d9864925dcc47f1f0810e33ba Fixes: #3830

→ <<cset 76ec285ba452>>