A FieldError should be raised when trying to update MTI inherited field with F reference to child field

[Shai Berger]

This is the converse of #30044.

In that bug, we made sure a proper error was raised when we do

ChildModel.objects.update(child_field=F('parent_field'))

We should similarly make sure a proper error is raised when we do

ChildModel.objects.update(parent_field=F('child_field'))

With current main, if we add the obvious test

tests/expressions/tests.py

@@ -318 +318 @@
         with self.assertRaisesMessage(FieldError, msg):
             RemoteEmployee.objects.update(adjusted_salary=F('salary') * 5)
 
+    def test_update_set_inherited_field_from_child(self):
+        msg = 'Joined field references are not permitted in this query'
+        with self.assertRaisesMessage(FieldError, msg):
+            RemoteEmployee.objects.update(salary=F('adjusted_salary') / 5)
+
     def test_object_update_unsaved_objects(self):
         # F expressions cannot be used to update attributes on objects which do
         # not yet exist in the database

it fails

======================================================================
FAIL: test_update_set_inherited_field_from_child (expressions.tests.BasicExpressionsTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.9/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/usr/lib/python3.9/unittest/case.py", line 592, in run
    self._callTestMethod(testMethod)
  File "/usr/lib/python3.9/unittest/case.py", line 550, in _callTestMethod
    method()
  File "/home/django/django/tests/expressions/tests.py", line 324, in test_update_set_inherited_field_from_child
    RemoteEmployee.objects.update(salary=F('adjusted_salary') / 5)
  File "/usr/lib/python3.9/contextlib.py", line 137, in __exit__
    self.gen.throw(typ, value, traceback)
  File "/home/django/django/django/test/testcases.py", line 687, in _assert_raises_or_warns_cm
    self.assertIn(expected_message, str(getattr(cm, cm_attr)))
  File "/usr/lib/python3.9/unittest/case.py", line 1096, in assertIn
    self.fail(self._formatMessage(msg, standardMsg))
  File "/usr/lib/python3.9/unittest/case.py", line 668, in fail
    raise self.failureException(msg)
AssertionError: 'Joined field references are not permitted in this query' not found in "Cannot resolve keyword 'adjusted_salary' into field. Choices are: company_ceo_set, company_point_of_contact_set, firstname, id, lastname, manager, manager_id, remoteemployee, salary"

----------------------------------------------------------------------

[Abhijith Ganesh]

Was this bug patched, I would like to work on this bug and patch it. Would that be possible @Shai Berger

[Mariusz Felisiak]

Shai, thanks for the report.

Abhijith, feel-free to prepare a patch.

[Abhijith Ganesh]

This is my first attempt of patching an actual django bug, now once i have made the patch and pushed it to my repository, I should be creating a PR. Is that all?

[Abhijith Ganesh]

I am self assigning myself for this issue to avoid confusion.

[Abhijith Ganesh]

For fixing this I should be adding a conditional statement on query.py file right? and can I get some elaboration as to which method I should be resolving, I am under the assumption the same resolve method (function) will be getting a new conditional statement

[Abhijith Ganesh]

if annotation and not allow_joins:
          raise FieldError('Parent field references are not allowed in this query.')

Would this be a good fix?
This is my first fix and I am looking forward to learn, thanks in advance.

[Shai Berger]

Hi Abhijith, sorry for failing to respond earlier.

The best answer is, try: I've included in the ticket description a failing test for it. Add this test, and check if your suggested fix makes it pass (and doesn't break other tests).

[Bhuvnesh]

I think there is no direct way to get child field from opts of parent model in inheritance. For the testcase provided by shai the value of field is None at ​this part ,throwing different field error ​here. Is it the expected behaviour? or we are supposed to get the child field and later raise field error Joined field references are not permitted in this query.

[Shai Berger]

Replying to Bhuvnesh:

I think there is no direct way to get child field from opts of parent model in inheritance.

That makes sense

For the testcase provided by shai the value of field is None at ​this part ,throwing different field error ​here. Is it the expected behaviour?

No, that's the bug...

or we are supposed to get the child field and later raise field error Joined field references are not permitted in this query.

Yes, basically. Or, putting it another way (given your input): The update() logic uses the parent model opts, because the fields to be updated are all from that model; but then it uses the parent model to resolve all references, and that is wrong if there are expressions referencing the child model. Or, more succinctly: The move to the parent model is too early.