Opened 7 years ago

Closed 7 years ago

#5514 closed (wontfix)

Consistency issue when using ForeignKeys

Reported by: PhiR Owned by: PhiR
Component: Core (Other) Version: master
Severity: Keywords: select_related
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Consider the following models:

class Operand(models.Model):
    value_1 = models.CharField(maxlength=200)
    value_2 = models.CharField(maxlength=200)

class Operation(models.Model):
    arg = models.ForeignKey(Operand)

Now let's suppose we want to do some work on Operands:

>>> common_arg = Operand(value_1="val1", value_2="val2")
>>> common_arg.save()
>>> op1 = Operation(arg = common_arg)
>>> op2 = Operation(arg = common_arg)
>>> op1.save()
>>> op2.save()
>>> operations = Operation.objects.all().select_related()
>>> operations
[<Operation: Operation object>, <Operation: Operation object>]
>>> operations[0].arg.value_1 = "new_val1"
>>> operations[0].arg.save()
>>> operations[1].arg.value_2 = "new_val2"
>>> operations[1].arg.save()
>>> modified_arg = Operand.objects.get(id=1)
# OOPS we lost one change here !!
>>> modified_arg.value_1
u'val1'
>>> modified_arg.value_2
u'new_val2'

# restore the initial state 
>>> modified_arg.value_2 = "val2"
>>> modified_arg.save()

# this is not limited to select_related but pervasive with FKs
>>> work_to_do = [(op, op.arg) for op in Operation.objects.all()]
>>> work_to_do[0][1].value_1 = "new_val1"
>>> work_to_do[0][1].value_1
'new_val1'

# oops, operation 2 will use the outdated argument
>>> work_to_do[1][1].value_1
u'val1'

#17 could provide a solution for this issue: caching instances in memory so get() and friends can return the instance we already have instead of creating a new, distinct one.

The issue is especially easy to encounter with select_related because all related fields will be preloaded in distinct instances in memory. Walking Operation.objects.all() and
modifying the Operands from there would load an new instance from the DB for each operation and work as expected in most cases.

Change History (2)

comment:1 Changed 7 years ago by PhiR

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Design decision needed

comment:2 Changed 7 years ago by jacob

  • Resolution set to wontfix
  • Status changed from assigned to closed

Though I agree this is counter-intuitive, this isn't going to be fixed. Doing so would require a complex identity-mapper style solution, and that doesn't fit with Django's "80% ORM" philosophy.

Note: See TracTickets for help on using tickets.
Back to Top