id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
34207	"Incorrect SQL query when adding a ManyToMany related object with a ""through"" table prevents adding a new relationship if the new relationship is identical except for a different value for ""through_defaults"""	Credentive	nobody	"I cannot add a related element to an object that is related via a ManyToMany relationship with a through field. I believe I have traced the error to a bug in the SQL query generated.

Some Code extracts to set the stage:

models.py:

{{{
class Version(models.Model):
uuid = models.UUIDField(primary_key=True, unique=True, editable=False, default=uuid.uuid4, )
<...>

class Section(models.Model):
uuid = models.UUIDField(primary_key=True, unique=True, editable=False, default=uuid.uuid4, )
<...>


class Statement(models.Model):
uuid = models.UUIDField(primary_key=True, unique=True, editable=False, default=uuid.uuid4, )
<...>
section = models.ManyToManyField(
        Section, through=""StatementLocation"", symmetrical=False, related_name=""statements_in_section""
    )


class StatementLocation(models.Model):
    section = models.ForeignKey(Section, on_delete=models.CASCADE)
    statement = models.ForeignKey(Statement, on_delete=models.CASCADE)
    sl_version = models.ForeignKey(Version, on_delete=models.CASCADE)

    class Meta:
        constraints = [
            models.UniqueConstraint(
                fields=[""section"", ""statement"", ""sl_version""], name=""unique_statement_location_per_version""
            ),
            models.UniqueConstraint(fields=[""statement"", ""sl_version""], name=""statement_appears_once_per_version""),
        ]
}}}

The idea behind the model is that we are representing a document that has statements in sections. Statements can be moved around in different versions, so the relationship between a statement and a section, called ""statement location"", must be tied to a version.

I have a function under the Version model called ""copy"". The ""copy"" function produces a copy of the version and recreates the existing relationships. 

models.Version (extract):

{{{
    def copy(self, name: str, public: bool) -> ""Version"":
        # Make a copy of a published version and return a reference to the copy
        # Create the new version object in the Database
        new_version = Version(
            name=name,
            state=""DRAFT"",
            public=public,
            obsoletes=self,
            authority=self.authority,
        )
        new_version.save()

        return new_version
        <...>

        for statement in self.version_data.statements:
            <...>
            sect = statement.section.filter(version=self).get()
            statement.section.add(sect, through_defaults={""sl_version"": new_version})
            <...>
}}}

When executing the function,  I noticed that the new ""StatementLocation"" objects were not being created. For example:


{{{
>>> Version.objects.all()
<QuerySet [<Version: Document - 2.2>]>
>>> version = Version.objects.get()
>>> StatementLocation.objects.count()
89
>>> StatementLocation.objects.filter(sl_version=version).count()
89
>>> new_verson = version.copy(name=""2.3"", public=True)
>>> new_verson
<Version: Document - 2.3>
>>> StatementLocation.objects.count()
89
>>> StatementLocation.objects.filter(sl_version=new_verson).count()
0
>>> 
}}}

If I try a simplified version, with SQL query logging enabled, I get the following:


{{{
# Create New Version
>>> new_version = Version(name=""2.3"", state=""DRAFT"", public=True, obsoletes=version, authority=version.authority)

# Save It
>>> new_version.save()
(0.002) SELECT ""policypublisher_version"".""uuid"", ""policypublisher_version"".""name"", ""policypublisher_version"".""state"", ""policypublisher_version"".""public"", ""policypublisher_version"".""published_date"", ""policypublisher_version"".""effective_date"", ""policypublisher_version"".""obsoletes_id"", ""policypublisher_version"".""authority_id"" FROM ""policypublisher_version"" WHERE ""policypublisher_version"".""uuid"" = 'bcbb218ebdc5435b9e5caf879b55cd24' LIMIT 21; args=('bcbb218ebdc5435b9e5caf879b55cd24',); alias=default
(0.014) INSERT INTO ""policypublisher_version"" (""uuid"", ""name"", ""state"", ""public"", ""published_date"", ""effective_date"", ""obsoletes_id"", ""authority_id"") VALUES ('bcbb218ebdc5435b9e5caf879b55cd24', '2.3', 'DRAFT', 1, NULL, NULL, 'be8225c85fab4a0ba8ea879a3d992abe', '833c1b9b30964f3a90b045e2a1254c1b'); args=('bcbb218ebdc5435b9e5caf879b55cd24', '2.3', 'DRAFT', True, None, None, 'be8225c85fab4a0ba8ea879a3d992abe', '833c1b9b30964f3a90b045e2a1254c1b'); alias=default

# Get a random statement
>>> statement=Statement.objects.first()
(0.003) SELECT ""policypublisher_statement"".""uuid"", ""policypublisher_statement"".""text"", ""policypublisher_statement"".""type"", ""policypublisher_statement"".""public"", ""policypublisher_statement"".""obsoletes_id"" FROM ""policypublisher_statement"" ORDER BY ""policypublisher_statement"".""uuid"" ASC LIMIT 1; args=(); alias=default

# Make sure we got one
>>> statement
<Statement: Rules for interpreting the pivFASC-N name>

# Add the new version to the statement's version list
>>> statement.version.add(new_version)
(0.000) BEGIN; args=None; alias=default
(0.006) INSERT OR IGNORE INTO ""policypublisher_statement_version"" (""statement_id"", ""version_id"") VALUES ('0509afa269604028818dc1f53b8f93d2', 'bcbb218ebdc5435b9e5caf879b55cd24'); args=('0509afa269604028818dc1f53b8f93d2', 'bcbb218ebdc5435b9e5caf879b55cd24'); alias=default

# get the section for the statement under the old version
>>> sect=statement.section.filter(version=version).get()
(0.001) SELECT ""policypublisher_section"".""uuid"", ""policypublisher_section"".""name"", ""policypublisher_section"".""public"" FROM ""policypublisher_section"" INNER JOIN ""policypublisher_statementlocation"" ON (""policypublisher_section"".""uuid"" = ""policypublisher_statementlocation"".""section_id"") INNER JOIN ""policypublisher_section_version"" ON (""policypublisher_section"".""uuid"" = ""policypublisher_section_version"".""section_id"") WHERE (""policypublisher_statementlocation"".""statement_id"" = '0509afa269604028818dc1f53b8f93d2' AND ""policypublisher_section_version"".""version_id"" = 'be8225c85fab4a0ba8ea879a3d992abe') LIMIT 21; args=('0509afa269604028818dc1f53b8f93d2', 'be8225c85fab4a0ba8ea879a3d992abe'); alias=default

# Make Sure we got one
>>> sect
<Section: Rules for Interpreting Various Name Forms>

# Add a new section via the through field # NOTE NO INSERT!
>>> statement.section.add(sect, through_defaults={""sl_version"":new_version})
(0.000) BEGIN; args=None; alias=default
(0.002) SELECT ""policypublisher_statementlocation"".""section_id"" FROM ""policypublisher_statementlocation"" WHERE (""policypublisher_statementlocation"".""section_id"" IN ('1cc10c2e5bba4d238d5fabbfdfd72d83') AND ""policypublisher_statementlocation"".""statement_id"" = '0509afa269604028818dc1f53b8f93d2'); args=('1cc10c2e5bba4d238d5fabbfdfd72d83', '0509afa269604028818dc1f53b8f93d2'); alias=default

# No new object was created
>>> StatementLocation.objects.filter(sl_version=new_version).count()
(0.003) SELECT COUNT(*) AS ""__count"" FROM ""policypublisher_statementlocation"" WHERE ""policypublisher_statementlocation"".""sl_version_id"" = 'bcbb218ebdc5435b9e5caf879b55cd24'; args=('bcbb218ebdc5435b9e5caf879b55cd24',); alias=default
0
}}}

On the second to last statement, I believe Django is checking to see whether there is an existing M2M relationship, but it is not adding the version_id to the query, so it thinks the relationship already exists!

Note that I can manually create the relationship, so this is not a constraints problem:


{{{
>>> new_sl = StatementLocation(section=sect, statement=statement, sl_version=new_version)
>>> new_sl.save()
(0.027) INSERT INTO ""policypublisher_statementlocation"" (""section_id"", ""statement_id"", ""sl_version_id"") VALUES ('1cc10c2e5bba4d238d5fabbfdfd72d83', '0509afa269604028818dc1f53b8f93d2', 'bcbb218ebdc5435b9e5caf879b55cd24'); args=['1cc10c2e5bba4d238d5fabbfdfd72d83', '0509afa269604028818dc1f53b8f93d2', 'bcbb218ebdc5435b9e5caf879b55cd24']; alias=default
>>> StatementLocation.objects.filter(sl_version=new_version).count()
(0.001) SELECT COUNT(*) AS ""__count"" FROM ""policypublisher_statementlocation"" WHERE ""policypublisher_statementlocation"".""sl_version_id"" = 'bcbb218ebdc5435b9e5caf879b55cd24'; args=('bcbb218ebdc5435b9e5caf879b55cd24',); alias=default
1
>>> 
}}}


The FIX:

When a ManyToMany relationship is created with a ""through"" table, and through_defaults are provided, that should be part of the search query that django uses to determine if the relationship already exists.

e.g. 
SELECT ""policypublisher_statementlocation"".""section_id"" FROM ""policypublisher_statementlocation"" WHERE (""policypublisher_statementlocation"".""section_id"" IN ('1cc10c2e5bba4d238d5fabbfdfd72d83') AND ""policypublisher_statementlocation"".""statement_id"" = '0509afa269604028818dc1f53b8f93d2') AND ""policypublisher_statementlocation"".""sl_version_id"" = 'bcbb218ebdc5435b9e5caf879b55cd24';
"	Bug	closed	Database layer (models, ORM)	4.1	Normal	duplicate	ManyToManyField through		Unreviewed	0	0	0	0	0	0