Opened 8 years ago

Last modified 4 years ago

#3461 new Bug

DatabaseWrapper should pass through args and kwargs to underlying database adapter

Reported by: Jack Moffitt <metajack@…> Owned by: cgrady
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: metajack@…, ramiro, manel.clos@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Currently the DatabaseWrapper (at least for the postgresql_psycopg2end) does not pass args and kwargs for cursor() calls to the underlying database adapter. This makes it impossible to use the adapter fully at the low level. For example, to use dict cursors in psycopg2 you have to pass a different cursor factory via the cursor_factory keyword argument to cursor(). The attached patch passes through args and kwargs for cursor() calls.

Attachments (2)

psycopg2.diff (1016 bytes) - added by Jack Moffitt <metajack@…> 8 years ago.
pass through args and kwargs for calls to cursor()
3461-cursor-options.patch (2.7 KB) - added by cgrady 7 years ago.

Download all attachments as: .zip

Change History (16)

Changed 8 years ago by Jack Moffitt <metajack@…>

pass through args and kwargs for calls to cursor()

comment:1 Changed 8 years ago by Marc Fargas <telenieko@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

comment:2 Changed 8 years ago by eliott@…

Any update on this?
Would be very nice to not have to patch my working copy of the django libs, and so I can be more sure it will work on other installs.

comment:3 Changed 8 years ago by jacob

  • Needs documentation set
  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Design decision needed to Accepted

A good idea -- orthogonal to DATABASE_OPTIONS added some time back -- but this needs to be done in a cross-database way, not just for one backend.

comment:4 Changed 7 years ago by Jack Moffitt <metajack@…>

  • Cc metajack@… added

Sure, a cross database way would just be to apply the same style patch to other backends, no? Why not file a seperate bug for that project to replicate this patch on the other backends, instead of holding this one up until a perfect solution arrives.

Changed 7 years ago by cgrady

comment:5 Changed 7 years ago by cgrady

  • Owner changed from nobody to cgrady

comment:6 Changed 7 years ago by mtredinnick

  • milestone set to 1.0 maybe

comment:7 Changed 7 years ago by jacob

  • milestone changed from 1.0 maybe to post-1.0

comment:8 Changed 6 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:9 Changed 6 years ago by mtredinnick

The current patch still needs documentation (topics/db/sql.txt would seem to be a reasonable location) and doesn't patch the SQLite or Oracle backends. Also, it should be possible to test this, at least in a backend-specific fashion (checking settings.DATABASE_ENGINE in the test). I'd like to see a test for at least one backend.

Whoever fixes it should have a "reasonable attempt" at the Oracle backend -- I realise testing that can be tricky, but we can get confirmation of correctness from other people before applying.

comment:10 Changed 6 years ago by ramiro

  • Cc ramiro added

comment:11 Changed 4 years ago by manelclos

  • Cc manel.clos@… added

Checked with Django 1.2.3, works ok. Just one addition to the patch:

db/backends/__init__.py:73

    def cursor(self, *args, **kwargs):
        from django.conf import settings
        cursor = self._cursor(*args, **kwargs)

comment:12 Changed 4 years ago by lrekucki

  • Severity set to Normal
  • Type set to Bug

comment:13 Changed 3 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:14 Changed 3 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

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