Opened 14 years ago
Closed 9 years ago
#13774 closed New feature (fixed)
Add model Field.rel_db_type() method
Reported by: | Alexander Schepanovski | Owned by: | Alexander Schepanovski |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.2 |
Severity: | Normal | Keywords: | db, ForeignKey, related fields |
Cc: | Sergey Kolosov, alecs.box@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | yes |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
rel_db_type() method will return db_type for ForeignKey / OneToOneField to a field,
if not exists value of db_type() method should be used as it's done now.
It will remove a need for a hack in ForeignKey.db_type and make possible subclasses of AutoField work such as:
class BigAutoField(models.AutoField): def __init__(self, verbose_name=None, name=None, external_sequence=False, **kwargs): self.external_sequence = external_sequence models.AutoField.__init__(self, verbose_name, name, **kwargs) def db_type(self, connection): # now only for PostgreSQL, should be done more carefully to support other backends return 'bigint' if self.external_sequence else 'bigserial' def rel_db_type(self, connection): return 'bigint'
Attachments (2)
Change History (18)
by , 14 years ago
Attachment: | rel_db_type.diff added |
---|
comment:1 by , 14 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
comment:2 by , 14 years ago
Resolution: | wontfix |
---|---|
Status: | closed → reopened |
Patch is needed not to avoid a hack, but to make possible custom AutoField subclasses as in example.
I'll provide whole rel_db_type() cleanup patch to follow Yoda.
And no, it's not a precursor for NoSQL.
by , 14 years ago
Attachment: | rel_db_type_cleanup.diff added |
---|
rel_db_type for all fields, custom for AutoField, PositiveIntegerField and PositiveSmallIntegerField
comment:3 by , 14 years ago
Needs documentation: | set |
---|---|
Needs tests: | set |
Triage Stage: | Unreviewed → Accepted |
Ok - this makes a little more sense now. It still needs tests and documentation, however.
I also have a sneaking suspicion that this may be linked with #11319.
comment:4 by , 14 years ago
Cc: | added |
---|
comment:5 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → New feature |
comment:6 by , 13 years ago
Cc: | removed |
---|---|
Easy pickings: | unset |
UI/UX: | unset |
comment:7 by , 12 years ago
Cc: | added |
---|
comment:8 by , 12 years ago
Status: | reopened → new |
---|
comment:9 by , 9 years ago
Needs documentation: | unset |
---|---|
Needs tests: | unset |
comment:10 by , 9 years ago
Triage Stage: | Accepted → Unreviewed |
---|
comment:11 by , 9 years ago
Triage Stage: | Unreviewed → Ready for checkin |
---|
comment:12 by , 9 years ago
Triage Stage: | Ready for checkin → Accepted |
---|
comment:13 by , 9 years ago
Patch needs improvement: | set |
---|
comment:14 by , 9 years ago
Cc: | added |
---|---|
Patch needs improvement: | unset |
comment:15 by , 9 years ago
Needs documentation: | set |
---|---|
Summary: | add optional rel_db_type() method for model field → Add model Field.rel_db_type() method |
Docs are still missing in the PR.
I'm afraid I don't see why this is required. The explanation in ticket says it's to avoid a hack, but doesn't describe what that hack is required to solve; the patch doesn't contain any test cases to demonstrate why the feature is required (or that it even works as suggested). I'm also not especially inspired by a patch for a new feature that contains a TODO marker. As Yoda would advise: Do or do not... there is no TODO :-)
If this is a precursor to the NoSQL work, then it should be folded into that work; we're not going to accept and modify code that pre-empts the solution introduced by a SoC project.