Opened 15 years ago
Closed 10 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 , 15 years ago
| Attachment: | rel_db_type.diff added |
|---|
comment:1 by , 15 years ago
| Resolution: | → wontfix |
|---|---|
| Status: | new → closed |
comment:2 by , 15 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 , 15 years ago
| Attachment: | rel_db_type_cleanup.diff added |
|---|
rel_db_type for all fields, custom for AutoField, PositiveIntegerField and PositiveSmallIntegerField
comment:3 by , 15 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 , 15 years ago
| Cc: | added |
|---|
comment:5 by , 15 years ago
| Severity: | → Normal |
|---|---|
| Type: | → New feature |
comment:6 by , 14 years ago
| Cc: | removed |
|---|---|
| Easy pickings: | unset |
| UI/UX: | unset |
comment:7 by , 13 years ago
| Cc: | added |
|---|
comment:8 by , 13 years ago
| Status: | reopened → new |
|---|
comment:9 by , 10 years ago
| Needs documentation: | unset |
|---|---|
| Needs tests: | unset |
comment:10 by , 10 years ago
| Triage Stage: | Accepted → Unreviewed |
|---|
comment:11 by , 10 years ago
| Triage Stage: | Unreviewed → Ready for checkin |
|---|
comment:12 by , 10 years ago
| Triage Stage: | Ready for checkin → Accepted |
|---|
comment:13 by , 10 years ago
| Patch needs improvement: | set |
|---|
comment:14 by , 10 years ago
| Cc: | added |
|---|---|
| Patch needs improvement: | unset |
comment:15 by , 10 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.