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.


Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#======================================== jwieck@debis.com (Jan Wieck) #



Mon, 1 Feb 1999 21:35:29 +0100 (MET)
jwieck@debis.com (Jan Wieck)
Small patches in copy.c and trigger.c
> 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 change is in the handler of the "copy" command
  - most other areas of the backend already used the correct functions.
  This change will make the way for cleanup of the internal stuff there -
  now that all the functions accessing the file descriptors are 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, Pennsylvania 19026

Mon, 1 Feb 1999 18:23:59 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Small patches in copy.c and trigger.c
Bruce Momjian <maillist@candle.pha.pa.us> writes:
> 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.
> Several users have complained about 6.4.* COPY slowing down when loading
> rows.  This may be the cause.  Good job finding it.

I thought Magnus' changes were only in the current CVS branch, not
in REL6_4 ?

			regards, tom lane

Mon, 01 Feb 1999 19:46:58 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] Small patches in copy.c and trigger.c 
> Bruce Momjian <maillist@candle.pha.pa.us> writes:
> > 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.
> > Several users have complained about 6.4.* COPY slowing down when loading
> > rows.  This may be the cause.  Good job finding it.
> 
> I thought Magnus' changes were only in the current CVS branch, not
> in REL6_4 ?

You are absolutely right.  Sorry Magnus.  The COPY complaint I heard
obviously was not from this.


Back to our regularly scheduled program.

-- 
  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, Pennsylvania 19026

Mon, 1 Feb 1999 21:58:32 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Small patches in copy.c and trigger.c
Hello!

On Mon, 1 Feb 1999, Jan Wieck wrote:
>     I've  just  committed  two  very simple patches to copy.c and
>     trigger.c which caused backend to grow until transaction end.

   Any chance I will see the patch? Or even better, postgres 6.4.3? Recently I
got a problem loading db dump...

Oleg.
---- 
    Oleg Broytmann     http://members.xoom.com/phd2/     phd2@earthling.net
           Programmers don't die, they just GOSUB without RETURN.


Tue, 2 Feb 1999 12:45:34 +0300 (MSK)
Oleg Broytmann <phd@sun.med.ru>
Re: [HACKERS] Small patches in copy.c and trigger.c
>
> Hello!
>
> On Mon, 1 Feb 1999, Jan Wieck wrote:
> >     I've  just  committed  two  very simple patches to copy.c and
> >     trigger.c which caused backend to grow until transaction end.
>
>    Any chance I will see the patch? Or even better, postgres 6.4.3? Recently I
> got a problem loading db dump...

    Must  be  something  different. I've checked v6.4 and v6.4.2,
    and the duplicate palloc()  for  values  isn't  there.  Since
    triggers are created after loading the data in any dump, that
    one couldn't be it either.

    I'll do some memory tests with copy in the released  versions
    and if I find something, put a patch onto the ftp server.


Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#======================================== jwieck@debis.com (Jan Wieck) #



Tue, 2 Feb 1999 10:54:04 +0100 (MET)
jwieck@debis.com (Jan Wieck)
Re: [HACKERS] Small patches in copy.c and trigger.c
>
> > Bruce Momjian <maillist@candle.pha.pa.us> writes:
> > > 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.
> > > Several users have complained about 6.4.* COPY slowing down when loading
> > > rows.  This may be the cause.  Good job finding it.
> >
> > I thought Magnus' changes were only in the current CVS branch, not
> > in REL6_4 ?
>
> You are absolutely right.  Sorry Magnus.  The COPY complaint I heard
> obviously was not from this.
>

    If we don't discover any errors on COPY FROM after some days,
    I think I should fix it in REL6_4 too.

    Do we plan to release v6.4.3 sometimes? If so, are there  any
    neat things I could add to the v6.4.3 feature patch?


Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#======================================== jwieck@debis.com (Jan Wieck) #



Tue, 2 Feb 1999 14:19:25 +0100 (MET)
jwieck@debis.com (Jan Wieck)
Re: [HACKERS] Small patches in copy.c and trigger.c
> > > I thought Magnus' changes were only in the current CVS branch, not
> > > in REL6_4 ?
> >
> > You are absolutely right.  Sorry Magnus.  The COPY complaint I heard
> > obviously was not from this.
> >
> 
>     If we don't discover any errors on COPY FROM after some days,
>     I think I should fix it in REL6_4 too.
> 
>     Do we plan to release v6.4.3 sometimes? If so, are there  any
>     neat things I could add to the v6.4.3 feature patch?

You mean your fixes, right.  Magnus's stuff was only in current, and is
fixed now.

-- 
  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, Pennsylvania 19026

Tue, 2 Feb 1999 11:35:45 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Small patches in copy.c and trigger.c
>
> > > > I thought Magnus' changes were only in the current CVS branch, not
> > > > in REL6_4 ?
> > >
> > > You are absolutely right.  Sorry Magnus.  The COPY complaint I heard
> > > obviously was not from this.
> > >
> >
> >     If we don't discover any errors on COPY FROM after some days,
> >     I think I should fix it in REL6_4 too.
> >
> >     Do we plan to release v6.4.3 sometimes? If so, are there  any
> >     neat things I could add to the v6.4.3 feature patch?
>
> You mean your fixes, right.  Magnus's stuff was only in current, and is
> fixed now.

    That one in COPY.

    But  the  other  one  in  ExecBRDeleteTriggers()  is still in
    place.  If  a  trigger  returns  something  it  created  with
    SPI_copytuple() (what's done for PL/pgSQL triggers allways if
    returning NEW or OLD), that heap_tuple isn't pfree()'d  until
    transaction end.

    Since  it's  safe to throw it away I'll go ahead and put that
    into REL6_4.


Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#======================================== jwieck@debis.com (Jan Wieck) #



Tue, 2 Feb 1999 17:49:53 +0100 (MET)
jwieck@debis.com (Jan Wieck)
Re: [HACKERS] Small patches in copy.c and trigger.c