Code

Opened 4 years ago

Last modified 16 months ago

#13774 new New feature

add optional rel_db_type() method for model field

Reported by: Suor Owned by: Suor
Component: Database layer (models, ORM) Version: 1.2
Severity: Normal Keywords: db, ForeignKey, related fields
Cc: sergeykolosov Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes 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 Suor 4 years ago.
rel_db_type_cleanup.diff (3.9 KB) - added by Suor 4 years ago.
rel_db_type for all fields, custom for AutoField, PositiveIntegerField and PositiveSmallIntegerField

Download all attachments as: .zip

Change History (10)

Changed 4 years ago by Suor

comment:1 Changed 4 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to wontfix
  • Status changed from new to closed

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 4 years ago by Suor

  • Resolution wontfix deleted
  • Status changed from closed to 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.

Changed 4 years ago by Suor

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

comment:3 Changed 4 years ago by russellm

  • Needs documentation set
  • Needs tests set
  • Triage Stage changed from Unreviewed to 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 Changed 3 years ago by wkornewald

  • Cc wkornewald@… added

comment:5 Changed 3 years ago by graham_king

  • Severity set to Normal
  • Type set to New feature

comment:6 Changed 3 years ago by wkornewald

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

comment:7 Changed 23 months ago by sergeykolosov

  • Cc sergeykolosov added

comment:8 Changed 16 months ago by aaugustin

  • Status changed from reopened to new

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from Suor to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.