Opened 7 years ago

Closed 6 years ago

Last modified 4 years ago

#6064 closed Uncategorized (fixed)

Allow database connection initialization commands

Reported by: jacob Owned by: floguy
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: telenieko@…, floguy@…, jshaffer, sam@…, feteanumarius@…, contact@…, greg@…, michael.greene@…, matthew.russell@…, hank.gay@…, dvanliere@…, dev@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX:

Description

There's sometimes a need to pass specific SQL that'll get executed at the beginning of every database connection (for just one example see #1051).

This could be in the form of a setting, a signal, or perhaps some other mechanism.

Attachments (4)

6064.diff (5.6 KB) - added by floguy 7 years ago.
Patch to add CONNECTION_INIT_SQL setting and use it on connection initialization.
6064-using-signals.diff (7.8 KB) - added by floguy 7 years ago.
Changed the implementation method to emit a signal instead of execute a tuple from the settings.
6064-using-signals-2.diff (6.1 KB) - added by mdh 6 years ago.
Updated patch to apply against trunk as of r9477.
6064-using-signals-3.diff (6.4 KB) - added by jbronn 6 years ago.

Download all attachments as: .zip

Change History (32)

comment:1 Changed 7 years ago by jacob

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 7 years ago by telenieko

  • Cc telenieko@… added

cc'ing me

comment:3 Changed 7 years ago by jacob

See also #3460.

comment:4 Changed 7 years ago by floguy

  • Cc floguy@… added
  • Has patch set
  • Needs tests set

Wrote a patch to fix this, with documentation. It should be fairly straighforward:

Added a CONNECTION_INIT_SQL setting, which is a tuple of strings to be executed on connection initialization.

For example,

CONNECTION_INIT_SQL = (
    'SET search_path TO django_application_schema',
)

would accomplish the same thing as ticket:1051

Ran test suite and there were no regressions, but tests for this new feature are needed.

Changed 7 years ago by floguy

Patch to add CONNECTION_INIT_SQL setting and use it on connection initialization.

comment:5 Changed 7 years ago by floguy

  • Owner changed from nobody to floguy
  • Status changed from new to assigned

comment:6 Changed 7 years ago by Simon G <dev@…>

  • Needs tests unset
  • Triage Stage changed from Accepted to Ready for checkin

comment:7 Changed 7 years ago by jacob

  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to Accepted

Looking at this, I *REALLY* don't like having SQL in the settings file. So I think that a connection-created signal is the right way to do this, not a setting.

comment:8 Changed 7 years ago by floguy

I have added a new patch which uses signals instead of a setting. I think you're right, too, that the signal method is better. It allows for more flexibility while not cluttering up the settings file. The only reason that I originally implemented the settings file originally was due to concerns about performance.

One thing: I wasn't sure where to put the new signal. It doesn't seem to fit where any of the other signals are defined, so I created django.db.backends.signals with this one signal in it.

Changed 7 years ago by floguy

Changed the implementation method to emit a signal instead of execute a tuple from the settings.

comment:9 Changed 7 years ago by anonymous

  • Needs tests unset
  • Patch needs improvement unset

comment:10 Changed 7 years ago by jshaffer

  • Cc jshaffer added

comment:11 Changed 7 years ago by anonymous

  • Cc sam@… added

comment:12 Changed 7 years ago by MariusBoo

  • Cc feteanumarius@… added

For anyone looking for a quick and dirty solution you ca define your table like this: db_table = '"django"."company"'. This will fool the quote function to think that your table is properly quoted. This also means that you have to specify the schema for each table manually.

comment:13 Changed 7 years ago by jfsimon_fr

  • Cc contact@… added

cc'ing me too (hello !)

comment:14 Changed 7 years ago by cogat

  • Cc greg@… added

comment:15 Changed 7 years ago by dan90

  • Cc dan90 added

Changed 6 years ago by mdh

Updated patch to apply against trunk as of r9477.

comment:16 Changed 6 years ago by mdh

I just updated this patch to work with the current trunk, since it hadn't been touched for a year and had gotten rather stale. I also fixed some bugs that were causing connection_created signals to be sent on every cursor-creation for some back ends, rather than only when a new database connection has really been made.

Unfortunately, fixing that bug broke the test for sqlite3, since in-memory sqlite3 databases (of the sort used by the tests) are never actually closed, and thus cannot be reopened by the tests. So I've had to disable the test altogether for sqlite3, unless someone can come up with a definitely-safe way to open a second temporary database somehow without clobbering the old one. Seems like more trouble than it's worth.

It occurs to me that a separate cursor_created signal might actually be useful, especially if it were passed a reference to the new cursor, but, well, one thing at a time.

comment:17 Changed 6 years ago by euphoria

  • Cc michael.greene@… added

comment:18 Changed 6 years ago by mattrussell

  • Cc matthew.russell@… added

cc'ing

comment:19 Changed 6 years ago by jbronn

  • milestone set to 1.1

Changed 6 years ago by jbronn

comment:20 Changed 6 years ago by jbronn

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [10182]) Fixed #6064 -- Added the connection_created signal for when a database connection is created.

comment:21 Changed 6 years ago by hank.gay@…

Does this address the case where I'd like to do some work with the connection? It seems like the sender should be the newly created connection, not the class.

comment:22 Changed 6 years ago by anonymous

  • Cc dan90 removed

comment:23 Changed 6 years ago by hank.gay@…

  • Cc hank.gay@… added
  • Resolution fixed deleted
  • Status changed from closed to reopened

I am reopening this ticket because the accepted patch does not appear to address the same issue as the original patch. The original patch performed initialization work on each connection as it was established. The accepted patch fires a signal when a connection has been created, but the sender is the class, not the newly created connection, so how can the desired initialization be performed? If there is a consensus that the sender should be changed to the new connection, I am happy to submit a patch that does that.

comment:24 Changed 6 years ago by euphoria

  • Resolution set to fixed
  • Status changed from reopened to closed

Please ask questions like this on an appropriate mailing list (django-user in this case) instead of resurrecting old tickets.

def set_schema(sender, **kwargs):
    from django.db import connection

    cursor = connection.cursor()
    cursor.execute("INITIALIZE SOME THINGS VIA SQL")

if django.VERSION >= (1,1):
    from django.db.backends.signals import connection_created
    connection_created.connect(set_schema)

comment:25 Changed 5 years ago by tback

i never found out where at what place i had to register the signal handler.

the best solution i found for the schema problem was to set a connect_query in pgbouncer.

comment:26 Changed 5 years ago by drdee

  • Cc dvanliere@… added

comment:27 Changed 4 years ago by brillgen

  • Cc dev@… added
  • Easy pickings unset
  • Severity set to Normal
  • Type set to Uncategorized

comment:28 Changed 4 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

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