From cf6a4ec062c1dcde441c8253af70c59f4c5a5dd6 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 5 Jun 2011 15:36:02 +0100 Subject: [PATCH 1/2] Added pqpath functions to get/set the value for GUC parameters The aim of these function is to allow the connection to make a better use of the pqpath functions instead of using PQexec for these small things. Also, the functions are to be called with the connection lock: this makes composing higher level functions using them easier. --- psycopg/pqpath.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++ psycopg/pqpath.h | 6 ++++ 2 files changed, 98 insertions(+) diff --git a/psycopg/pqpath.c b/psycopg/pqpath.c index f50b00b4..fb0736a5 100644 --- a/psycopg/pqpath.c +++ b/psycopg/pqpath.c @@ -597,6 +597,98 @@ pq_reset(connectionObject *conn) } +/* Get a session parameter. + * + * The function should be called on a locked connection without + * holding the GIL. + * + * The result is a new string allocated with malloc. + */ + +char * +pq_get_guc_locked( + connectionObject *conn, const char *param, + PGresult **pgres, char **error, PyThreadState **tstate) +{ + char query[256]; + int size; + char *rv = NULL; + + Dprintf("pq_get_guc_locked: reading %s", param); + + size = PyOS_snprintf(query, sizeof(query), "SHOW %s;", param); + if (size >= sizeof(query)) { + *error = strdup("SHOW: query too large"); + goto cleanup; + } + + Dprintf("pq_get_guc_locked: pgconn = %p, query = %s", conn->pgconn, query); + + *error = NULL; + if (!psyco_green()) { + *pgres = PQexec(conn->pgconn, query); + } else { + PyEval_RestoreThread(*tstate); + *pgres = psyco_exec_green(conn, query); + *tstate = PyEval_SaveThread(); + } + + if (*pgres == NULL) { + Dprintf("pq_get_guc_locked: PQexec returned NULL"); + if (!PyErr_Occurred()) { + const char *msg; + msg = PQerrorMessage(conn->pgconn); + if (msg && *msg) { *error = strdup(msg); } + } + goto cleanup; + } + if (PQresultStatus(*pgres) != PGRES_TUPLES_OK) { + Dprintf("pq_get_guc_locked: result was not TUPLES_OK (%d)", + PQresultStatus(*pgres)); + goto cleanup; + } + + rv = strdup(PQgetvalue(*pgres, 0, 0)); + CLEARPGRES(*pgres); + +cleanup: + return rv; +} + +/* Set a session parameter. + * + * The function should be called on a locked connection without + * holding the GIL + */ + +int +pq_set_guc_locked( + connectionObject *conn, const char *param, const char *value, + PGresult **pgres, char **error, PyThreadState **tstate) +{ + char query[256]; + int size; + int rv = -1; + + Dprintf("pq_set_guc_locked: setting %s to %s", param, value); + + if (0 == strcmp(value, "default")) { + size = PyOS_snprintf(query, sizeof(query), + "SET %s TO DEFAULT;", param); + } + else { + size = PyOS_snprintf(query, sizeof(query), + "SET %s TO '%s';", param, value); + } + if (size >= sizeof(query)) { + *error = strdup("SET: query too large"); + } + + rv = pq_execute_command_locked(conn, query, pgres, error, tstate); + + return rv; +} + /* Call one of the PostgreSQL tpc-related commands. * * This function should only be called on a locked connection without diff --git a/psycopg/pqpath.h b/psycopg/pqpath.h index 080047c0..bf012ade 100644 --- a/psycopg/pqpath.h +++ b/psycopg/pqpath.h @@ -47,6 +47,12 @@ HIDDEN int pq_abort(connectionObject *conn); HIDDEN int pq_reset_locked(connectionObject *conn, PGresult **pgres, char **error, PyThreadState **tstate); HIDDEN int pq_reset(connectionObject *conn); +HIDDEN char *pq_get_guc_locked(connectionObject *conn, const char *param, + PGresult **pgres, + char **error, PyThreadState **tstate); +HIDDEN int pq_set_guc_locked(connectionObject *conn, const char *param, + const char *value, PGresult **pgres, + char **error, PyThreadState **tstate); HIDDEN int pq_tpc_command_locked(connectionObject *conn, const char *cmd, const char *tid, PGresult **pgres, char **error, From 869d48b6f0192ceb781e9b8a59588552df6b2500 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 5 Jun 2011 16:26:01 +0100 Subject: [PATCH 2/2] Use the pqpath functions to get/set GUC parameters Functions conn_setup(), conn_get_isolation_level(), conn_set_transaction(), conn_switch_isolation_level(), conn_set_client_encoding() reimplemented using the pqpath funtitons. Dropped analogous function in the connection, as it had to take the lock, thus it was hard to build consistent pieces of functionality with it. --- psycopg/connection.h | 5 +- psycopg/connection_int.c | 170 +++++++++++++++++++++----------------- psycopg/connection_type.c | 43 ++++------ 3 files changed, 116 insertions(+), 102 deletions(-) diff --git a/psycopg/connection.h b/psycopg/connection.h index d79392bc..30ac5f42 100644 --- a/psycopg/connection.h +++ b/psycopg/connection.h @@ -59,7 +59,6 @@ extern "C" { later change it, she must know what she's doing... these are the queries we need to issue */ #define psyco_datestyle "SET DATESTYLE TO 'ISO'" -#define psyco_transaction_isolation "SHOW default_transaction_isolation" extern HIDDEN PyTypeObject connectionType; @@ -134,7 +133,9 @@ HIDDEN int conn_connect(connectionObject *self, long int async); HIDDEN void conn_close(connectionObject *self); HIDDEN int conn_commit(connectionObject *self); HIDDEN int conn_rollback(connectionObject *self); -HIDDEN int conn_set(connectionObject *self, const char *param, const char *value); +HIDDEN int conn_set_transaction(connectionObject *self, const char *isolevel, + const char *readonly, const char *deferrable, + int autocommit); HIDDEN int conn_set_autocommit(connectionObject *self, int value); HIDDEN int conn_switch_isolation_level(connectionObject *self, int level); HIDDEN int conn_set_client_encoding(connectionObject *self, const char *enc); diff --git a/psycopg/connection_int.c b/psycopg/connection_int.c index 24d424df..4170b32f 100644 --- a/psycopg/connection_int.c +++ b/psycopg/connection_int.c @@ -364,7 +364,8 @@ exit: int conn_get_isolation_level(connectionObject *self) { - PGresult *pgres; + PGresult *pgres = NULL; + char *error = NULL; int rv = -1; char *lname; const IsolationLevel *level; @@ -376,24 +377,13 @@ conn_get_isolation_level(connectionObject *self) Py_BEGIN_ALLOW_THREADS; pthread_mutex_lock(&self->lock); - Py_BLOCK_THREADS; - if (!psyco_green()) { - Py_UNBLOCK_THREADS; - pgres = PQexec(self->pgconn, psyco_transaction_isolation); - Py_BLOCK_THREADS; - } else { - pgres = psyco_exec_green(self, psyco_transaction_isolation); - } - - if (pgres == NULL || PQresultStatus(pgres) != PGRES_TUPLES_OK) { - PyErr_SetString(OperationalError, - "can't fetch default_transaction_isolation"); + if (!(lname = pq_get_guc_locked(self, "default_transaction_isolation", + &pgres, &error, &_save))) { goto endlock; } /* find the value for the requested isolation level */ - lname = PQgetvalue(pgres, 0, 0); level = conn_isolevels; while ((++level)->name) { if (0 == strcasecmp(level->name, lname)) { @@ -401,23 +391,26 @@ conn_get_isolation_level(connectionObject *self) break; } } - if (-1 == rv) { - char msg[256]; - snprintf(msg, sizeof(msg), "unexpected isolation level: '%s'", lname); - PyErr_SetString(OperationalError, msg); + error = malloc(256); + PyOS_snprintf(error, 256, + "unexpected isolation level: '%s'", lname); } -endlock: - IFCLEARPGRES(pgres); + free(lname); - Py_UNBLOCK_THREADS; +endlock: pthread_mutex_unlock(&self->lock); Py_END_ALLOW_THREADS; + if (rv < 0) { + pq_complete_error(self, &pgres, &error); + } + return rv; } + int conn_get_protocol_version(PGconn *pgconn) { @@ -465,8 +458,8 @@ conn_is_datestyle_ok(PGconn *pgconn) int conn_setup(connectionObject *self, PGconn *pgconn) { - PGresult *pgres; - int green; + PGresult *pgres = NULL; + char *error = NULL; self->equote = conn_get_standard_conforming_strings(pgconn); self->server_version = conn_get_server_version(pgconn); @@ -490,31 +483,20 @@ conn_setup(connectionObject *self, PGconn *pgconn) pthread_mutex_lock(&self->lock); Py_BLOCK_THREADS; - green = psyco_green(); - - if (green && (pq_set_non_blocking(self, 1, 1) != 0)) { + if (psyco_green() && (pq_set_non_blocking(self, 1, 1) != 0)) { return -1; } if (!conn_is_datestyle_ok(self->pgconn)) { - if (!green) { - Py_UNBLOCK_THREADS; - Dprintf("conn_connect: exec query \"%s\";", psyco_datestyle); - pgres = PQexec(pgconn, psyco_datestyle); - Py_BLOCK_THREADS; - } else { - pgres = psyco_exec_green(self, psyco_datestyle); - } - - if (pgres == NULL || PQresultStatus(pgres) != PGRES_COMMAND_OK ) { - PyErr_SetString(OperationalError, "can't set datestyle to ISO"); - IFCLEARPGRES(pgres); - Py_UNBLOCK_THREADS; - pthread_mutex_unlock(&self->lock); - Py_BLOCK_THREADS; + int res; + Py_UNBLOCK_THREADS; + res = pq_set_guc_locked(self, "datestyle", "ISO", + &pgres, &error, &_save); + Py_BLOCK_THREADS; + if (res < 0) { + pq_complete_error(self, &pgres, &error); return -1; } - CLEARPGRES(pgres); } /* for reset */ @@ -976,30 +958,53 @@ conn_rollback(connectionObject *self) return res; } -/* conn_set - set a guc parameter */ - int -conn_set(connectionObject *self, const char *param, const char *value) +conn_set_transaction(connectionObject *self, + const char *isolevel, const char *readonly, const char *deferrable, + int autocommit) { - char query[256]; PGresult *pgres = NULL; char *error = NULL; - int res = 1; - - Dprintf("conn_set: setting %s to %s", param, value); + int res = -1; Py_BEGIN_ALLOW_THREADS; pthread_mutex_lock(&self->lock); - if (0 == strcmp(value, "default")) { - sprintf(query, "SET %s TO DEFAULT;", param); - } - else { - sprintf(query, "SET %s TO '%s';", param, value); + if (isolevel) { + Dprintf("conn_set_transaction: setting isolation to %s", isolevel); + if ((res = pq_set_guc_locked(self, + "default_transaction_isolation", isolevel, + &pgres, &error, &_save))) { + goto endlock; + } } - res = pq_execute_command_locked(self, query, &pgres, &error, &_save); + if (readonly) { + Dprintf("conn_set_transaction: setting read only to %s", readonly); + if ((res = pq_set_guc_locked(self, + "default_transaction_read_only", readonly, + &pgres, &error, &_save))) { + goto endlock; + } + } + if (deferrable) { + Dprintf("conn_set_transaction: setting deferrable to %s", deferrable); + if ((res = pq_set_guc_locked(self, + "default_transaction_deferrable", deferrable, + &pgres, &error, &_save))) { + goto endlock; + } + } + + if (self->autocommit != autocommit) { + Dprintf("conn_set_transaction: setting autocommit to %d", autocommit); + self->autocommit = autocommit; + } + + res = 0; + +endlock: pthread_mutex_unlock(&self->lock); Py_END_ALLOW_THREADS; @@ -1029,7 +1034,10 @@ conn_set_autocommit(connectionObject *self, int value) int conn_switch_isolation_level(connectionObject *self, int level) { + PGresult *pgres = NULL; + char *error = NULL; int curr_level; + int ret = -1; /* use only supported levels on older PG versions */ if (self->server_version < 80000) { @@ -1050,16 +1058,21 @@ conn_switch_isolation_level(connectionObject *self, int level) /* Emulate the previous semantic of set_isolation_level() using the * functions currently available. */ + Py_BEGIN_ALLOW_THREADS; + pthread_mutex_lock(&self->lock); + /* terminate the current transaction if any */ - pq_abort(self); + if ((ret = pq_abort_locked(self, &pgres, &error, &_save))) { + goto endlock; + } if (level == 0) { - if (0 != conn_set(self, "default_transaction_isolation", "default")) { - return -1; - } - if (0 != conn_set_autocommit(self, 1)) { - return -1; + if ((ret = pq_set_guc_locked(self, + "default_transaction_isolation", "default", + &pgres, &error, &_save))) { + goto endlock; } + self->autocommit = 1; } else { /* find the name of the requested level */ @@ -1070,22 +1083,33 @@ conn_switch_isolation_level(connectionObject *self, int level) } } if (!isolevel->name) { - PyErr_SetString(OperationalError, "bad isolation level value"); - return -1; + ret = -1; + error = strdup("bad isolation level value"); + goto endlock; } - if (0 != conn_set(self, "default_transaction_isolation", isolevel->name)) { - return -1; - } - if (0 != conn_set_autocommit(self, 0)) { - return -1; + if ((ret = pq_set_guc_locked(self, + "default_transaction_isolation", isolevel->name, + &pgres, &error, &_save))) { + goto endlock; } + self->autocommit = 0; } Dprintf("conn_switch_isolation_level: switched to level %d", level); - return 0; + +endlock: + pthread_mutex_unlock(&self->lock); + Py_END_ALLOW_THREADS; + + if (ret < 0) { + pq_complete_error(self, &pgres, &error); + } + + return ret; } + /* conn_set_client_encoding - switch client encoding on connection */ int @@ -1093,7 +1117,6 @@ conn_set_client_encoding(connectionObject *self, const char *enc) { PGresult *pgres = NULL; char *error = NULL; - char query[48]; int res = 1; char *codec = NULL; char *clean_enc = NULL; @@ -1109,16 +1132,14 @@ conn_set_client_encoding(connectionObject *self, const char *enc) Py_BEGIN_ALLOW_THREADS; pthread_mutex_lock(&self->lock); - /* set encoding, no encoding string is longer than 24 bytes */ - PyOS_snprintf(query, 47, "SET client_encoding = '%s'", clean_enc); - /* abort the current transaction, to set the encoding ouside of transactions */ if ((res = pq_abort_locked(self, &pgres, &error, &_save))) { goto endlock; } - if ((res = pq_execute_command_locked(self, query, &pgres, &error, &_save))) { + if ((res = pq_set_guc_locked(self, "client_encoding", clean_enc, + &pgres, &error, &_save))) { goto endlock; } @@ -1142,7 +1163,6 @@ conn_set_client_encoding(connectionObject *self, const char *enc) self->encoding, self->codec); endlock: - pthread_mutex_unlock(&self->lock); Py_END_ALLOW_THREADS; diff --git a/psycopg/connection_type.c b/psycopg/connection_type.c index 3a5b7feb..f3a17f93 100644 --- a/psycopg/connection_type.c +++ b/psycopg/connection_type.c @@ -465,11 +465,16 @@ _psyco_conn_parse_onoff(PyObject *pyval) static PyObject * psyco_conn_set_transaction(connectionObject *self, PyObject *args, PyObject *kwargs) { - PyObject *isolation_level = Py_None; + PyObject *isolevel = Py_None; PyObject *readonly = Py_None; PyObject *deferrable = Py_None; PyObject *autocommit = Py_None; + const char *c_isolevel = NULL; + const char *c_readonly = NULL; + const char *c_deferrable = NULL; + int c_autocommit = self->autocommit; + static char *kwlist[] = {"isolation_level", "readonly", "deferrable", "autocommit", NULL}; @@ -478,46 +483,34 @@ psyco_conn_set_transaction(connectionObject *self, PyObject *args, PyObject *kwa EXC_IF_IN_TRANSACTION(self, set_transaction); if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|OOOO", kwlist, - &isolation_level, &readonly, &deferrable, &autocommit)) { + &isolevel, &readonly, &deferrable, &autocommit)) { return NULL; } - if (Py_None != isolation_level) { - const char *value = NULL; - if (!(value = _psyco_conn_parse_isolevel(self, isolation_level))) { - return NULL; - } - if (0 != conn_set(self, "default_transaction_isolation", value)) { + if (Py_None != isolevel) { + if (!(c_isolevel = _psyco_conn_parse_isolevel(self, isolevel))) { return NULL; } } if (Py_None != readonly) { - const char *value = NULL; - if (!(value = _psyco_conn_parse_onoff(readonly))) { - return NULL; - } - if (0 != conn_set(self, "default_transaction_read_only", value)) { + if (!(c_readonly = _psyco_conn_parse_onoff(readonly))) { return NULL; } } - if (Py_None != deferrable) { - const char *value = NULL; - if (!(value = _psyco_conn_parse_onoff(deferrable))) { - return NULL; - } - if (0 != conn_set(self, "default_transaction_deferrable", value)) { + if (!(c_deferrable = _psyco_conn_parse_onoff(deferrable))) { return NULL; } } - if (Py_None != autocommit) { - int value = PyObject_IsTrue(autocommit); - if (-1 == value) { return NULL; } - if (0 != conn_set_autocommit(self, value)) { - return NULL; - } + c_autocommit = PyObject_IsTrue(autocommit); + if (-1 == c_autocommit) { return NULL; } + } + + if (0 != conn_set_transaction(self, + c_isolevel, c_readonly, c_deferrable, c_autocommit)) { + return NULL; } Py_INCREF(Py_None);