Too aggressive pk control in create_reverse_many_to_one_manager

[Claude Paroz]

In the context of #19580, Django now requires an instance pk to even instanciate a related manager [7ba6ebe9149a].

Now I have a use case where I need to introspect the model used by a related manager (MyModel().related_set.model) and Django 4.1 refuses that with ValueError: 'MyModel' instance needs to have a primary key value before this relationship can be used.

My opinion is that is is too aggressive of a check and would suggest to let the __init__ succeed even if the instance has no pk. Other calls to _check_fk_val in the class seems sufficient to me to safeguard against shooting in the foot.

[Simon Charette]

I think this worth doing, having get_queryset perform self._check_fk_val() should catch the currently tested .all() case as well.

In the mean time I think you should be able to use MyModel._meta.get_field("related_set") for your introspection needs.

[Claude Paroz]

Replying to Simon Charette:
...

In the mean time I think you should be able to use MyModel._meta.get_field("related_set") for your introspection needs.

Thanks for the tip! I just needed to remove the _set as it's not part of the ManyToOneRel field name in _meta.

[David Wobrock]

I provided a small ​PR with the changes discussed by Simon.
There a few behaviour changes, tell me what you think! :)

[Mariusz Felisiak]

Regression in 7ba6ebe9149ae38257d70100e8bfbfd0da189862.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 806e9e2d:

Fixed #33952 -- Reallowed creating reverse foreign key managers on unsaved instances.

Thanks Claude Paroz for the report.

Regression in 7ba6ebe9149ae38257d70100e8bfbfd0da189862.

[Mariusz Felisiak <felisiak.mariusz@…>]

In fca05531:

[4.1.x] Fixed #33952 -- Reallowed creating reverse foreign key managers on unsaved instances.

Thanks Claude Paroz for the report.

Regression in 7ba6ebe9149ae38257d70100e8bfbfd0da189862.

Backport of 806e9e2d0dcf8f58e376fb7e2a8b9771e2a9ce16 from main

[Claude Paroz]

Many thanks David for the fix!

[Steven Mapes]

Replying to Claude Paroz:

Many thanks David for the fix!

Are we sure this is fixed as I still get these errors in all 4.1 versions? I posted an example on another ticket, #33984, which I thought was more related to the issue. you can find that report here

[Alex Matos]

I already made a comment on #19580, but since this ticket is related, I'm gonna comment here as well. I believe the changes on these two tickets should be fully reverted. I'm upgrading a large Django codebase from version 3.2 to 4.2 and a lot of code broke due to these changes, where code that was once returning an empty queryset, now raises a ValueError. In the name of Django's stability, I think changing user code behavior without a deprecation warning should be avoided at all costs.

[Andrey Ferriyan]

Any news on this?.

Several codes broke due to the migration to the stable LTS version 4.2. At least we have like temporary solution if it is possible.

[Mariusz Felisiak]

Replying to Andrey Ferriyan:

Any news on this?.

Several codes broke due to the migration to the stable LTS version 4.2. At least we have like temporary solution if it is possible.

I'm not sure what kind of news do you expect. This change was made in Django 4.1 (which is already in extended support). Nobody started a discussion as I requested 20 months ago, so there is nothing new here that could change a previous decision. Also, please don't spread comments across multiple tickets. If you have something to add, leave a comment in #19580.