**tl;dr**
This will cause psycopg to raise `ProgrammingError: cannot adapt type
'dict'`
{{{
class Foo(models.Model):
json = models.JSONField(db_default={"foo": "bar"})
}}}
however this works:
{{{
class Foo(models.Model):
json = models.JSONField(db_default=json.dumps({"foo": "bar"}))
}}}
At the very least this may be a doc update but I feel like it violates
principle of least astonishment to not use the same type that JSONField
attrs use. If we do decide to make this consistent it seems like
`BaseSchemaEditor.db_default_sql()` [1] just needs to be aware of the
field's type and adapt it correctly βΒ something like calling
`get_db_prep_value()`?
**Further details**
Migration generated without json.dumps:
{{{
class Migration(migrations.Migration):
initial = True
dependencies = []
operations = [
migrations.CreateModel(
name="Foo",
fields=[
(
"id",
models.BigAutoField(
auto_created=True,
primary_key=True,
serialize=False,
verbose_name="ID",
),
),
("json", models.JSONField(db_default=models.Value({"foo":
"bar"}))),
],
),
]
}}}
Traceback when applying above migration:
{{{
django-sample % dj migrate jsonfield_dbdefault
Operations to perform:
Apply all migrations: jsonfield_dbdefault
Running migrations:
Applying jsonfield_dbdefault.0001_initial...Traceback (most recent call
last):
File "/path/to/projects/django-sample/./manage.py", line 22, in <module>
main()
File "/path/to/projects/django-sample/./manage.py", line 18, in main
execute_from_command_line(sys.argv)
File "/path/to/projects/django/django/core/management/__init__.py", line
442, in execute_from_command_line
utility.execute()
File "/path/to/projects/django/django/core/management/__init__.py", line
436, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/path/to/projects/django/django/core/management/base.py", line
413, in run_from_argv
self.execute(*args, **cmd_options)
File "/path/to/projects/django/django/core/management/base.py", line
459, in execute
output = self.handle(*args, **options)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/path/to/projects/django/django/core/management/base.py", line
107, in wrapper
res = handle_func(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File
"/path/to/projects/django/django/core/management/commands/migrate.py",
line 356, in handle
post_migrate_state = executor.migrate(
^^^^^^^^^^^^^^^^^
File "/path/to/projects/django/django/db/migrations/executor.py", line
135, in migrate
state = self._migrate_all_forwards(
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/path/to/projects/django/django/db/migrations/executor.py", line
167, in _migrate_all_forwards
state = self.apply_migration(
^^^^^^^^^^^^^^^^^^^^^
File "/path/to/projects/django/django/db/migrations/executor.py", line
252, in apply_migration
state = migration.apply(state, schema_editor)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/path/to/projects/django/django/db/migrations/migration.py", line
132, in apply
operation.database_forwards(
File
"/path/to/projects/django/django/db/migrations/operations/models.py", line
97, in database_forwards
schema_editor.create_model(model)
File "/path/to/projects/django/django/db/backends/base/schema.py", line
485, in create_model
self.execute(sql, params or None)
File "/path/to/projects/django/django/db/backends/postgresql/schema.py",
line 46, in execute
sql = self.connection.ops.compose_sql(str(sql), params)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File
"/path/to/projects/django/django/db/backends/postgresql/operations.py",
line 193, in compose_sql
return mogrify(sql, params, self.connection)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File
"/path/to/projects/django/django/db/backends/postgresql/psycopg_any.py",
line 22, in mogrify
return ClientCursor(cursor.connection).mogrify(sql, params)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/path/to/projects/django-sample/.direnv/python-3.11/lib/python3.11
/site-packages/psycopg/client_cursor.py", line 40, in mogrify
pgq = self._convert_query(query, params)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/path/to/projects/django-sample/.direnv/python-3.11/lib/python3.11
/site-packages/psycopg/client_cursor.py", line 79, in _convert_query
pgq.convert(query, params)
File "/path/to/projects/django-sample/.direnv/python-3.11/lib/python3.11
/site-packages/psycopg/_queries.py", line 213, in convert
self.dump(vars)
File "/path/to/projects/django-sample/.direnv/python-3.11/lib/python3.11
/site-packages/psycopg/_queries.py", line 223, in dump
self.params = tuple(
^^^^^^
File "/path/to/projects/django-sample/.direnv/python-3.11/lib/python3.11
/site-packages/psycopg/_queries.py", line 224, in <genexpr>
self._tx.as_literal(p) if p is not None else b"NULL" for p in params
^^^^^^^^^^^^^^^^^^^^^^
File "/path/to/projects/django-sample/.direnv/python-3.11/lib/python3.11
/site-packages/psycopg/_transform.py", line 198, in as_literal
dumper = self.get_dumper(obj, PY_TEXT)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/path/to/projects/django-sample/.direnv/python-3.11/lib/python3.11
/site-packages/psycopg/_transform.py", line 245, in get_dumper
raise ex from None
File "/path/to/projects/django-sample/.direnv/python-3.11/lib/python3.11
/site-packages/psycopg/_transform.py", line 243, in get_dumper
dcls = self.adapters.get_dumper(key, format)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/path/to/projects/django-sample/.direnv/python-3.11/lib/python3.11
/site-packages/psycopg/_adapters_map.py", line 223, in get_dumper
raise e.ProgrammingError(
psycopg.ProgrammingError: cannot adapt type 'dict' using placeholder '%t'
(format: TEXT)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35149>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
This is definitely a bug.
The `JSONField` could have an explicit `encoder` assigned that is required
to be used for the `db_default` and it should be considered as well.
Two questions for you David
1. If you edit the migration to assign an `output_field` to the `Value`
does it address the issue? e.g.
`models.JSONField(db_default=models.Value({"foo": "bar"}, JSONField()))`
2. Can you also reproduce on `psycopg2`?
--
Ticket URL: <https://code.djangoproject.com/ticket/35149#comment:1>
1. Nice, yes editing the migration to define `output_field=JSONField()`
works π
2. Yes reproducible on psycopg2
--
Ticket URL: <https://code.djangoproject.com/ticket/35149#comment:2>
Ok then the solution is likely related IMO.
We could either have the current field being assigned as `output_field` of
the `Value` at deconstruction time (without `db_default` to prevent
recursions π
) or, likely more nicely, when `db_default` associated is
generated if we're dealing with a `Value` and it doesn't have an explicit
`_output_field` then use the current field.
I'm not sure why we wrap `db_default` in `Value` at initialization time in
the first place. The fact that a provided value doesn't adhere to the
expression protocol seems like it could have been a nice way to provide
the `Value` wrapper only when necessary at run time instead.
Some ideas
[https://github.com/django/django/compare/main...charettes:django:ticket-35149
in this branch]. Ultimately we'll need to find a way to ''bind''
`Value._output_field is None` to the current field, not wrapping
`db_default` in `Value` in the first place seems like it makes it easier
to achieve but it does cause all migrations generated since 5.0 release
that make use of `db_default` with a literal require a new noop migration.
--
Ticket URL: <https://code.djangoproject.com/ticket/35149#comment:3>
I like it π
The low impact change I was thinking was to assign `field` as the output
field in `db_default_sql()` however I feel that centralising this is
better.
2 questions: If someone provided supplied an expression without an
output_field, and it's not resolvable, should your cached property try to
set it?
Also should there be a check to warn if someone supplied an expression
with a resolved type not matching the field?
eg currently setting something like `foo =
models.IntegerField(db_default="asdf")` will happily generate a migration
but will error when it comes time to migrate.
--
Ticket URL: <https://code.djangoproject.com/ticket/35149#comment:4>
* cc: Simon Charette (added)
Comment:
Simon, David, thanks for the details so far! Would any of you be available
to provide a fix for this?
--
Ticket URL: <https://code.djangoproject.com/ticket/35149#comment:5>
> If someone provided supplied an expression without an output_field, and
it's not resolvable, should your cached property try to set it?
I feel like implicit `output_field` assignment only makes sense in a
context where we offer no way for the user to set it such as when they
provide a non-expression themselves. In all other cases the user can
provide an explicit `output_field` so it feels like we'd be guessing for
them.
> Also should there be a check to warn if someone supplied an expression
with a resolved type not matching the field?
That's a good one. I'd say we might want to consider a check in future
versions but not necessarily backport such check to 5.0 and have this work
included as part of this ticket?
> Simon, David, thanks for the details so far! Would any of you be
available to provide a fix for this?
I'm happy to provide a fix myself or let someone familiar with the
`db_default` work do it as well and provide a review. Were you interested
in taking the work when you reported the issue David?
--
Ticket URL: <https://code.djangoproject.com/ticket/35149#comment:6>
> I feel like implicit output_field assignment only makes sense in a
context where we offer no way for the user to set it such as when they
provide a non-expression themselves. In all other cases the user can
provide an explicit output_field so it feels like we'd be guessing for
them.
Yep agreed, so thinking more about this:
>when they provide a non-expression themselves.
This is handled when you wrap it with Value so all good β
> in all other cases
What made me wonder was this case `foo =
JSONField(db_default=Value({'foo': 'bar'}))` however if we setup the
warning one option may be to include warning about expressions that don't
resolve like this.
For complex expressions that don't resolve ideally an output_field should
be supplied. Implicit setting output_field isn't helpful here π
> That's a good one. I'd say we might want to consider a check in future
versions but not necessarily backport such check to 5.0 and have this work
included as part of this ticket?
Yup not as a backport π As per process will ask on forum before creating
a ticket.
> Were you interested in taking the work when you reported the issue
David?
Not especially π€·ββοΈ I feel like you've already authored a solution βΊοΈ
--
Ticket URL: <https://code.djangoproject.com/ticket/35149#comment:7>
* owner: nobody => Simon Charette
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/35149#comment:8>
* cc: Lily Foote (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/35149#comment:9>
* cc: Carlton Gibson (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/35149#comment:10>
Thank you Simon for grabbing this ticket. Is there anything that we/I can
do to help?
--
Ticket URL: <https://code.djangoproject.com/ticket/35149#comment:11>
I should be able to work on it this weekend, this have been a bit hectic
at work this week. If someone wants to pick it up to work begins sooner
feel free to do so, I'm happy to review the changes.
--
Ticket URL: <https://code.djangoproject.com/ticket/35149#comment:12>
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/35149#comment:13>
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/35149#comment:14>
In [changeset:"fe1cb62f5c3f87fafc4a6b52fee2ccc6c80c41e2" fe1cb62]:
{{{#!CommitTicketReference repository=""
revision="fe1cb62f5c3f87fafc4a6b52fee2ccc6c80c41e2"
Refs #35149 -- Made equivalent db_default alterations noops.
This allows for an easier transition of preserving the literal nature of
non-compilable db_default.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35149#comment:15>
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"e67d7d70fa10c06aca36b9057f82054eda45269d" e67d7d70]:
{{{#!CommitTicketReference repository=""
revision="e67d7d70fa10c06aca36b9057f82054eda45269d"
Fixed #35149 -- Fixed crashes of db_default with unresolvable output
field.
Field.db_default accepts either literal Python values or compilables
(as_sql) and wrap the former ones in Value internally.
While 1e38f11 added support for automatic resolving of output fields for
types such as str, int, float, and other unambigous ones it's cannot do
so for all types such as dict or even contrib.postgres and contrib.gis
primitives.
When a literal, non-compilable, value is provided it likely make the
most sense to bind its output field to the field its attached to avoid
forcing the user to provide an explicit `Value(output_field)`.
Thanks David Sanders for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35149#comment:16>
In [changeset:"914eee1a9bee7ae48e6b23ef905ec00bcc241791" 914eee1]:
{{{#!CommitTicketReference repository=""
revision="914eee1a9bee7ae48e6b23ef905ec00bcc241791"
[5.0.x] Refs #35149 -- Made equivalent db_default alterations noops.
This allows for an easier transition of preserving the literal nature of
non-compilable db_default.
Backport of fe1cb62f5c3f87fafc4a6b52fee2ccc6c80c41e2 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35149#comment:17>
In [changeset:"761946f8e1b6d725f83fa4f3b04ca9750f486009" 761946f8]:
{{{#!CommitTicketReference repository=""
revision="761946f8e1b6d725f83fa4f3b04ca9750f486009"
[5.0.x] Fixed #35149 -- Fixed crashes of db_default with unresolvable
output field.
Field.db_default accepts either literal Python values or compilables
(as_sql) and wrap the former ones in Value internally.
While 1e38f11 added support for automatic resolving of output fields for
types such as str, int, float, and other unambigous ones it's cannot do
so for all types such as dict or even contrib.postgres and contrib.gis
primitives.
When a literal, non-compilable, value is provided it likely make the
most sense to bind its output field to the field its attached to avoid
forcing the user to provide an explicit `Value(output_field)`.
Thanks David Sanders for the report.
Backport of e67d7d70fa10c06aca36b9057f82054eda45269d from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35149#comment:18>