GSOC 2023 Discussion and Feedback: Database-level Cascades

[Akash Sen]

Hello everyone,
I’ve started this discussion to get feedback for my proposal for the 
project: Database-level Cascades Functionality to Django ORM. I have never 
contributed to any of the official Django projects earlier but I have 
experience using the framework for last 2 years. In this discussion I would 
like to propose a high level view of how this can be implemented, any kind 
of feedback and reference to resources for a more detailed and clear 
perspective about the project from your side would be very much appreciated,

Based on my study it seems quite straightforward and simple to implement.

   1. Conduct thorough research and gain an understanding of the existing 
   discussions, pull requests, and issues associated with adding support for 
   database-level cascading options in the Django web framework.
   2. Specify the requirements for the new feature, including its name and 
   behavior, and determine whether it should be a new option (on_delete_db) or 
   a modification of the existing one (on_delete).
   3. Develop a ForeignKey subclass that sets a flag indicating that the 
   database should manage the cascading options.
   4. Modify the DatabaseSchemaEditor’s add_field() and sql_create_fk() 
   methods to recognize the flag and generate SQL accordingly for the specific 
   database backend.
   5. Ensure that the implementation performs as intended when tested for a 
   single database backend. Then, generalize the implementation so that it 
   works for all supported database backends, generating appropriate SQL for 
   each.
   6. Integrate the implementation with the Django migrations framework to 
   ensure proper handling of database-level cascading options during schema 
   migrations.
   7. Write tests to verify that the new feature functions correctly and 
   does not interfere with existing functionality.
   8. Submit one or more pull requests containing the changes, and 
   collaborate with the Django community to address any feedback or issues 
   that may arise.

Here is a link to my proposal : Database-level Cascades Proposal - GSoC '23 
- Google Docs 5 


Due to my beginner level understanding of the codebase there is a lot of 
scope room for improvement. Your suggestions will be of great help.

-- 
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 django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/bd82a0de-8f0a-47b5-b40d-1672ed05736fn%40googlegroups.com.

Re: GSOC 2023 Discussion and Feedback: Database-level Cascades

[Akash Sen]

Hi David,
Thank you for your suggestion and nice implementation example. I would try 
to include that approach too.

*Regards,*
Akash Kumar Sen

LinkedIn <https://www.linkedin.com/in/akash-kumar-sen-220b651b0/> | GitHub 
<https://github.com/Akash-Kumar-Sen>

On Thursday, March 30, 2023 at 5:00:10 AM UTC+5:30 charettes wrote:

> The difficulty of switching to database level constraint effectively 
> resides in how your lose the great parts of cascade deletion emulations 
> most notably signals post and pre-delete signals.
>
> If you're willing to make this compromise then you must make sure that all 
> cycle of model relationships in your app are also switched to not using 
> post and pre-delete signals as `DB_CASCADE` and `CASCADE` don't play well 
> together. That's easier said than done when using third party applications 
> with limited support for swappable models or that require signals machinery 
>
> What I meant by that is that you can't have the following models
>
> class Foo(models.Model):
>pass
>
> class Bar(models.Model):
>foo = models.ForeignKey(Foo, on_delete=DB_CASCADE)
>
> class Baz(models.Model):
>bar = models.ForeignKey(Bar, on_delete=CASCADE)
>
> As nothing will trigger the Django cascade emulation deletion of Baz when 
> a Bar row gets a database initiated cascade deletion on Foo deletion and 
> you'll run into integrity errors. In the case of signals handling the 
> situation is similar, deleting a Foo object would not trigger a deletion 
> signals for receivers connected to Bar as a sender.
>
> A lot of this work has already been done[1] to cover these issues so I 
> consider this problem to be mostly a matter of pushing it through the 
> finish lines and adding extra checks for deletion signal registration on 
> models involved in `DB_CASCADE` chains.
>
> While it's true that you can use a `ForeignKeyConstraint(BaseConstraint)` 
> and `db_constraint=DO_NOTHING` to achieve the creation of database 
> constraints at the database level you do loose safe rails that ensures you 
> don't run into surprises in the medium term.
>
> Cheers,
> Simon
>
> [1] https://github.com/django/django/pull/14550
> Le mercredi 29 mars 2023 à 04:11:55 UTC-4, Carlton Gibson a écrit :
>
>> Hey David. 
>>
>> Nice example! 
>>
>> I've done this, again just PostgreSQL, overriding the schema 
>> editor's sql_create_fk and sql_create_column_inline_fk to add to the 
>> necessary ON DELETE (for an FK subclass), but doing it with a constraint is 
>> a lovely touch. (I shall play with that 😜)
>>
>> I didn't look at all at making it work for each backend (but it should 
>> no? 🤔)
>>
>> My need-to-double-check-but… thought on #21961 was that a new kwarg (such 
>> as, say) db_on_delete, which would error if used with on_delete, and set 
>> that to DO_NOTHING, responsible for controlling the generated SQL would be 
>> the way forward here. 
>>
>> That was all before constraints, though, so thoughts from 
>> Simon/Mariusz/... on best way forward would be good. 
>>
>> For me, it would be **great** (and akin with various other changes in 
>> recent versions) to be able to push this responsibility into the DB in a 
>> recommended/supported way. 
>>
>> Kind Regards,
>>
>> Carlton
>>
>> On Monday, 27 March 2023 at 18:16:37 UTC+2 David Sanders wrote:
>>
>>> Hi Akash,
>>>
>>> Database-level cascading deletes is a topic that has been discussed 
>>> often since, well probably the dawn of Django 😁  From recollection the 
>>> main issue isn't the implementation, it's getting it to play nicely with 
>>> Django's cascading emulation.
>>>
>>> There are other tickets, but I believe this is the main ticket to refer 
>>> to: https://code.djangoproject.com/ticket/21961  There are also plenty 
>>> of threads on this group I'm sure you could dig up.
>>>
>>> Note that with the introduction of constraints a while back, there's 
>>> nothing stopping people from adding their own custom foreign keys that adds 
>>> the necessary `ON DELETE` clauses. Here's a demonstration of that:  
>>> https://github.com/shangxiao/stupid-django-tricks/tree/master/abusing_constraints#database-level-cascading-deletes
>>>
>>> The gist is:
>>>
>>>1. Set your fks to "do nothing" (I haven't tested with other options 
>>>- seems like there could be some conflicts there though)
>>>2. Define your own fk by extending BaseConstraint and add the 
>&g

Added support for Database Level Cascades

[Akash Sen]

*Added support for Database Level Cascades*

*PR link *: https://github.com/django/django/pull/16851

*Ref discussion : *
https://groups.google.com/g/django-developers/c/Sxj7eS7-8SQ/m/jaM1rPbZEQAJ

*Approach :*
The on_delete argument for ForeignKey class now supports an extra option 
DB_CASCADE this will behave like DO_NOTHING in Django, but leverage  the 
Database level cascading options supported by SQL.

A new optional ar gument named  on_delete_db is also passed to the 
ForeignKey. This will specify the type of database level cascading we are 
looking for if the on_delete is set to DB_CASCADE. Supported values of the 
on_delete_db argument can be the following:

ON_DELETE_DB_CHOICES.DO_NOTHING_DB : Does nothing, used as default value.
ON_DELETE_DB_CHOICES.CASCADE_DB : Deletes the child on parent deletion.
ON_DELETE_DB_CHOICES.RESTRICT_DB : Prevent the deletion if child exists.
ON_DELETE_DB_CHOICES.SET_NULL_DB : Set the value to null in child table.

This will modify the SQL query during foreignkey generation.

*Checks performed :*
1. on_delete = DB_CASCADE and the model containing the field has a non 
abstract parent model. As inherited the model contains a parent_ptr with 
in-python cascading options,(OneToOneField with on_delete=models.CASCADE) 
 both cannot be used  together.
*Code :*
 
https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/django/db/models/fields/related.py#L1041
*Related testcase :*
 
https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/tests/db_cascade_checks/tests.py#L93

2. on_delete = DB_CASCADE and the model containing generic foreign keys or 
generic relation. In that case we prevent using DB_CASCADE too.
*Code :*
 
https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/django/db/models/fields/related.py#L1060
*Related testcase :*
 
https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/tests/db_cascade_checks/tests.py#L144

3. Both on_delete and on_delete_db are specified. In order to specify the 
value of on_delete_db, on_delete must be set to DB_CASCADE.
*Code :*
 
https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/django/db/models/fields/related.py#L1137
*Related testcase :*
 
https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/tests/db_cascade_checks/tests.py#L11

4. on_delete_db is set to SET_NULL_DB but the field is not nullable.
*Code :*
 
https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/django/db/models/fields/related.py#L1150
*Related testcase :*
 
https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/tests/db_cascade_checks/tests.py#L66

5. Restricting the use of normal cascading if the foreignkey parent uses 
DB_CASCADING as this will generate untracable integrity errors.
*Code :*  1. 
https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/django/db/models/fields/related.py#L1104
  2. 
https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/django/db/models/fields/related.py#L1162
*Related testcase :*
 
https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/tests/db_cascade_checks/tests.py#L33

-- 
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 django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/2b406f18-fbf1-4487-813b-582edf211971n%40googlegroups.com.

Re: Added support for Database Level Cascades

[Akash Sen]

No it will not trigger the delete signals. The point is to delete an object 
with O(1) database queries instead of O(n), Have to add another warning 
message if the signals are being used with DB level cascading.

On Sunday, May 14, 2023 at 11:45:56 AM UTC+5:30 Harro wrote:

> I have just one question, will this still trigger on delete signals for 
> cascaded models?
> That's the main reason the cascade happens in Django and not in the 
> database.
>
> On Saturday, 13 May 2023 at 16:35:01 UTC+2 Akash Sen wrote:
>
>> *Added support for Database Level Cascades*
>>
>> *PR link *: https://github.com/django/django/pull/16851
>>
>> *Ref discussion : *
>> https://groups.google.com/g/django-developers/c/Sxj7eS7-8SQ/m/jaM1rPbZEQAJ
>>
>> *Approach :*
>> The on_delete argument for ForeignKey class now supports an extra option 
>> DB_CASCADE this will behave like DO_NOTHING in Django, but leverage  the 
>> Database level cascading options supported by SQL.
>>
>> A new optional ar gument named  on_delete_db is also passed to the 
>> ForeignKey. This will specify the type of database level cascading we are 
>> looking for if the on_delete is set to DB_CASCADE. Supported values of the 
>> on_delete_db argument can be the following:
>>
>> ON_DELETE_DB_CHOICES.DO_NOTHING_DB : Does nothing, used as default value.
>> ON_DELETE_DB_CHOICES.CASCADE_DB : Deletes the child on parent deletion.
>> ON_DELETE_DB_CHOICES.RESTRICT_DB : Prevent the deletion if child exists.
>> ON_DELETE_DB_CHOICES.SET_NULL_DB : Set the value to null in child table.
>>
>> This will modify the SQL query during foreignkey generation.
>>
>> *Checks performed :*
>> 1. on_delete = DB_CASCADE and the model containing the field has a non 
>> abstract parent model. As inherited the model contains a parent_ptr with 
>> in-python cascading options,(OneToOneField with on_delete=models.CASCADE) 
>>  both cannot be used  together.
>> *Code :* 
>> https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/django/db/models/fields/related.py#L1041
>> *Related testcase :* 
>> https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/tests/db_cascade_checks/tests.py#L93
>>
>> 2. on_delete = DB_CASCADE and the model containing generic foreign keys 
>> or generic relation. In that case we prevent using DB_CASCADE too.
>> *Code :* 
>> https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/django/db/models/fields/related.py#L1060
>> *Related testcase :* 
>> https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/tests/db_cascade_checks/tests.py#L144
>>
>> 3. Both on_delete and on_delete_db are specified. In order to specify the 
>> value of on_delete_db, on_delete must be set to DB_CASCADE.
>> *Code :* 
>> https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/django/db/models/fields/related.py#L1137
>> *Related testcase :* 
>> https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/tests/db_cascade_checks/tests.py#L11
>>
>> 4. on_delete_db is set to SET_NULL_DB but the field is not nullable.
>> *Code :* 
>> https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/django/db/models/fields/related.py#L1150
>> *Related testcase :* 
>> https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/tests/db_cascade_checks/tests.py#L66
>>
>> 5. Restricting the use of normal cascading if the foreignkey parent uses 
>> DB_CASCADING as this will generate untracable integrity errors.
>> *Code :*  1. 
>> https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/django/db/models/fields/related.py#L1104
>>   2. 
>> https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/django/db/models/fields/related.py#L1162
>> *Related testcase :* 
>> https://github.com/django/django/blob/04df10e20853c8e549deddf46fac8c289e265225/tests/db_cascade_checks/tests.py#L33
>>
>>

-- 
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 django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/3e17a21f-7c90-4254-a429-fae6d8bc67e8n%40googlegroups.com.

Ticket #30382 force_insert flag is not passed when saving parents on inherited models

[Akash Sen]

*Ticket link : *https://code.djangoproject.com/ticket/30382

*Problem : *When saving we pass force_insert=True to prevent the extra 
UPDATE statement that precedes the INSERT. The force_insert flag is 
respected on the child table but not on the parent.

*The main problem : *On our first go this issue would seem an easy picking 
as we just have to pass the force_insert flag while saving the parents, but 
as mentioned here  it 
is going to create a disaster of integrity errors in the 
Model.objects.create() method with non-abstract inherited parents as the 
create method assumes force_insert=True.

*Approach 1: *Set the force_insert=False inside create method if that 
contains a non-abstract parent model. 

The problem with it is it generates an extra query while creating the non 
abstract parent model when the child exists, more details can be found 
here... 
 (Tried 
in the PR)

*Approach 2: *Pass an extra flag to the save method and if the method call 
is coming from create then prevent passing force_insert flag(i.e set it to 
false) while saving parents when called via create, but as we cannot change 
the signature of the save method we cannot simply take an extra flag, so 
this comes down to a very simple question : 

*How to pass a flag to a python method without changing its signature:*
I can think of three approaches for this,
1. Inspecting a stack frame(Deprecated because of performance issues) - 
Tried in the PR
2. Using a global variable (Deprecated because of performance issues)
3. Using a class variable (Deprecated because of backward compatibility as 
names can collide with method, variable, property name of a model) - Tried 
in the PR

*PR link :* https://github.com/django/django/pull/16830

-- 
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 django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/fd92a1b1-cc0f-4800-ad61-33a098959707n%40googlegroups.com.

Ticket #30382 force_insert flag is not passed when saving parents on inherited models

[Akash Sen]

*[*Looking for an alternate approach*]*
*Ticket link : *https://code.djangoproject.com/ticket/30382

*Problem : *When saving we pass force_insert=True to prevent the extra 
UPDATE statement that precedes the INSERT. The force_insert flag is 
respected on the child table but not on the parent.

*The main problem : *On our first go this issue would seem an easy picking 
as we just have to pass the force_insert flag while saving the parents, but 
as mentioned here  it 
is going to create a disaster of integrity errors in the 
Model.objects.create() method with non-abstract inherited parents as the 
create method assumes force_insert=True.

*Approach 1: *Set the force_insert=False inside create method if that 
contains a non-abstract parent model. 

The problem with it is it generates an extra query while creating the non 
abstract child model when the parent exists, more details can be found 
here... 
 (Tried 
in the PR)

*Approach 2: *Pass an extra flag to the save method and if the method call 
is coming from create then prevent passing force_insert flag(i.e set it to 
false) while saving parents when called via create, but as we cannot change 
the signature of the save method we cannot simply take an extra flag, so 
this comes down to a very simple question : 

*How to pass a flag to a python method without changing its signature:*
I can think of three approaches for this,
1. Inspecting a stack frame(Deprecated because of performance issues) - 
Tried in the PR
2. Using a global variable (Deprecated because of performance issues)
3. Using a class variable (Deprecated because of backward compatibility as 
names can collide with method, variable, property name of a model) - Tried 
in the PR

*PR link :* https://github.com/django/django/pull/16830

If someone can suggest an alternate approach that 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 django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/f20a4231-17ce-4f7d-88ca-064f73f3b5e8n%40googlegroups.com.

Ticket #34634 : Creating objects with nested MTI crashes

[Akash Sen]

Ticket : https://code.djangoproject.com/ticket/34634
PR : https://github.com/django/django/pull/16952

*Possible reason behind the bug:*

   - The local OneToOneField of place_ptr inside 
   ItalianRestaurantManyParents is being inherited from the place_ptr field 
   of Restaurant. Which is one of the ancestor of the current model.
   - That successfully inserts the model Place while calling _save_parents()
for Restaurant inside the recursion tree.
   - So the local field of place_ptr does not have an attribute with name 
   attname.

*Possible fix :*

   - When fetching the immediate parents inside the _save_parents() method, 
   eliminate the ones which have a subclass inside that list. As they willbe 
   called again when we insert their subclass.

*Problem with the fix:*
Not being able to reproduce the behavior of the failed test in Local with 
postgresql and sqlite3 database. All the assertions are passing there.

Traceback (most recent call last): File 
"/home/jenkins/workspace/pull-requests-focal/database/mysql/label/focal-pr/python/python3.11/tests/model_inheritance/tests.py",
 
line 169, in test_create_diamond_mti_common_parents 
self.assertEqual(itrmp.name, "Ristorante Miron") AssertionError: '' != 
'Ristorante Miron' + Ristorante Miron

-- 
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 django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/020f45df-a8ee-4d5e-9ee1-34593513dedcn%40googlegroups.com.

Re: Ticket #30382 force_insert flag is not passed when saving parents on inherited models

[Akash Sen]

Also the current method initially tries to UPDATE, if not UPDATED then it 
tries to INSERT. So the case of having consecutive UPDATES is already 
optimized with minimum possible number of queries.

On Friday, June 9, 2023 at 12:10:18 AM UTC+5:30 charettes wrote:

> Sarah, not a stupid question at all.
>
> While `force_update` is a somewhat opposite of `force_insert` there are 
> nuances in how they are handled in the face of MTI (multi-table 
> inheritance).
>
> The proposed issubclass'esque additional signature (ModelBase | 
> tuple[ModelBase]) works in the case of `force_insert` because if a base has 
> to be inserted then all of its subclasses must be as well. In other words, 
> if a row must be inserted all rows that depend on it for referential 
> integrity must as well. This property doesn't hold for `force_update` as 
> you might want to update a base but not all its children.
>
> We already have a way to specify which base should be  `force_update`d via 
> `update_fields` which even offer per-base-field granularity.
>
> To summarize, while `force_update` and `force_insert` are mutually 
> exclusive on a per-base basis I don't think the latter should accept the 
> same pattern particularly because it can already be achieved by using 
> `update_fields`.
>
> Cheers,
> Simon
> Le jeudi 8 juin 2023 à 10:56:08 UTC-4, Sarah Boyce a écrit :
>
>> +1
>> Looking at the PR it looks like a boolean is still an accepted value for 
>> force_insert which is nice as the feature looks to be used quite a bit 
>> (just a rough idea 
>> https://github.com/search?type=code&auto_enroll=true&q=force_insert%3DTrue
>> ). 
>>
>> Might be a stupid question, reading the docs 
>> https://docs.djangoproject.com/en/4.2/ref/models/instances/#ref-models-force-insert,
>>  
>> force_update is currently the opposite of force_insert. Would/should 
>> force_update accept the same pattern? 
>> On Friday, 19 May 2023 at 06:15:00 UTC+2 charettes wrote:
>>
>>> I left some notes on the PR but I think the crux of the issue here is 
>>> that you are trying to change the meaning of Model.save(force_insert=True) 
>>> from force the insert of the current model to force the insert of the model 
>>> and all its bases.
>>>
>>> This is a problem not only for QuerySet.create but likely for a many 
>>> other use cases in the wild that need a way to only force the creation of 
>>> the leaf table but not others. In other words, the change you are proposing 
>>> take one feature way (only insert leaf model) to introduce a new one 
>>> (insert all bases) and thus is fundamentally backward incompatible.
>>>
>>> To me the only way forward here is to introduce a new feature but as 
>>> you've mentioned changing the signature of Model.save() is comple.
>>>
>>> The only viable option I can think of is to change the accepted type for 
>>> force_insert from bool to Type[Model] | Tuple[Type[Model]] to allow 
>>> save_parent to determine which one should be forced inserted and which 
>>> shouldn't.
>>>
>>> In the case of the reported usage case
>>>
>>> ```
>>> class ParentModel(models.Model):
>>> id = models.BigIntegerField(primary_key=True)
>>>
>>> class ChildModel(ParentModel): pass
>>> ```
>>>
>>> That would mean that force_insert=ParentModel must be passed to opt-in 
>>> into the full insertion mode (both issubclass(ChildModel, ParentModel) 
>>> and issubclass(ParentModel, ParentModel))
>>>
>>> ```
>>> ChildModel(id=1).save(force_insert=ParentModel)
>>> ```
>>>
>>> That seems  like a viable option to me because
>>>
>>> - An empty tuple is falsy which is coherent with force_insert=False
>>> - A non-empty tuple is truthy and which is coherent with the fact the 
>>> current leaf model must be inserted (current behavior)
>>> - Complex MTI scenario forced insertion needs can be targeted with 
>>> specially crafted base tuples (think of diamond inheritance)
>>> - If a deep ancestor must be inserted then it means that all its 
>>> children must also be inserted (makes sense due to how children must 
>>> reference to parent)
>>> - force_insert=models.Model can be used as a generic way to specify that 
>>> all bases must be inserted independently of the chain of models involved
>>>
>>> Hope that makes sense!
>>>
>>> Cheers,
>>> Simon
>>>
>>> Le jeudi 18 mai 2023 à 22:46:35 UTC-4, Akash Se

Looking for a code that separates fields while applying migrations

[Akash Sen]

Hello everyone,
There are certain attributes in a field, after changing these the migration 
generated willbe applied to database, (for example take foreignkey) like 
null=True or db_default=1. As they have something to do with the database 
column.
There are some other attributes after changing these the migration 
generated will not be applied to database, like on_delete. As currently 
they have nothing to do with the database column.
In search of the code that takes care of that. Some help 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 django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/12539052-6e21-491a-b79e-e63edf451165n%40googlegroups.com.

Re: Looking for a code that separates fields while applying migrations

[Akash Sen]

This one worked :   SQL: 
https://github.com/django/django/blob/f4e72e6523e6968d9628dfbff914ab57dbf19e6b/django/db/models/fields/__init__.py#L142
 .
Thank you Adam.

On Monday, October 2, 2023 at 3:44:34 PM UTC+5:30 Adam Johnson wrote:

> See alte_field in the schema editor: 
> https://github.com/django/django/blob/f4e72e6523e6968d9628dfbff914ab57dbf19e6b/django/db/backends/base/schema.py#L811
>
> It steps through what has changed in the field and generates SQL for the 
> database relevant changes, queueing up statements with self.execute. Any 
> non-SQL changes are basically ignored.
>
> Field.non_db_attrs tracks the field attributes rhat don’t affect SQL: 
> https://github.com/django/django/blob/f4e72e6523e6968d9628dfbff914ab57dbf19e6b/django/db/models/fields/__init__.py#L142
>  . 
> Note they may be modified by field subclasses, such as inDjango-MySQL’s 
> EnumField.
>
> On Sat, Sep 30, 2023, at 3:57 PM, Akash Sen wrote:
>
> Hello everyone,
> There are certain attributes in a field, after changing these the 
> migration generated willbe applied to database, (for example take 
> foreignkey) like null=True or db_default=1. As they have something to do 
> with the database column.
> There are some other attributes after changing these the migration 
> generated will not be applied to database, like on_delete. As currently 
> they have nothing to do with the database column.
> In search of the code that takes care of that. Some help 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 django-develop...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/django-developers/12539052-6e21-491a-b79e-e63edf451165n%40googlegroups.com
>  
> <https://groups.google.com/d/msgid/django-developers/12539052-6e21-491a-b79e-e63edf451165n%40googlegroups.com?utm_medium=email&utm_source=footer>
> .
>
>
>

-- 
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 django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/c0b8d3cc-cef3-411a-a975-21db8aa70acfn%40googlegroups.com.

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.

-- 
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 django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/b338adc5-78ce-4a78-8fe2-d7f5d5b17cban%40googlegroups.com.

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

[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 
<https://code.djangoproject.com/ticket/31202> or #29771 
<https://code.djangoproject.com/ticket/29771#ticket>. I wanted to propose 
these solution as a hotfix unless we get #31202 
<https://code.djangoproject.com/ticket/31202> or #29771 
<https://code.djangoproject.com/ticket/29771#ticket> 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 
<https://code.djangoproject.com/ticket/31202> and #29771 
<https://code.djangoproject.com/ticket/29771#ticket> as there are a lot of 
them, will try to propose a solution soon once the #21961 
<https://code.djangoproject.com/ticket/21961> is closed.

*Regards,*
Akash Kumar Sen

LinkedIn <https://www.linkedin.com/in/akash-kumar-sen-220b651b0/> | GitHub 
<https://github.com/Akash-Kumar-Sen>

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 
> <https://code.djangoproject.com/ticket/33647#comment:4> 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 
> <https://code.djangoproject.com/ticket/31202> and is the reason why we 
> need to perform casting in the first place 
> <https://github.com/django/django/pull/9606#discussion_r173390105>. While 
> knowing that there is complexity 
> <https://code.djangoproject.com/ticket/33647#comment:5> in doing so we 
> already have pointers on how to approach the problem 
> <https://code.djangoproject.com/ticket/29771> and it was already 
> discussed in the past 
> <https://groups.google.com/g/django-developers/c/jIGj1KKuaDM/m/zZERk0MpAgAJ>
> .
>
> 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 <https://code.djangoproject.com/ticket/33647>
>>
>> *Approach 1:*
>> As mentioned by Simon in this comment 
>> <https://code.djangoproject.com/ticket/33647#comment:3> 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 
>> <https://github.com/django/django/pull/17366#issuecomment-1763440668> "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 django-developers+unsubscr...@googlegroups.com.
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

[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 django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/bfce41c1-b914-45d6-ac52-08f2e0f3a306n%40googlegroups.com.