Обсуждение: 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.
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) #
> 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
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, Pennsylvania19026
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 justGOSUB 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, Pennsylvania19026
>
> > > > 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) #