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

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

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

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.

Phew - I was going crazy trying to find out why I had touched anything to do
with palloc() :-)
Plus it was not in 6.4...

//Magnus

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.

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) #

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) #

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

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) #