Opened 6 years ago

Closed 12 months 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)

rel_db_type.diff (899 bytes) - added by Alexander Schepanovski 6 years ago.
rel_db_type_cleanup.diff (3.9 KB) - added by Alexander Schepanovski 6 years ago.
rel_db_type for all fields, custom for AutoField, PositiveIntegerField and PositiveSmallIntegerField

Download all attachments as: .zip

Change History (18)

Changed 6 years ago by Alexander Schepanovski

Attachment: rel_db_type.diff added

comment:1 Changed 6 years ago by Russell Keith-Magee

Resolution: wontfix
Status: newclosed

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.

comment:2 Changed 6 years ago by Alexander Schepanovski

Resolution: wontfix
Status: closedreopened

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.

Changed 6 years ago by Alexander Schepanovski

Attachment: rel_db_type_cleanup.diff added

rel_db_type for all fields, custom for AutoField, PositiveIntegerField and PositiveSmallIntegerField

comment:3 Changed 6 years ago by Russell Keith-Magee

Needs documentation: set
Needs tests: set
Triage Stage: UnreviewedAccepted

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 Changed 6 years ago by Waldemar Kornewald

Cc: wkornewald@… added

comment:5 Changed 6 years ago by Graham King

Severity: Normal
Type: New feature

comment:6 Changed 5 years ago by Waldemar Kornewald

Cc: wkornewald@… removed
Easy pickings: unset
UI/UX: unset

comment:7 Changed 4 years ago by Sergey Kolosov

Cc: Sergey Kolosov added

comment:8 Changed 4 years ago by Aymeric Augustin

Status: reopenednew

comment:9 Changed 15 months ago by Alexander

Needs documentation: unset
Needs tests: unset

comment:10 Changed 15 months ago by Alexander

Triage Stage: AcceptedUnreviewed

comment:11 Changed 15 months ago by Alexander

Triage Stage: UnreviewedReady for checkin

comment:12 Changed 15 months ago by Claude Paroz

Triage Stage: Ready for checkinAccepted

comment:13 Changed 15 months ago by Tim Graham

Patch needs improvement: set

comment:14 Changed 14 months ago by Alexander

Cc: alecs.box@… added
Patch needs improvement: unset

comment:15 Changed 14 months ago by Tim Graham

Needs documentation: set
Summary: add optional rel_db_type() method for model fieldAdd model Field.rel_db_type() method

Docs are still missing in the PR.

comment:16 Changed 12 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In b61eab18:

Fixed #13774 -- Added models.Field.rel_db_type().

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