Needed feedback on the approach to solve bulk_update silently truncating values for size limited fields

[Akash Sen]

Ticket#33647 : bulk_update silently truncating values for size limited fields

Approach 1:

As mentioned by Simon in this comment we can introduce a new argument maybe named  generic in the database function Cast similar to the following

Cast(expr, ArrayField(CharField(max_length=20), generic=True).

Although as mentioned by Mariusz in this comment "we cannot introduce new keyword arguments into database function that will be only be used to fix usage elsewhere in the Django code."

But the explanation is still a little unclear to me, it would be great if someone can explain the reason.

Approach 2:

As we will not be able to introduce a new new keyword arguments into database function we can create a new database function named something like GenericCast,That will cast according to the type only without specifying the maximum length.

And we will use this to the only line causing this regression in bulk_update, i.e : https://github.com/django/django/blob/a1e4e86f923dc8387b0a9c3025bdd5d096a6ebb8/django/db/models/query.py#L765 .

I would like to move forward with the Approach 2. Some feedback would be great.

[charettes]

Hello Akash,

Another approach that isn't mentioned here but alluded to in the ticket is to actually move bulk_update away from using Case / When which is the root of the issue here.

We know that using Case / When is slow and is the reason why we need to perform casting in the first place. While knowing that there is complexity in doing so we already have pointers on how to approach the problem and it was already discussed in the past.

I think that even if it requires a larger investment than adding a new kwarg to Cast or have bulk_update call a new specialized bulk_update_cast method on the backend instead of checking for the requires_casted_case_in_updates flag it should be a strongly considered option.

Cheers,

Simon

[Akash Sen]

The argument of having new specialized bulk_update_cast method is more logical than the earlier two approaches. But the best approach should be drop using Case / When which will eventually come with #31202 or #29771. I wanted to propose these solution as a hotfix unless we get #31202 or #29771 done. As we are going to upgrade the bulk_update in near future, resolving the bug for now seems reasonable to me.

PS: Currently going through all the discussions for #31202 and #29771 as there are a lot of them, will try to propose a solution soon once the #21961 is closed.

Regards,
Akash Kumar Sen

LinkedIn | GitHub

[Mariusz Felisiak]

I wanted to propose these solution as a hotfix unless we get #31202 or #29771 done. As we are going to upgrade the bulk_update in near future, resolving the bug for now seems reasonable to me.

This is not a critical or security issue, we generally don't add "hotfixes" to bugs. Moreover, temporarily changing signatures of existing functions would be against our API stability policy.

[Akash Sen]

Got it. Thank you for the response Mariusz and Simon.