Opened 7 years ago

Closed 3 months ago

#13161 closed Cleanup/optimization (wontfix)

Avoid global registration of psycopg2 adapters/extensions

Reported by: spoksss Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: psycopg2, unicode
Cc: Carl Meyer, Florian Apolloner Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hi,

I have some problem while I use Django connection based on psycopg2 in the same time while using some other database application.
Because Django changes default psycopg2 settings (psycopg2.extensions.register_type(psycopg2.extensions.UNICODE)),
I get "psycopg2.InterfaceError: can't decode into unicode string from SQLASCII", when I try use my external database application which works with SQLASCII after import django settings.

I think it isn't good way when Django changes default, global settings of psycopg2, because other application can have problems.
I have read documentation of psycopg2, and it is possible to set that psycopg2.extensions.UNICODE just for custom connection or cursor, so I think it is good idea to use that feature in Django.
http://initd.org/psycopg/docs/extensions.html#psycopg2.extensions.register_type

I have tested my changes with psycopg 2.0.8 and Django-1.2-beta-1 and everything work correctly.

Attachments (4)

base.py (8.1 KB) - added by spoksss 7 years ago.
Small usefull change.
patch-r12807.diff (1.6 KB) - added by spoksss 7 years ago.
diff file
project_with_test_ticket_13161.tar.gz (3.7 KB) - added by spoksss 6 years ago.
Small project with test to raise psycopg2 error.
base.py.diff13161 (1.0 KB) - added by Piotr Maślanka 4 years ago.
git patch fixing the problem

Download all attachments as: .zip

Change History (22)

Changed 7 years ago by spoksss

Attachment: base.py added

Small usefull change.

comment:1 Changed 7 years ago by Alex Gaynor

Changes should be uploaded as diffs from the root of the source tree.

Changed 7 years ago by spoksss

Attachment: patch-r12807.diff added

diff file

comment:2 Changed 7 years ago by Russell Keith-Magee

Component: UncategorizedDatabase layer (models, ORM)
Has patch: set
Needs tests: set
Triage Stage: UnreviewedAccepted

Strictly, this looks to be a duplicate of #5171, but I'll keep this open given that we have a patch that seems to address the implementation dead-end on that ticket.

This is going to need a test before it gets to trunk.

comment:3 Changed 6 years ago by spoksss

What could I do to test it?
This solution works correctly in my environment,
and it will be nice if I can help somehow and this patch will be add to trunk.

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

You write a test that does exactly the thing that causes the problem to occur -- that is, write a test that accesses psycopg directly as well as using Django's ORM, demonstrating that the way that Django's ORM sets the default value is problematic. The test should be integrated as part of Djanog's test suite -- the regressiontests.backend module would be an appropriate location; the test should fail using current trunk, and pass once your patch is applied.

comment:5 Changed 6 years ago by Carl Meyer

Cc: Carl Meyer added

Changed 6 years ago by spoksss

Small project with test to raise psycopg2 error.

comment:6 Changed 6 years ago by spoksss

Hi,

I created some project just to show what is a problem, I attached it in this comment.

Set settings, syncdb and then try to run below test:

python manage.py test myapp.MyModelTestCase

comment:7 Changed 6 years ago by Luke Plant

Type: Bug

comment:8 Changed 6 years ago by Luke Plant

Severity: Normal

comment:9 Changed 5 years ago by spoksss

Easy pickings: unset
UI/UX: unset

Hi,
what is status of this bug?
It is possible that the patch will be applied to trunk?

regards,

comment:10 Changed 4 years ago by anonymous

Version: 1.2-beta1.4

Now (version 1.4 of Django) there is only one supported backend (psycopg2) for postgresql, so it is possible, that this patch will be in trunk.
What else can be done to commit it to trunk?

comment:11 in reply to:  10 Changed 4 years ago by Aymeric Augustin

Replying to anonymous:

What else can be done to commit it to trunk?

The patch is still marked as needing tests.

comment:12 Changed 4 years ago by Piotr Maślanka

psycopg version "2.4.5 (dt dec pq3 ext)" fails to respect the per-connection settings, but respects the per-cursor setting just fine. Patch would be best served if setting command was moved to cursor creation (DatabaseWrapper._cursor()).

Changed 4 years ago by Piotr Maślanka

Attachment: base.py.diff13161 added

git patch fixing the problem

comment:13 Changed 4 years ago by Piotr Maślanka

Needs tests: unset

comment:14 Changed 4 years ago by Ramiro Morales

Needs tests: set

comment:15 Changed 4 years ago by Florian Apolloner

Cc: Florian Apolloner added

What about register_adapter in the most recent patch, isn't that "global" too? If yes we probable should fix all issues and not just the unicode issue.

comment:16 Changed 4 years ago by Anssi Kääriäinen

I guess the SafeString adapters aren't much of an issue as they don't apply to anything else than the SafeString types. But yeah, global they are.

I'd like to confirm that creating a cursor isn't slowed down too much by the change.

comment:17 Changed 16 months ago by Tim Graham

Summary: Ticked 2514 and 5171, solution for using Django with other psycopg2 applications.Avoid global registration of psycopg2 adapters/extensions
Type: BugCleanup/optimization
Version: 1.4master

comment:18 Changed 3 months ago by Tim Graham

Resolution: wontfix
Status: newclosed

The unicode type registration will be dropped with Python 2. There are some other extensions/types registered but I'm not sure they're causing any problems. Feel free to reopen if someone sees a reason to change things here.

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