Opened 18 years ago

Last modified 4 years ago

#3461 assigned Bug

DatabaseWrapper should pass through args and kwargs to underlying database adapter

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

Description (last modified by Asif Saifuddin Auvi)

Currently the DatabaseWrapper (at least for the postgresql) 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@…> 18 years ago.
pass through args and kwargs for calls to cursor()
3461-cursor-options.patch (2.7 KB ) - added by Collin Grady 16 years ago.

Download all attachments as: .zip

Change History (27)

by Jack Moffitt <metajack@…>, 18 years ago

Attachment: psycopg2.diff added

pass through args and kwargs for calls to cursor()

comment:1 by Marc Fargas <telenieko@…>, 18 years ago

Triage Stage: UnreviewedDesign decision needed

comment:2 by eliott@…, 17 years ago

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 by Jacob, 17 years ago

Needs documentation: set
Needs tests: set
Patch needs improvement: set
Triage Stage: Design decision neededAccepted

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 by Jack Moffitt <metajack@…>, 16 years ago

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.

by Collin Grady, 16 years ago

Attachment: 3461-cursor-options.patch added

comment:5 by Collin Grady, 16 years ago

Owner: changed from nobody to Collin Grady

comment:9 by Malcolm Tredinnick, 16 years ago

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 by Ramiro Morales, 16 years ago

Cc: Ramiro Morales added

comment:11 by Manel Clos, 14 years ago

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 by Łukasz Rekucki, 14 years ago

Severity: Normal
Type: Bug

comment:13 by Aymeric Augustin, 13 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:14 by Aymeric Augustin, 13 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:15 by Asif Saifuddin Auvi, 9 years ago

Owner: changed from Collin Grady to Asif Saifuddin Auvi
Status: newassigned

comment:16 by Asif Saifuddin Auvi, 9 years ago

Owner: Asif Saifuddin Auvi removed
Status: assignednew

comment:17 by Olivier Tabone, 8 years ago

Owner: set to Olivier Tabone
Status: newassigned

comment:18 by Olivier Tabone, 8 years ago

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

please comment on PR

comment:19 by Mariusz Felisiak, 8 years ago

Patch needs improvement: set

comment:20 by Asif Saifuddin Auvi, 8 years ago

Owner: changed from Olivier Tabone to Asif Saifuddin Auvi

comment:21 by Asif Saifuddin Auvi, 8 years ago

Needs documentation: set
Patch needs improvement: unset

I am unsure where to put the doc changes, so asked in the pr but did not get any response. after adding docs and release note I will squash commits

comment:22 by Tim Graham, 8 years ago

comment:9 contains a suggestion about the documentation. The content is more important; the location can easily be changed if needed.

comment:23 by Asif Saifuddin Auvi, 8 years ago

Needs documentation: unset

comment:24 by Tim Graham, 8 years ago

Patch needs improvement: set

comment:25 by Asif Saifuddin Auvi, 8 years ago

Needs documentation: set

comment:26 by Asif Saifuddin Auvi, 7 years ago

Patch needs improvement: unset

comment:27 by Asif Saifuddin Auvi, 7 years ago

Description: modified (diff)

comment:28 by Asif Saifuddin Auvi, 4 years ago

Patch needs improvement: set

needs retarget 3.2 and add missing docs :( hopefully, make this happen this time!

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