Re: [Django] #33647: bulk_update and bulk_create silently truncating values for size limited fields on postgres

[Django]

#33647: bulk_update and bulk_create silently truncating values for size limited
fields on postgres
-------------------------------------+-------------------------------------
Reporter: jerch | Owner: Rowan
| Douglas
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Rowan Douglas):

Thank you for the suggestions. I will make sure to go over the history of
this ticket and the related tickets you mentioned before starting.

As a new contributor, I would like to clarify the backport reasoning. Am I
correct in understanding that changing the signature of Field.db_type is
only something that can occur on the active branch as it is not purely a
fix, but also a change in the general interface. Therefore we will make a
specific fix for the stable branches first, and then introduce the bigger
change as part of the next release.

Let me know if I have misunderstood anything.
--
Ticket URL: <https://code.djangoproject.com/ticket/33647#comment:30>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#33647: bulk_update and bulk_create silently truncating values for size limited
fields on postgres
-------------------------------------+-------------------------------------
Reporter: jerch | Owner: Rowan
| Douglas
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jacob Walls):

In principle that's right, but all contributions go through main, and
mergers will handle any backports, so just target `main`.

We really only need to repair 6.0 and 5.2 to the state before the `UNNEST`
strategy replicated this old issue inside `bulk_create`. Then the
`parameterized=False` bit to clean up that planned Postgres `bulk_create`
fix and finally fix `bulk_update` would not get backported, so make sure
that work is in a separate commit.
--
Ticket URL: <https://code.djangoproject.com/ticket/33647#comment:31>

[Django]

#33647: bulk_update and bulk_create silently truncating values for size limited
fields on postgres
-------------------------------------+-------------------------------------
Reporter: jerch | Owner: Rowan
| Douglas
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Jacob Walls):

Hi Rowan, just a heads up that we're targeting a patch release fixing 6.0
regressions on January 6. I'm likely to assign this to myself around Dec
30 or so if there's not a PR yet. Just wanted to over-communicate with the
holidays approaching. Thanks again.
--
Ticket URL: <https://code.djangoproject.com/ticket/33647#comment:32>

[Django]

#33647: bulk_update and bulk_create silently truncating values for size limited
fields on postgres
-------------------------------------+-------------------------------------
Reporter: jerch | Owner: Rowan
| Douglas
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Jacob Walls):

#36823 was a dupe for the 5.2 behavior in `bulk_create()`.
--
Ticket URL: <https://code.djangoproject.com/ticket/33647#comment:33>

[Django]

#33647: bulk_update and bulk_create silently truncating values for size limited
fields on postgres
-------------------------------------+-------------------------------------
Reporter: jerch | Owner: Rowan
| Douglas
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

Jacob, just to make sure we don't end up racing for an alternative
solution during the holidays I've tested out the `split("(")[0]` approach
[https://github.com/django/django/compare/main...charettes:django:ticket-33647
here] and it passes the full suite.
--
Ticket URL: <https://code.djangoproject.com/ticket/33647#comment:34>

[Django]

#33647: bulk_update and bulk_create silently truncating values for size limited
fields on postgres
-------------------------------------+-------------------------------------
Reporter: jerch | Owner: Rowan
| Douglas
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Lily):

* cc: Lily (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/33647#comment:35>

[Django]

#33647: bulk_update and bulk_create silently truncating values for size limited
fields on postgres
-------------------------------------+-------------------------------------
Reporter: jerch | Owner: Rowan
| Douglas
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jacob Walls <jacobtylerwalls@…>):

In [changeset:"d6ae2ed868e43671afc4d433c3d8f4d27f7eb555" d6ae2ed]:
{{{#!CommitTicketReference repository=""
revision="d6ae2ed868e43671afc4d433c3d8f4d27f7eb555"
Refs #33647 -- Fixed silent data truncation in bulk_create on Postgres.

Regression in a16eedcf9c69d8a11d94cac1811018c5b996d491.

The UNNEST strategy is affected by the same problem bulk_update has wrt/
to silent data truncation due to its usage of db_type which always returns
a parametrized subtype.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33647#comment:36>

[Django]

#33647: bulk_update and bulk_create silently truncating values for size limited
fields on postgres
-------------------------------------+-------------------------------------
Reporter: jerch | Owner: Rowan
| Douglas
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Jacob Walls <jacobtylerwalls@…>):

In [changeset:"764af478be8c25c186d27031c50c9a1d0697781b" 764af478]:
{{{#!CommitTicketReference repository=""
revision="764af478be8c25c186d27031c50c9a1d0697781b"
[6.0.x] Refs #33647 -- Fixed silent data truncation in bulk_create on

Postgres.

Regression in a16eedcf9c69d8a11d94cac1811018c5b996d491.

The UNNEST strategy is affected by the same problem bulk_update has wrt/
to silent data truncation due to its usage of db_type which always returns
a parametrized subtype.

Backport of d6ae2ed868e43671afc4d433c3d8f4d27f7eb555 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33647#comment:37>

[Django]

#33647: bulk_update and bulk_create silently truncating values for size limited
fields on postgres
-------------------------------------+-------------------------------------
Reporter: jerch | Owner: Rowan
| Douglas
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Jacob Walls <jacobtylerwalls@…>):

In [changeset:"2ca2afdffdc7a33344f3189965b6ebc92196dc10" 2ca2afdf]:
{{{#!CommitTicketReference repository=""
revision="2ca2afdffdc7a33344f3189965b6ebc92196dc10"
[5.2.x] Refs #33647 -- Fixed silent data truncation in bulk_create on

Postgres.

Regression in a16eedcf9c69d8a11d94cac1811018c5b996d491.

The UNNEST strategy is affected by the same problem bulk_update has wrt/
to silent data truncation due to its usage of db_type which always returns
a parametrized subtype.

Backport of d6ae2ed868e43671afc4d433c3d8f4d27f7eb555 from main.
}}}
--

Ticket URL: <https://code.djangoproject.com/ticket/33647#comment:38>

[Django]

#33647: bulk_update and bulk_create silently truncating values for size limited
fields on postgres
-------------------------------------+-------------------------------------
Reporter: jerch | Owner: Rowan
| Douglas
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |

Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* severity: Release blocker => Normal


Old description:

> On postgres backend, `bulk_update` passes overlong values for size
> limited fields along without any notification/exception, instead
> truncating the value.
>
> Repro:
> {{{
> #!div style="font-size: 80%"
> Code highlighting:
> {{{#!python
> # some model to repro
> class TestModel(models.Model):
> name = models.CharField(max_length=32)
>
> # in the shell
> >>> from bulk_test.models import TestModel
> >>> tm=TestModel(name='hello')
> >>> tm.save()
> >>> tm.name
> 'hello'
> >>> tm.name='m'*100
> >>> tm.save() # good, raises:
> ...
> django.db.utils.DataError: value too long for type character
> varying(32)
>
> >>> TestModel.objects.all().values('name')
> <QuerySet [{'name': 'hello'}]>
> >>> TestModel.objects.all().update(name='z'*100) # good, raises as
> well:
> ...
> django.db.utils.DataError: value too long for type character
> varying(32)
>
> >>> TestModel.objects.all().values('name')
> <QuerySet [{'name': 'hello'}]>
> >>> TestModel.objects.bulk_update([tm], ['name']) # not raising,
> instead truncating:
> 1
> >>> TestModel.objects.all().values('name')
> <QuerySet [{'name': 'mmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmm'}]>
> }}}
> }}}
>
> Not sure, if this is intended/expected behavior, well it is inconsistent
> to `.save` or `.update`, which both raise here. I only tested postgres
> backend for this, it may apply to other size limiting databases as well
> (sqlite itself is not affected, as it does not limit values).
>
> If this is intended, it may be a good idea to at least document the
> slightly different behavior, so users are aware of it, and can prepare
> their code to avoid silent truncation with follow-up errors. A better way
> prolly would fix `bulk_update` to spot value overflows and raise, but I
> am not sure, if thats feasible.

New description:

EDIT: This issue originally only affected `bulk_update`, then started
affecting `bulk_create` in Django 5.2, but that aspect was fixed in Django
5.2.10 and Django 6.0.1.

Original report follows:

----

On postgres backend, `bulk_update` passes overlong values for size limited
fields along without any notification/exception, instead truncating the
value.

Repro:
{{{
#!div style="font-size: 80%"
Code highlighting:
{{{#!python
# some model to repro
class TestModel(models.Model):
name = models.CharField(max_length=32)

# in the shell
>>> from bulk_test.models import TestModel
>>> tm=TestModel(name='hello')
>>> tm.save()
>>> tm.name
'hello'
>>> tm.name='m'*100
>>> tm.save() # good, raises:
...
django.db.utils.DataError: value too long for type character varying(32)

>>> TestModel.objects.all().values('name')
<QuerySet [{'name': 'hello'}]>
>>> TestModel.objects.all().update(name='z'*100) # good, raises as
well:
...
django.db.utils.DataError: value too long for type character varying(32)

>>> TestModel.objects.all().values('name')
<QuerySet [{'name': 'hello'}]>
>>> TestModel.objects.bulk_update([tm], ['name']) # not raising,
instead truncating:
1
>>> TestModel.objects.all().values('name')
<QuerySet [{'name': 'mmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmm'}]>
}}}
}}}

Not sure, if this is intended/expected behavior, well it is inconsistent
to `.save` or `.update`, which both raise here. I only tested postgres
backend for this, it may apply to other size limiting databases as well
(sqlite itself is not affected, as it does not limit values).

If this is intended, it may be a good idea to at least document the
slightly different behavior, so users are aware of it, and can prepare
their code to avoid silent truncation with follow-up errors. A better way
prolly would fix `bulk_update` to spot value overflows and raise, but I am
not sure, if thats feasible.

--
Comment:

Downgrading from release blocker now that we've backported the fix to
`bulk_create()`. Only `bulk_update()` is still affected (as before).
Clarified the ticket description, but leaving both methods in the ticket
title so that the reference from the release notes is understandable.
--
Ticket URL: <https://code.djangoproject.com/ticket/33647#comment:39>

[Django]

#33647: bulk_update and bulk_create silently truncating values for size limited
fields on postgres
-------------------------------------+-------------------------------------
Reporter: jerch | Owner: Rowan
| Douglas
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

When reviewing the currently proposed patch for #23902 it reminded me that
we also use `Field.db_type` based casting when performing field type
alteration on Postgres since #25002.

We ran into the exact same data truncation problem discussed here in
#28816 by naively using `db_type` casting via `USING` and I only now
realize that we only fixed in 1378d665a1c85897d951f2ca9618b848fdbba2e7 for
a subset of the cases.

We ''fixed'' the problem by not casting when the internal type remains the
same (e.g. `varfield(255) -> varfield(254)`) but the data loss problem
persist when moving from something like `integer` as to `varchar(2)` for
example as `len(str(999)) > 2` obviously.

It stroke me as yet another location where
`Field.db_type(parametrized=False)` would be useful as it would allow us
to
1. Keep not casting via `USING` when the unparametrized type remain the
same (what #28816 started doing)
2. In other case make sure to use `USING %(unparametrized_type)s` to avoid
silent data truncations

Here's [https://dryorm.xterm.info/postgres-alter-type-silent-data-
truncation a DryORM demonstrating the issue]. Not sure if we want a
standalone ticket or if we should fix all the `db_type` based casting
issues (`bulk_update`, schema editor) here.
--
Ticket URL: <https://code.djangoproject.com/ticket/33647#comment:40>