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

2023-10-16 Thread Akash Sen 
Got it. Thank you for the response Mariusz and Simon.

On Tuesday, October 17, 2023 at 11:43:00 AM UTC+5:30 Mariusz Felisiak wrote:

> 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. 
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/bfce41c1-b914-45d6-ac52-08f2e0f3a306n%40googlegroups.com.

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

2023-10-16 Thread 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. 

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/70924d73-bdb3-46bf-a50e-3331dd93c27an%40googlegroups.com.

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

2023-10-16 Thread 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 


On Tuesday, October 17, 2023 at 4:27:50 AM UTC+5:30 charettes wrote:

> 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
> Le lundi 16 octobre 2023 à 10:17:07 UTC-4, Akash Sen a écrit :
>
>> 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.
>>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/ee2de40e-398c-47b1-ad33-c2be3947c47bn%40googlegroups.com.

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

2023-10-16 Thread 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
Le lundi 16 octobre 2023 à 10:17:07 UTC-4, Akash Sen a écrit :

> 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.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/9ce02adc-ab52-44af-8274-dabc04208a10n%40googlegroups.com.