Skip to content

Commit

Permalink
Fixed django#373 -- Added CompositePrimaryKey.
Browse files Browse the repository at this point in the history
Thanks Lily Foote and Simon Charette for reviews and mentoring
this Google Summer of Code 2024 project.
  • Loading branch information
csirmazbendeguz authored and sarahboyce committed Nov 19, 2024
1 parent 512a2ba commit 46c77ed
Show file tree
Hide file tree
Showing 43 changed files with 3,081 additions and 29 deletions.
5 changes: 5 additions & 0 deletions django/contrib/admin/sites.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ def register(self, model_or_iterable, admin_class=None, **options):
"The model %s is abstract, so it cannot be registered with admin."
% model.__name__
)
if model._meta.is_composite_pk:
raise ImproperlyConfigured(
"The model %s has a composite primary key, so it cannot be "
"registered with admin." % model.__name__
)

if self.is_registered(model):
registered_admin = str(self.get_model_admin(model))
Expand Down
3 changes: 3 additions & 0 deletions django/core/serializers/python.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from django.apps import apps
from django.core.serializers import base
from django.db import DEFAULT_DB_ALIAS, models
from django.db.models import CompositePrimaryKey
from django.utils.encoding import is_protected_type


Expand Down Expand Up @@ -39,6 +40,8 @@ def get_dump_object(self, obj):
return data

def _value_from_field(self, obj, field):
if isinstance(field, CompositePrimaryKey):
return [self._value_from_field(obj, f) for f in field]
value = field.value_from_object(obj)
# Protected types (i.e., primitives like None, numbers, dates,
# and Decimals) are passed through as is. All other values are
Expand Down
12 changes: 12 additions & 0 deletions django/db/backends/base/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
)
from django.db.backends.utils import names_digest, split_identifier, truncate_name
from django.db.models import NOT_PROVIDED, Deferrable, Index
from django.db.models.fields.composite import CompositePrimaryKey
from django.db.models.sql import Query
from django.db.transaction import TransactionManagementError, atomic
from django.utils import timezone
Expand Down Expand Up @@ -106,6 +107,7 @@ class BaseDatabaseSchemaEditor:
sql_check_constraint = "CHECK (%(check)s)"
sql_delete_constraint = "ALTER TABLE %(table)s DROP CONSTRAINT %(name)s"
sql_constraint = "CONSTRAINT %(name)s %(constraint)s"
sql_pk_constraint = "PRIMARY KEY (%(columns)s)"

sql_create_check = "ALTER TABLE %(table)s ADD CONSTRAINT %(name)s CHECK (%(check)s)"
sql_delete_check = sql_delete_constraint
Expand Down Expand Up @@ -282,6 +284,11 @@ def table_sql(self, model):
constraint.constraint_sql(model, self)
for constraint in model._meta.constraints
)

pk = model._meta.pk
if isinstance(pk, CompositePrimaryKey):
constraint_sqls.append(self._pk_constraint_sql(pk.columns))

sql = self.sql_create_table % {
"table": self.quote_name(model._meta.db_table),
"definition": ", ".join(
Expand Down Expand Up @@ -1999,6 +2006,11 @@ def _constraint_names(
result.append(name)
return result

def _pk_constraint_sql(self, columns):
return self.sql_pk_constraint % {
"columns": ", ".join(self.quote_name(column) for column in columns)
}

def _delete_primary_key(self, model, strict=False):
constraint_names = self._constraint_names(model, primary_key=True)
if strict and len(constraint_names) != 1:
Expand Down
2 changes: 2 additions & 0 deletions django/db/backends/oracle/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,8 @@ def _field_should_be_indexed(self, model, field):
return create_index

def _is_identity_column(self, table_name, column_name):
if not column_name:
return False
with self.connection.cursor() as cursor:
cursor.execute(
"""
Expand Down
15 changes: 14 additions & 1 deletion django/db/backends/sqlite3/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from django.db.backends.base.schema import BaseDatabaseSchemaEditor
from django.db.backends.ddl_references import Statement
from django.db.backends.utils import strip_quotes
from django.db.models import NOT_PROVIDED, UniqueConstraint
from django.db.models import NOT_PROVIDED, CompositePrimaryKey, UniqueConstraint


class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
Expand Down Expand Up @@ -104,6 +104,13 @@ def is_self_referential(f):
f.name: f.clone() if is_self_referential(f) else f
for f in model._meta.local_concrete_fields
}

# Since CompositePrimaryKey is not a concrete field (column is None),
# it's not copied by default.
pk = model._meta.pk
if isinstance(pk, CompositePrimaryKey):
body[pk.name] = pk.clone()

# Since mapping might mix column names and default values,
# its values must be already quoted.
mapping = {
Expand Down Expand Up @@ -296,6 +303,12 @@ def add_field(self, model, field):
# Special-case implicit M2M tables.
if field.many_to_many and field.remote_field.through._meta.auto_created:
self.create_model(field.remote_field.through)
elif isinstance(field, CompositePrimaryKey):
# If a CompositePrimaryKey field was added, the existing primary key field
# had to be altered too, resulting in an AddField, AlterField migration.
# The table cannot be re-created on AddField, it would result in a
# duplicate primary key error.
return
elif (
# Primary keys and unique fields are not supported in ALTER TABLE
# ADD COLUMN.
Expand Down
2 changes: 2 additions & 0 deletions django/db/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
)
from django.db.models.fields import * # NOQA
from django.db.models.fields import __all__ as fields_all
from django.db.models.fields.composite import CompositePrimaryKey
from django.db.models.fields.files import FileField, ImageField
from django.db.models.fields.generated import GeneratedField
from django.db.models.fields.json import JSONField
Expand Down Expand Up @@ -82,6 +83,7 @@
"ProtectedError",
"RestrictedError",
"Case",
"CompositePrimaryKey",
"Exists",
"Expression",
"ExpressionList",
Expand Down
19 changes: 18 additions & 1 deletion django/db/models/aggregates.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
"""

from django.core.exceptions import FieldError, FullResultSet
from django.db.models.expressions import Case, Func, Star, Value, When
from django.db import NotSupportedError
from django.db.models.expressions import Case, ColPairs, Func, Star, Value, When
from django.db.models.fields import IntegerField
from django.db.models.functions.comparison import Coalesce
from django.db.models.functions.mixins import (
Expand Down Expand Up @@ -174,6 +175,22 @@ def __init__(self, expression, filter=None, **extra):
raise ValueError("Star cannot be used with filter. Please specify a field.")
super().__init__(expression, filter=filter, **extra)

def resolve_expression(self, *args, **kwargs):
result = super().resolve_expression(*args, **kwargs)
expr = result.source_expressions[0]

# In case of composite primary keys, count the first column.
if isinstance(expr, ColPairs):
if self.distinct:
raise NotSupportedError(
"COUNT(DISTINCT) doesn't support composite primary keys"
)

cols = expr.get_cols()
return Count(cols[0], filter=result.filter)

return result


class Max(Aggregate):
function = "MAX"
Expand Down
73 changes: 71 additions & 2 deletions django/db/models/base.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import copy
import inspect
import warnings
from collections import defaultdict
from functools import partialmethod
from itertools import chain

Expand Down Expand Up @@ -30,6 +31,7 @@
from django.db.models.constants import LOOKUP_SEP
from django.db.models.deletion import CASCADE, Collector
from django.db.models.expressions import DatabaseDefault
from django.db.models.fields.composite import CompositePrimaryKey
from django.db.models.fields.related import (
ForeignObjectRel,
OneToOneField,
Expand Down Expand Up @@ -508,7 +510,7 @@ def __init__(self, *args, **kwargs):
for field in fields_iter:
is_related_object = False
# Virtual field
if field.attname not in kwargs and field.column is None or field.generated:
if field.column is None or field.generated:
continue
if kwargs:
if isinstance(field.remote_field, ForeignObjectRel):
Expand Down Expand Up @@ -663,7 +665,11 @@ def _set_pk_val(self, value):
pk = property(_get_pk_val, _set_pk_val)

def _is_pk_set(self, meta=None):
return self._get_pk_val(meta) is not None
pk_val = self._get_pk_val(meta)
return not (
pk_val is None
or (isinstance(pk_val, tuple) and any(f is None for f in pk_val))
)

def get_deferred_fields(self):
"""
Expand Down Expand Up @@ -1454,6 +1460,11 @@ def _get_unique_checks(self, exclude=None, include_meta_constraints=False):
name = f.name
if name in exclude:
continue
if isinstance(f, CompositePrimaryKey):
names = tuple(field.name for field in f.fields)
if exclude.isdisjoint(names):
unique_checks.append((model_class, names))
continue
if f.unique:
unique_checks.append((model_class, (name,)))
if f.unique_for_date and f.unique_for_date not in exclude:
Expand Down Expand Up @@ -1728,6 +1739,7 @@ def check(cls, **kwargs):
*cls._check_constraints(databases),
*cls._check_default_pk(),
*cls._check_db_table_comment(databases),
*cls._check_composite_pk(),
]

return errors
Expand Down Expand Up @@ -1764,6 +1776,63 @@ def _check_default_pk(cls):
]
return []

@classmethod
def _check_composite_pk(cls):
errors = []
meta = cls._meta
pk = meta.pk

if not isinstance(pk, CompositePrimaryKey):
return errors

seen_columns = defaultdict(list)

for field_name in pk.field_names:
hint = None

try:
field = meta.get_field(field_name)
except FieldDoesNotExist:
field = None

if not field:
hint = f"{field_name!r} is not a valid field."
elif not field.column:
hint = f"{field_name!r} field has no column."
elif field.null:
hint = f"{field_name!r} field may not set 'null=True'."
elif field.generated:
hint = f"{field_name!r} field is a generated field."
else:
seen_columns[field.column].append(field_name)

if hint:
errors.append(
checks.Error(
f"{field_name!r} cannot be included in the composite primary "
"key.",
hint=hint,
obj=cls,
id="models.E042",
)
)

for column, field_names in seen_columns.items():
if len(field_names) > 1:
field_name, *rest = field_names
duplicates = ", ".join(repr(field) for field in rest)
errors.append(
checks.Error(
f"{duplicates} cannot be included in the composite primary "
"key.",
hint=f"{duplicates} and {field_name!r} are the same fields.",
obj=cls,
id="models.E042",
)
)

return errors

@classmethod
def _check_db_table_comment(cls, databases):
if not cls._meta.db_table_comment:
Expand Down
2 changes: 2 additions & 0 deletions django/db/models/fields/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,8 @@ def deconstruct(self):
path = path.replace("django.db.models.fields.json", "django.db.models")
elif path.startswith("django.db.models.fields.proxy"):
path = path.replace("django.db.models.fields.proxy", "django.db.models")
elif path.startswith("django.db.models.fields.composite"):
path = path.replace("django.db.models.fields.composite", "django.db.models")
elif path.startswith("django.db.models.fields"):
path = path.replace("django.db.models.fields", "django.db.models")
# Return basic info - other fields should override this.
Expand Down
Loading

0 comments on commit 46c77ed

Please sign in to comment.