Re: Updated patch to quote delimiter and null characters in copy_* functions

On Wed, 2008-09-24 at 09:59 +0200, Federico Di Gregorio wrote:
> Il giorno mer, 24/09/2008 alle 18.04 +1000, Alejandro Dubrovsky ha
> scritto:
> > On Tue, 2008-09-23 at 15:34 +0200, Federico Di Gregorio wrote:
> > > Il giorno mar, 23/09/2008 alle 14.38 +1000, Alejandro Dubrovsky ha scritto:
> > > > This patch enlarges the default size of the copy buffer and also checks
> > > > to see if the copy query got truncated.  ie copy_to(somefile, '(some
> > > > long query)') is very handy and I went crazy with it until I broke the
> > > > first patch.
> > > 
> > > Your patch seems fine to me and it surely add needed functionality but
> > > I don't like to have to different quoting functions. Do you have the
> > > time to move all functionality of qstring_escape() to
> > > psycopg_internal_escape_string() and then have qstring_escape call that?
> > > Also note that the patch you sent does not include utils.h and utils.c.
> > > 
> > I'm not sure I understand what you are suggesting.  In the patch I sent,
> > psycopg_internal_escape_string calls qstring_escape, there aren't two
> > quoting functions.  psycopg_internal_escape_string is just a wrapper
> > around qstring_escape that makes it easier on the caller.  
> > 
> > Would you prefer the calling structure to be in reverse? (qstring_escape
> > calling psycopg2_internal_escape_string)  That seems wrong to me since
> > psycopg_internal_escape_string creates a new buffer, but qstring_escape
> > is given one, so qstring_escape would just copy the result from
> > psycopg_internal_escape_string and free the newly allocated buffer, even
> > though there was no need to create or destroy the buffer in the first
> > place.
> > 
> > Getting rid of qstring_escape altogether, on the other hand, that I
> > could understand.
> 
> The problem were the missing files. ;) Without them I guessed you
> duplicated the qstring_escape code, not that you just wrapped it.
> Looking at utils.c everything seems fine to me. I'll commit ASAP.
> 
> federico
> 
Excellent, thanks!