From ff8158d7c023bc7a49894e07f9c66b4dc2d0bd69 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 19 Oct 2011 20:31:09 +0100 Subject: [PATCH 1/4] Simplification in the COPY command composition Dropped the branch if NULL is specified or not: just use the default \N. Also fixed copy_from/copy_to docstrings. --- psycopg/cursor_type.c | 98 +++++++++++++++---------------------------- 1 file changed, 34 insertions(+), 64 deletions(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 52baa6dc..d644df52 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1241,7 +1241,7 @@ exit: /* extension: copy_from - implements COPY FROM */ #define psyco_curs_copy_from_doc \ -"copy_from(file, table, sep='\\t', null='\\N', size=8192, columns=None) -- Copy table from file." +"copy_from(file, table, sep='\\t', null='\\\\N', size=8192, columns=None) -- Copy table from file." static int _psyco_curs_has_read_check(PyObject* o, void* var) @@ -1271,7 +1271,8 @@ psyco_curs_copy_from(cursorObject *self, PyObject *args, PyObject *kwargs) char query_buffer[DEFAULT_COPYBUFF]; Py_ssize_t query_size; const char *table_name; - const char *sep = "\t", *null = NULL; + const char *sep = "\t"; + const char *null = "\\N"; Py_ssize_t bufsize = DEFAULT_COPYBUFF; PyObject *file, *columns = NULL, *res = NULL; char *columnlist = NULL; @@ -1303,41 +1304,25 @@ psyco_curs_copy_from(cursorObject *self, PyObject *args, PyObject *kwargs) goto exit; } + if (!(quoted_null = psycopg_escape_string( + (PyObject*)self->conn, null, 0, NULL, NULL))) { + PyErr_NoMemory(); + goto exit; + } + query = query_buffer; - if (null) { - if (!(quoted_null = psycopg_escape_string( - (PyObject*)self->conn, null, 0, NULL, NULL))) { + query_size = PyOS_snprintf(query, DEFAULT_COPYBUFF, + "COPY %s%s FROM stdin WITH DELIMITER AS %s NULL AS %s", + table_name, columnlist, quoted_delimiter, quoted_null); + if (query_size >= DEFAULT_COPYBUFF) { + /* Got truncated, allocate dynamically */ + if (!(query = PyMem_New(char, query_size + 1))) { PyErr_NoMemory(); goto exit; } - query_size = PyOS_snprintf(query, DEFAULT_COPYBUFF, + PyOS_snprintf(query, query_size + 1, "COPY %s%s FROM stdin WITH DELIMITER AS %s NULL AS %s", table_name, columnlist, quoted_delimiter, quoted_null); - if (query_size >= DEFAULT_COPYBUFF) { - /* Got truncated, allocate dynamically */ - if (!(query = PyMem_New(char, query_size + 1))) { - PyErr_NoMemory(); - goto exit; - } - PyOS_snprintf(query, query_size + 1, - "COPY %s%s FROM stdin WITH DELIMITER AS %s NULL AS %s", - table_name, columnlist, quoted_delimiter, quoted_null); - } - } - else { - query_size = PyOS_snprintf(query, DEFAULT_COPYBUFF, - "COPY %s%s FROM stdin WITH DELIMITER AS %s", - table_name, columnlist, quoted_delimiter); - if (query_size >= DEFAULT_COPYBUFF) { - /* Got truncated, allocate dynamically */ - if (!(query = PyMem_New(char, query_size + 1))) { - PyErr_NoMemory(); - goto exit; - } - PyOS_snprintf(query, query_size + 1, - "COPY %s%s FROM stdin WITH DELIMITER AS %s", - table_name, columnlist, quoted_delimiter); - } } Dprintf("psyco_curs_copy_from: query = %s", query); @@ -1366,7 +1351,7 @@ exit: /* extension: copy_to - implements COPY TO */ #define psyco_curs_copy_to_doc \ -"copy_to(file, table, sep='\\t', null='\\N', columns=None) -- Copy table to file." +"copy_to(file, table, sep='\\t', null='\\\\N', columns=None) -- Copy table to file." static int _psyco_curs_has_write_check(PyObject* o, void* var) @@ -1390,7 +1375,8 @@ psyco_curs_copy_to(cursorObject *self, PyObject *args, PyObject *kwargs) size_t query_size; char *columnlist = NULL; const char *table_name; - const char *sep = "\t", *null = NULL; + const char *sep = "\t"; + const char *null = "\\N"; PyObject *file, *columns = NULL, *res = NULL; char *quoted_delimiter = NULL; char *quoted_null = NULL; @@ -1417,41 +1403,25 @@ psyco_curs_copy_to(cursorObject *self, PyObject *args, PyObject *kwargs) goto exit; } + if (!(quoted_null = psycopg_escape_string( + (PyObject*)self->conn, null, 0, NULL, NULL))) { + PyErr_NoMemory(); + goto exit; + } + query = query_buffer; - if (null) { - if (!(quoted_null = psycopg_escape_string( - (PyObject*)self->conn, null, 0, NULL, NULL))) { + query_size = PyOS_snprintf(query, DEFAULT_COPYBUFF, + "COPY %s%s TO stdout WITH DELIMITER AS %s NULL AS %s", + table_name, columnlist, quoted_delimiter, quoted_null); + if (query_size >= DEFAULT_COPYBUFF) { + /* Got truncated, allocate dynamically */ + if (!(query = PyMem_New(char, query_size + 1))) { PyErr_NoMemory(); goto exit; } - query_size = PyOS_snprintf(query, DEFAULT_COPYBUFF, - "COPY %s%s TO stdout WITH DELIMITER AS %s" - " NULL AS %s", table_name, columnlist, quoted_delimiter, quoted_null); - if (query_size >= DEFAULT_COPYBUFF) { - /* Got truncated, allocate dynamically */ - if (!(query = PyMem_New(char, query_size + 1))) { - PyErr_NoMemory(); - goto exit; - } - PyOS_snprintf(query, query_size + 1, - "COPY %s%s TO stdout WITH DELIMITER AS %s" - " NULL AS %s", table_name, columnlist, quoted_delimiter, quoted_null); - } - } - else { - query_size = PyOS_snprintf(query, DEFAULT_COPYBUFF, - "COPY %s%s TO stdout WITH DELIMITER AS %s", - table_name, columnlist, quoted_delimiter); - if (query_size >= DEFAULT_COPYBUFF) { - /* Got truncated, allocate dynamically */ - if (!(query = PyMem_New(char, query_size + 1))) { - PyErr_NoMemory(); - goto exit; - } - PyOS_snprintf(query, query_size + 1, - "COPY %s%s TO stdout WITH DELIMITER AS %s", - table_name, columnlist, quoted_delimiter); - } + PyOS_snprintf(query, query_size + 1, + "COPY %s%s TO stdout WITH DELIMITER AS %s NULL AS %s", + table_name, columnlist, quoted_delimiter, quoted_null); } Dprintf("psyco_curs_copy_to: query = %s", query); From 60b49f5c45926180b2f6011470867d6de9b64c0c Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 19 Oct 2011 21:01:53 +0100 Subject: [PATCH 2/4] Avoid PyOS_snprintf to calculate the copy command buffer size On windows it returns -1 instead of sometihing portable. So just ditch the static buffer and just use a dynamic one to compose the command. Also squashed a couple of buglets in copy_to: copyfile was decremented before being set to null, size_t was used instead of Py_ssize_t. --- psycopg/cursor_type.c | 91 ++++++++++++++++++++----------------------- 1 file changed, 42 insertions(+), 49 deletions(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index d644df52..f1ed38ee 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1267,20 +1267,23 @@ _psyco_curs_has_read_check(PyObject* o, void* var) static PyObject * psyco_curs_copy_from(cursorObject *self, PyObject *args, PyObject *kwargs) { - char *query = NULL; - char query_buffer[DEFAULT_COPYBUFF]; - Py_ssize_t query_size; - const char *table_name; + static char *kwlist[] = { + "file", "table", "sep", "null", "size", "columns", NULL}; + const char *sep = "\t"; const char *null = "\\N"; - Py_ssize_t bufsize = DEFAULT_COPYBUFF; - PyObject *file, *columns = NULL, *res = NULL; + const char *command = + "COPY %s%s FROM stdin WITH DELIMITER AS %s NULL AS %s"; + + Py_ssize_t query_size; + char *query = NULL; char *columnlist = NULL; char *quoted_delimiter = NULL; char *quoted_null = NULL; - static char *kwlist[] = { - "file", "table", "sep", "null", "size", "columns", NULL}; + const char *table_name; + Py_ssize_t bufsize = DEFAULT_COPYBUFF; + PyObject *file, *columns = NULL, *res = NULL; if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O&s|ss" CONV_CODE_PY_SSIZE_T "O", kwlist, @@ -1310,21 +1313,16 @@ psyco_curs_copy_from(cursorObject *self, PyObject *args, PyObject *kwargs) goto exit; } - query = query_buffer; - query_size = PyOS_snprintf(query, DEFAULT_COPYBUFF, - "COPY %s%s FROM stdin WITH DELIMITER AS %s NULL AS %s", - table_name, columnlist, quoted_delimiter, quoted_null); - if (query_size >= DEFAULT_COPYBUFF) { - /* Got truncated, allocate dynamically */ - if (!(query = PyMem_New(char, query_size + 1))) { - PyErr_NoMemory(); - goto exit; - } - PyOS_snprintf(query, query_size + 1, - "COPY %s%s FROM stdin WITH DELIMITER AS %s NULL AS %s", - table_name, columnlist, quoted_delimiter, quoted_null); + query_size = strlen(command) + strlen(table_name) + strlen(columnlist) + + strlen(quoted_delimiter) + strlen(quoted_null) + 1; + if (!(query = PyMem_New(char, query_size + 1))) { + PyErr_NoMemory(); + goto exit; } + PyOS_snprintf(query, query_size, command, + table_name, columnlist, quoted_delimiter, quoted_null); + Dprintf("psyco_curs_copy_from: query = %s", query); self->copysize = bufsize; @@ -1336,14 +1334,13 @@ psyco_curs_copy_from(cursorObject *self, PyObject *args, PyObject *kwargs) Py_INCREF(Py_None); } - self->copyfile = NULL; - Py_DECREF(file); + Py_CLEAR(self->copyfile); exit: PyMem_Free(columnlist); PyMem_Free(quoted_delimiter); PyMem_Free(quoted_null); - if (query != query_buffer) { PyMem_Free(query); } + PyMem_Free(query); return res; } @@ -1370,18 +1367,21 @@ _psyco_curs_has_write_check(PyObject* o, void* var) static PyObject * psyco_curs_copy_to(cursorObject *self, PyObject *args, PyObject *kwargs) { - char *query = NULL; - char query_buffer[DEFAULT_COPYBUFF]; - size_t query_size; - char *columnlist = NULL; - const char *table_name; + static char *kwlist[] = {"file", "table", "sep", "null", "columns", NULL}; + const char *sep = "\t"; const char *null = "\\N"; - PyObject *file, *columns = NULL, *res = NULL; + const char *command = + "COPY %s%s TO stdout WITH DELIMITER AS %s NULL AS %s"; + + Py_ssize_t query_size; + char *query = NULL; + char *columnlist = NULL; char *quoted_delimiter = NULL; char *quoted_null = NULL; - static char *kwlist[] = {"file", "table", "sep", "null", "columns", NULL}; + const char *table_name; + PyObject *file, *columns = NULL, *res = NULL; if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O&s|ssO", kwlist, _psyco_curs_has_write_check, &file, @@ -1409,21 +1409,16 @@ psyco_curs_copy_to(cursorObject *self, PyObject *args, PyObject *kwargs) goto exit; } - query = query_buffer; - query_size = PyOS_snprintf(query, DEFAULT_COPYBUFF, - "COPY %s%s TO stdout WITH DELIMITER AS %s NULL AS %s", - table_name, columnlist, quoted_delimiter, quoted_null); - if (query_size >= DEFAULT_COPYBUFF) { - /* Got truncated, allocate dynamically */ - if (!(query = PyMem_New(char, query_size + 1))) { - PyErr_NoMemory(); - goto exit; - } - PyOS_snprintf(query, query_size + 1, - "COPY %s%s TO stdout WITH DELIMITER AS %s NULL AS %s", - table_name, columnlist, quoted_delimiter, quoted_null); + query_size = strlen(command) + strlen(table_name) + strlen(columnlist) + + strlen(quoted_delimiter) + strlen(quoted_null) + 1; + if (!(query = PyMem_New(char, query_size))) { + PyErr_NoMemory(); + goto exit; } + PyOS_snprintf(query, query_size, command, + table_name, columnlist, quoted_delimiter, quoted_null); + Dprintf("psyco_curs_copy_to: query = %s", query); self->copysize = 0; @@ -1435,14 +1430,13 @@ psyco_curs_copy_to(cursorObject *self, PyObject *args, PyObject *kwargs) Py_INCREF(Py_None); } - Py_DECREF(file); - self->copyfile = NULL; + Py_CLEAR(self->copyfile); exit: PyMem_Free(columnlist); PyMem_Free(quoted_delimiter); PyMem_Free(quoted_null); - if (query != query_buffer) { PyMem_Free(query); } + PyMem_Free(query); return res; } @@ -1510,8 +1504,7 @@ psyco_curs_copy_expert(cursorObject *self, PyObject *args, PyObject *kwargs) Py_INCREF(res); } - self->copyfile = NULL; - Py_DECREF(file); + Py_CLEAR(self->copyfile); exit: Py_XDECREF(sql); From 2671472de864321d625a289f03acf0b198ddcf7c Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 20 Oct 2011 11:02:25 +0100 Subject: [PATCH 3/4] Dropped leftover extra char, already accounted for before --- psycopg/cursor_type.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index f1ed38ee..b06371a3 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1315,7 +1315,7 @@ psyco_curs_copy_from(cursorObject *self, PyObject *args, PyObject *kwargs) query_size = strlen(command) + strlen(table_name) + strlen(columnlist) + strlen(quoted_delimiter) + strlen(quoted_null) + 1; - if (!(query = PyMem_New(char, query_size + 1))) { + if (!(query = PyMem_New(char, query_size))) { PyErr_NoMemory(); goto exit; } From 83d457361e1d438359439575d8adc489dc32cc5e Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 20 Oct 2011 11:07:24 +0100 Subject: [PATCH 4/4] Fixed docs for the copy null parameter --- doc/src/cursor.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/src/cursor.rst b/doc/src/cursor.rst index d2a59ddd..8eb339aa 100644 --- a/doc/src/cursor.rst +++ b/doc/src/cursor.rst @@ -457,7 +457,7 @@ The ``cursor`` class The :sql:`COPY` command is a PostgreSQL extension to the SQL standard. As such, its support is a Psycopg extension to the |DBAPI|. - .. method:: copy_from(file, table, sep='\\t', null='\\N', size=8192, columns=None) + .. method:: copy_from(file, table, sep='\\t', null='\\\\N', size=8192, columns=None) Read data *from* the file-like object *file* appending them to the table named *table*. See :ref:`copy` for an overview. @@ -467,6 +467,7 @@ The ``cursor`` class :param table: name of the table to copy data into. :param sep: columns separator expected in the file. Defaults to a tab. :param null: textual representation of :sql:`NULL` in the file. + The default is the two character string ``\N``. :param size: size of the buffer used to read from the file. :param columns: iterable with name of the columns to import. The length and types should match the content of the file to read. @@ -489,7 +490,7 @@ The ``cursor`` class are encoded in the connection `~connection.encoding` when sent to the backend. - .. method:: copy_to(file, table, sep='\\t', null='\\N', columns=None) + .. method:: copy_to(file, table, sep='\\t', null='\\\\N', columns=None) Write the content of the table named *table* *to* the file-like object *file*. See :ref:`copy` for an overview. @@ -499,6 +500,7 @@ The ``cursor`` class :param table: name of the table to copy data from. :param sep: columns separator expected in the file. Defaults to a tab. :param null: textual representation of :sql:`NULL` in the file. + The default is the two character string ``\N``. :param columns: iterable with name of the columns to export. If not specified, export all the columns.