I have an initial MR for phase 1 here: 
https://github.com/django/django/pull/13179. It's lacking docs but I think 
the rest of the points are covered.

It still needs docs, but a few observations/thoughts:

`UUIDField` is not useful at all without a `default` being set. It's likely 
not safe to add a `uuid.uuid4` default to the field, so documentation will 
have to show creating a `UUIDField` subclass. This might cause big 
headaches with migrations though, if the field is moved around.

M2MFields are still possibly an issue - while you might want UUIDFields for 
all models you also might want plain `BigAutoField's` for M2M through 
tables for efficiency. 

This error message[1] is not helpful and is more likely to crop up now. Is 
it worth adding a ticket to somehow improve the message?

If I change the `MODEL_DEFAULT_PK` setting to `django.db.models.UUIDField` 
and run a test on Postgres I end up with a weird situation:

from django.contrib.sites.models import Site
Site._meta.pk
<django.db.models.fields.UUIDField: id>
Site.objects.values_list("pk")
<QuerySet [(1,)]>

The "UUIDField.to_python" method is also never invoked. I'm not clear if 
this is something to do with the Django test suite or indicative of a 
deeper problem. 

1. https://code.djangoproject.com/ticket/30526

On Sunday, 28 June 2020 at 14:27:28 UTC+1 Tom Forbes wrote:

> I spent some time last week experimenting with my patch, the biggest issue 
> I could find was that under some situations we add explicit casts to array 
> fields in Postgres based on the *expected* PK field type (i.e `WHERE 
> [1,2,3]::int @> [1,2,3]::bigint`) which causes the query to be invalid. The 
> casts all look redundant however working out when they are redundant is 
> pretty complex, so I'm not sure if this approach has any legs.
>
> > Regarding third party apps a solution could be to allow a per-app config 
> option or encourage them to explicitly choose which primary key they use 
> for their model but I'm not a big fan of baking logic in the migration 
> framework to treat some fields in a special way.
>
> Auto-generated M2M tables are an issue here, we would need to add some 
> option to ManyToManyField() to allow passing in a PK field type. But I like 
> the approach of just saying "if you ship migrations, be explicit about your 
> primary keys" as it seems pretty simple.
>
> Overall I think your two phase plan seems like a good approach 🙌 - lets 
> do it?
>
> On Thursday, 18 June 2020 at 15:18:35 UTC+1 charettes wrote:
>
>> +1 from me as well but I don't think we should add any logic to the 
>> migration framework to handle the transition.
>>
>> I think the release plan should be something along the following
>>
>> Phase 1:
>> - New projects are generated with MODEL_DEFAULT_PK = 
>> 'django.db.models.BigAutoField'
>> - A system check or warning is emitted when the setting is not defined 
>> and MODEL_DEFAULT_PK default to 'django.db.models.AutoField' during the 
>> transition period. The warning should warn that the value will default to 
>> 'django.db.models.AutoField' in a future version of Django.
>> - An upgrade path is mentioned in the docs to mention that explicit ` id 
>> = AutoField()` must be assigned to preserve previous behaviour and avoid 
>> migrations. This one of the thing projects such as django-codemod could 
>> really help with[0]
>>
>> Phase 2:
>> - settings.MODEL_DEFAULT_PK now defaults to 
>> 'django.db.models.BigAutoField'
>> - System check/warning about undefined MODEL_DEFAULT_PK setting is 
>> removed.
>> - Let the migration framework generate migrations just like it normally 
>> would for projects that didn't follow the documented migration path.
>> - Optionally add a system check that warns about usage of `AutoField` 
>> because of its possible overflow.
>>
>> > I think we should restrict the setting between normal and big auto 
>> fields only. Allowing UUID's would be changing the type, with the potential 
>> for havoc with code incompalities throughout django. It's also not possible 
>> to migrate tables over to the new type.
>>
>> I'm not sure I agree here. For folks that are setting a up a new project 
>> starting with UUIDField primary keys can be useful and if we're adding a 
>> setting for this purpose I think we should it make it as flexible as 
>> possible.
>>
>>
>> [0] https://github.com/browniebroke/django-codemod
>>
>> Le jeudi 11 juin 2020 11:28:59 UTC-4, Tom Forbes a Ă©crit :
>>>
>>> I’d like to re-propose switching Django to use BigAutoField’s rather 
>>> than the current AutoField. This has been proposed[1] before (and a MR 
>>> made[2]) but it was closed due to implementation issues and not much else 
>>> has happened since then.
>>>
>>> As many of you are aware the max value a standard AutoField can hold is 
>>> 2,147,483,647 (2.1 billion) which sounds like more than you can ever need. 
>>> But it’s often not, and you only find out at the worst possible time - out 
>>> of the blue at night and during a period of rapid growth. The process for 
>>> fixing this issue also becomes a lot harder as your data grows - when 
>>> you’ve hit the limit you’re looking at a multi-hour `ALTER TABLE` on 
>>> Postgres during which writes and reads are blocked, or a risky operation to 
>>> create a new table with the correct primary key and copy old data over in 
>>> batches. Basically if you’ve experienced this before you wouldn’t wish it 
>>> on your worst enemy.
>>>
>>> I’m proposing that we add a `MODELS_PRIMARY_KEY` (name improvements 
>>> welcome!) setting that _defaults_ to `BigAutoField`, with prominent 
>>> docs/release notes saying that to preserve the existing behaviour this 
>>> should be set to `AutoField`. I think this the only realistic way we can 
>>> implement this for new projects in a way that ensures it will be used 
>>> meaningfully and not forgotten about until it’s too late.
>>>
>>> Rails managed to do this migration somewhat painlessly due the big 
>>> differences between Rails and Django models. As Django migrations are 
>>> derived from the current model state so there’s no way I can think of to 
>>> express “make this auto-generated field a BigAutoField only if this model 
>>> is *new*”. The way I see it is that a global setting is very easy to 
>>> toggle and there is little chance of missing the large numbers of 
>>> migrations that would be generated during the Django update. Smaller 
>>> applications could apply the migrations with little issue and larger 
>>> applications would be able to opt-out (as well as be reminded that this is 
>>> a problem they could face!).
>>>
>>> Some specifics:
>>> - The setting would take any dotted path to a class, or a single class 
>>> name for a build in field. This would potentially solve [3], and could be 
>>> useful to people who want to default to other fields like UUIDs (or a 
>>> custom BigAutoField) for whatever reason
>>> - The setting would also be used for GenericForeignKeys, which right now 
>>> are backed by a PositiveIntegerField and so suffer from the same AutoField 
>>> limitations
>>>
>>> Any thoughts on this?
>>>
>>> Tom
>>>
>>> 1. 
>>> https://groups.google.com/d/msg/django-developers/imBJwRrtJkk/P4g0Y87lAgAJ
>>>
>>> 2. https://github.com/django/django/pull/8924/
>>>
>>> 3. https://code.djangoproject.com/ticket/56 
>>> <https://groups.google.com/d/msg/django-developers/VFXZpHnuEJc/bbefjX9yCQAJ>
>>>
>>>
>>>
>>>

-- 
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/2c853449-c519-4605-be23-4faa21ed40d4n%40googlegroups.com.

Reply via email to