PyGreSQL inserttable patch

=====================

I suggested an improvement of the inserttable in the PyGreSQL interface
already in January, but seemingly it was never implemented. I was told this
is the right place to get patches in for PyGreSQL, so I'm reposting my patch
here.

I consider the inserttable methode essential in populating the database
because of its benefits in performance compared to insert, so I think this
patch is quite essential. The attachment is an improved version of the
corresponding pg_inserttable function in pgmodule.c, which fixes the
following problems:

* The function raised exceptions because PyList_GetItem was used beyond the
size of the list. This was checked by comparing the result with NULL, but
the exception was not cleaned up, which could result in mysterious errors in
the following Python code. Instead of clearing the exception using
PyErr_Clear or something like that, I avoided throwing the exception at all
by at first requesting the size of the list. Using this opportunity, I also
checked the uniformity of the size of the rows passed in the lists/tuples.
The function also accepts (and silently ignores) empty lists and sublists.
* Python "None" values are now accepted and properly converted to PostgreSQL
NULL values
* The function now generates an error message in case of a line buffer
overflow
* It copes with tabulators, newlines and backslashes in strings now
* Rewrote the buffer filling code which should now run faster by avoiding
unnecessary string copy operations forth and back

Christoph Zwerschke

1 files changed, 181 insertions, 98 deletions

diff --git a/src/interfaces/python/pgmodule.c b/src/interfaces/python/pgmodule.c
index 4dd85e0eea2..2058ed35bb6 100644
--- a/src/interfaces/python/pgmodule.c
+++ b/src/interfaces/python/pgmodule.c
@@ -2324,18 +2324,21 @@ static char pg_inserttable__doc__[] =
 static PyObject *
 pg_inserttable(pgobject * self, PyObject * args)
 {
-	PGresult   *result;
-	char	   *table,
-			   *buffer,
-			   *temp;
-	char		temp_buffer[256];
-	PyObject   *list,
-			   *sublist,
-			   *item;
-	PyObject   *(*getitem) (PyObject *, int);
-	PyObject   *(*getsubitem) (PyObject *, int);
+	PGresult	*result;
+	char		*table,
+				*buffer,
+				*bufpt,
+				*temp;
+	size_t		bufsiz;
+	PyObject	*list,
+				*sublist,
+				*item;
+	PyObject	*(*getitem) (PyObject *, int);
+	PyObject	*(*getsubitem) (PyObject *, int);
 	int			i,
-				j;
+				j,
+				m,
+				n;
 
 	if (!self->cnx)
 	{
@@ -2347,123 +2350,203 @@ pg_inserttable(pgobject * self, PyObject * args)
 	if (!PyArg_ParseTuple(args, "sO:filter", &table, &list))
 	{
 		PyErr_SetString(PyExc_TypeError,
-					  "tableinsert(table, content), with table (string) "
-						"and content (list).");
+			"tableinsert(table, content), with table (string) "
+			"and content (list).");
 		return NULL;
 	}
 
 	/* checks list type */
 	if (PyTuple_Check(list))
+	{
+		m = PyTuple_Size(list);
 		getitem = PyTuple_GetItem;
+	}
 	else if (PyList_Check(list))
+	{
+		m = PyList_Size(list);
 		getitem = PyList_GetItem;
+	}
 	else
 	{
 		PyErr_SetString(PyExc_TypeError,
-						"second arg must be some kind of array.");
+			"second arg must be some kind of array.");
 		return NULL;
 	}
 
-	/* checks sublists type */
-	for (i = 0; (sublist = getitem(list, i)) != NULL; i++)
+	if (m)
 	{
-		if (!PyTuple_Check(sublist) && !PyList_Check(sublist))
+		/* checks sublists type and size */
+		for (i = 0; i < m; i++)
 		{
-			PyErr_SetString(PyExc_TypeError,
-						 "second arg must contain some kind of arrays.");
-			return NULL;
-		}
-	}
-
-	/* allocate buffer */
-	if (!(buffer = malloc(MAX_BUFFER_SIZE)))
-	{
-		PyErr_SetString(PyExc_MemoryError, "can't allocate insert buffer.");
-		return NULL;
-	}
-
-	/* starts query */
-	sprintf(buffer, "copy %s from stdin", table);
-
-	Py_BEGIN_ALLOW_THREADS
-		result = PQexec(self->cnx, buffer);
-	Py_END_ALLOW_THREADS
-
-		if (!result)
-	{
-		free(buffer);
-		PyErr_SetString(PyExc_ValueError, PQerrorMessage(self->cnx));
-		return NULL;
-	}
-
-	PQclear(result);
-
-	/* feeds table */
-	for (i = 0; (sublist = getitem(list, i)) != NULL; i++)
-	{
-		if (PyTuple_Check(sublist))
-			getsubitem = PyTuple_GetItem;
-		else
-			getsubitem = PyList_GetItem;
-
-		/* builds insert line */
-		buffer[0] = 0;
-
-		for (j = 0; (item = getsubitem(sublist, j)) != NULL; j++)
-		{
-			/* converts item to string */
-			if (PyString_Check(item))
-				temp = PyString_AS_STRING(item);
-			else if (PyInt_Check(item))
-			{
-				long		k;
-
-				k = PyInt_AsLong(item);
-				sprintf(temp_buffer, "%ld", k);
-				temp = temp_buffer;
-			}
-			else if (PyLong_Check(item))
+			sublist = getitem(list, i);
+			if (PyTuple_Check(sublist))
+				j = PyTuple_Size(sublist);
+			else if (PyList_Check(sublist))
+				j = PyList_Size(sublist);
+			else
 			{
-				long		k;
-
-				k = PyLong_AsLong(item);
-				sprintf(temp_buffer, "%ld", k);
-				temp = temp_buffer;
+				PyErr_SetString(PyExc_TypeError,
+					"second arg must contain some kind of arrays.");
+				return NULL;
 			}
-			else if (PyFloat_Check(item))
+			if (i)
 			{
-				double		k;
-
-				k = PyFloat_AS_DOUBLE(item);
-				sprintf(temp_buffer, "%g", k);
-				temp = temp_buffer;
+				if (j != n)
+				{
+					PyErr_SetString(PyExc_TypeError,
+						"arrays contained in second arg must have same size.");
+					return NULL;
+				}
 			}
 			else
+				n=j;
+		}
+		if (n)
+		{	
+			/* allocate buffer */
+			if (!(buffer = malloc(MAX_BUFFER_SIZE)))
+			{		
+				PyErr_SetString(PyExc_MemoryError,
+					"can't allocate insert buffer.");
+				return NULL;
+			}
+
+			/* starts query */
+			sprintf(buffer, "copy %s from stdin", table);
+
+			Py_BEGIN_ALLOW_THREADS
+				result = PQexec(self->cnx, buffer);
+			Py_END_ALLOW_THREADS
+
+			if (!result)
 			{
 				free(buffer);
-				PyErr_SetString(PyExc_ValueError,
-								"items must be strings, integers, "
-								"longs or double (real).");
+				PyErr_SetString(PyExc_ValueError, PQerrorMessage(self->cnx));
 				return NULL;
 			}
 
-			/* concats buffer */
-			if (strlen(buffer))
-				strncat(buffer, "\t", MAX_BUFFER_SIZE - strlen(buffer));
+			PQclear(result);
 
-			strncat(buffer, temp, MAX_BUFFER_SIZE - strlen(buffer));
-		}
+			/* feeds table */
+			for (i = 0; i < m; i++)
+			{
+				sublist = getitem(list, i);
+				if (PyTuple_Check(sublist))
+					getsubitem = PyTuple_GetItem;
+				else
+					getsubitem = PyList_GetItem;
+	
+				/* builds insert line */
+				bufpt=buffer;
+				bufsiz = MAX_BUFFER_SIZE - 1;
+
+				for (j = 0; j < n; j++)
+				{
+					item = getsubitem(sublist, j);
+					/* converts item to string */
+					if (item == Py_None)
+					{
+						if (bufsiz > 2)
+						{
+							*bufpt++ = '\\'; *bufpt++ = 'N';
+							bufsiz -= 2;
+						}
+						else
+							bufsiz = 0;
+					}
+					else if (PyString_Check(item))
+					{
+						temp = PyString_AS_STRING(item);
+						while (*temp && bufsiz)
+						{
+						     if (*temp == '\\'
+						      || *temp == '\t'
+						      || *temp == '\n')
+						     {
+						        *bufpt++='\\'; --bufsiz;
+						        if (!bufsiz) break;
+						     }
+						     *bufpt++=*temp++; --bufsiz;
+						}
+					}
+					else if (PyInt_Check(item))
+					{
+						long	k;
+						int 	h;
+
+						k = PyInt_AsLong(item);
+						h = snprintf(bufpt, bufsiz, "%ld", k);
+						if (h > 0)
+						{
+							bufpt += h; bufsiz -= h;
+						}
+						else
+							bufsiz = 0;
+					}
+					else if (PyLong_Check(item))
+					{
+						long	k;
+						int 	h;
+
+						k = PyLong_AsLong(item);
+						h = snprintf(bufpt, bufsiz, "%ld", k);
+						if (h > 0)
+						{
+							bufpt += h; bufsiz -= h;
+						}
+						else
+							bufsiz = 0;
+					}
+					else if (PyFloat_Check(item))
+					{
+						double	k;
+						int		h;
+
+						k = PyFloat_AS_DOUBLE(item);
+						h = snprintf(bufpt, bufsiz, "%g", k);
+						if (h > 0)
+						{
+							bufpt += h; bufsiz -= h;
+						}
+						else
+							bufsiz = 0;
+					}
+					else
+					{
+						free(buffer);
+						PyErr_SetString(PyExc_ValueError,
+							"items must be strings, integers, "
+							"longs or double (real).");
+						return NULL;
+					}
+
+					if (j < n-1)
+					{
+						*bufpt++ = '\t'; --bufsiz;
+					}
+			
+					if (bufsiz <= 0)
+					{
+						free(buffer);
+						PyErr_SetString(PyExc_MemoryError,
+							"insert buffer overflow.");
+						return NULL;
+					}
 
-		strncat(buffer, "\n", MAX_BUFFER_SIZE - strlen(buffer));
+				}
 
-		/* sends data */
-		PQputline(self->cnx, buffer);
-	}
+				*bufpt++ = '\n'; *bufpt = '\0';
 
-	/* ends query */
-	PQputline(self->cnx, "\\.\n");
-	PQendcopy(self->cnx);
-	free(buffer);
+				/* sends data */
+				PQputline(self->cnx, buffer);
+			}
+	
+			/* ends query */
+			PQputline(self->cnx, "\\.\n");
+			PQendcopy(self->cnx);
+			free(buffer);
+		}
+	}
 
 	/* no error : returns nothing */
 	Py_INCREF(Py_None);