Re: [HACKERS] Small patches in copy.c and trigger.c - Mailing list pgsql-hackers

From Bruce Momjian
Subject Re: [HACKERS] Small patches in copy.c and trigger.c
Date
Msg-id 199902012324.SAA03146@candle.pha.pa.us
Whole thread Raw
In response to Small patches in copy.c and trigger.c  (jwieck@debis.com (Jan Wieck))
Responses Re: [HACKERS] Small patches in copy.c and trigger.c
List pgsql-hackers
> Hi,
> 
>     I've  just  committed  two  very simple patches to copy.c and
>     trigger.c which caused backend to grow until transaction end.
> 
>     trigger.c  didn't  expected  that trigger function could have
>     returned another heap tuple that was built inside of  trigger
>     with SPI_copytuple().
> 
>     In  copy.c  I'n  not absolutely sure why it was as it was. In
>     CopyFrom() the array  for  the  values  was  palloc()'d  once
>     before  entering  the copy loop, and then again at the top of
>     the loop. But there was only one pfree() after loop exited.
> 
>     I've removed the palloc() inside the loop. Seems to work  for
>     the  regression  test. Telling here only for the case someone
>     encounters problems on COPY FROM.

I have a morbid curiosity, so I decided to find out how this got into
the source.  On November 1, 1998, Magus applied a patch:
 Here is a first patch to cleanup the backend side of libpq. This patch removes all external dependencies on the "Pfin"
and"Pfout" that are declared in pqcomm.h. These variables are also changed to "static" to make sure. Almost all the
changeis in the handler of the "copy" command - most other areas of the backend already used the correct functions.
Thischange will make the way for cleanup of the internal stuff there - now that all the functions accessing the file
descriptorsare confined to a single directory.
 

Several users have complained about 6.4.* COPY slowing down when loading
rows.  This may be the cause.  Good job finding it.

In fact, Magnus added two palloc's, when 'values' already had a
palloc().  Magnus, we all make goofy mistakes, so don't sweat it. 
However, if you had some deeper reason for adding the palloc's, please
let us know.  Jan, I will make sure there is only one palloc for 'values'
in CopyFrom().

I am about to commit TEMP tables anyway.

---------------------------------------------------------------------------

   values = (Datum *) palloc(sizeof(Datum) * attr_count);   nulls = (char *) palloc(attr_count);   index_nulls = (char
*)palloc(attr_count);   idatum = (Datum *) palloc(sizeof(Datum) * attr_count);   byval = (bool *) palloc(attr_count *
sizeof(bool));        for (i = 0; i < attr_count; i++)   {       nulls[i] = ' ';       index_nulls[i] = ' ';
byval[i]= (bool) IsTypeByVal(attr[i]->atttypid);   }   values = (Datum *) palloc(sizeof(Datum) * attr_count);
lineno= 0;   while (!done)   {     values = (Datum *) palloc(sizeof(Datum) * attr_count);       if (!binary)
 


---------------------------------------------------------------------------


--  Bruce Momjian                        |  http://www.op.net/~candle maillist@candle.pha.pa.us            |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026
 


pgsql-hackers by date:

Previous
From: jwieck@debis.com (Jan Wieck)
Date:
Subject: Small patches in copy.c and trigger.c
Next
From: "Jackson, DeJuan"
Date:
Subject: RE: [HACKERS] Patches