617 lines
26 KiB
Plaintext
617 lines
26 KiB
Plaintext
From tgl@sss.pgh.pa.us Mon Jun 14 20:50:41 1999
|
|
Received: from sss.sss.pgh.pa.us (sss.pgh.pa.us [206.210.65.6])
|
|
by candle.pha.pa.us (8.9.0/8.9.0) with ESMTP id UAA19110
|
|
for <maillist@candle.pha.pa.us>; Mon, 14 Jun 1999 20:50:39 -0400 (EDT)
|
|
Received: from sss.sss.pgh.pa.us (localhost [127.0.0.1])
|
|
by sss.sss.pgh.pa.us (8.9.1/8.9.1) with ESMTP id UAA21506;
|
|
Mon, 14 Jun 1999 20:51:07 -0400 (EDT)
|
|
To: Bruce Momjian <maillist@candle.pha.pa.us>
|
|
cc: Roman.Hodek@informatik.uni-erlangen.de, olly@lfix.co.uk,
|
|
PostgreSQL-development <pgsql-hackers@postgreSQL.org>
|
|
Subject: Cleaning up function interface (was Re: Patch for m68k architecture)
|
|
In-reply-to: Your message of Mon, 14 Jun 1999 17:53:25 -0400 (EDT)
|
|
<199906142153.RAA16276@candle.pha.pa.us>
|
|
Date: Mon, 14 Jun 1999 20:51:06 -0400
|
|
Message-ID: <21504.929407866@sss.pgh.pa.us>
|
|
From: Tom Lane <tgl@sss.pgh.pa.us>
|
|
Status: RO
|
|
|
|
Bruce Momjian <maillist@candle.pha.pa.us> writes:
|
|
>> ANSI C says results are undefined if you call a function via pointer
|
|
>> and the pointer is declared to return another type than the function
|
|
>> actually returns. So m68k compilers conform to the standard here.
|
|
|
|
> Yes, we admit that we break the standard with fmgr_ptr, because we
|
|
> return a variety of values depending on what function they call. It
|
|
> appears the egcs optimization on the powerpc or alpha cause a problem
|
|
> when optimization is -O2, but not -O. We may see more platforms with
|
|
> problems as optimizers get smarter.
|
|
|
|
Seeing as how we also know that the function-call interface ought to be
|
|
redesigned to handle NULLs better, maybe we should just bite the bullet
|
|
and fix all of these problems at once by adopting a new standard
|
|
interface for everything that can be called via fmgr. It'd uglify the
|
|
code, no doubt, but I think we are starting to see an accumulation of
|
|
problems that justify doing something.
|
|
|
|
Here is a straw-man proposal:
|
|
|
|
Datum function (bool *resultnull,
|
|
Datum *args,
|
|
bool *argnull,
|
|
int nargs)
|
|
|
|
args[i] is the i'th parameter, or undefined (perhaps always 0?)
|
|
when argnull[i] is true. The function is responsible for setting
|
|
*resultnull, and returns a Datum value if *resultnull is false.
|
|
Most standard functions could ignore nargs since they'd know what it
|
|
should be, but we ought to pass it for flexibility.
|
|
|
|
A useful addition to this scheme would be for fmgr to preset *resultnull
|
|
to the OR of the input argnull[] array just before calling the function.
|
|
In the typical case where the function is "strict" (ie, result is NULL
|
|
if any input is NULL), this would save the function from having to look
|
|
at argnull[] at all; it'd just check *resultnull and immediately return
|
|
if true.
|
|
|
|
As an example, int4 addition goes from
|
|
|
|
int32
|
|
int4pl(int32 arg1, int32 arg2)
|
|
{
|
|
return arg1 + arg2;
|
|
}
|
|
|
|
to
|
|
|
|
Datum
|
|
int4pl (bool *resultnull, Datum *args, bool *argnull, int nargs)
|
|
{
|
|
if (*resultnull)
|
|
return (Datum) 0; /* value doesn't really matter ... */
|
|
/* we can ignore argnull and nargs */
|
|
|
|
return Int32GetDatum(DatumGetInt32(args[0]) + DatumGetInt32(args[1]));
|
|
}
|
|
|
|
This is, of course, much uglier than the existing code, but we might be
|
|
able to improve matters with some well-chosen macros for the boilerplate
|
|
parts. What we actually end up writing might look something like
|
|
|
|
Datum
|
|
int4pl (PG_FUNCTION_ARGS)
|
|
{
|
|
PG_STRICT_FUNCTION( /* encapsulates null check */
|
|
PG_ARG0_INT32;
|
|
PG_ARG1_INT32;
|
|
|
|
PG_RESULT_INT32( arg0 + arg1 );
|
|
);
|
|
}
|
|
|
|
where the macros expand to things like "int32 arg0 = DatumGetInt32(args[0])"
|
|
and "return Int32GetDatum( x )". It'd be worth a little thought to
|
|
try to set up a group of macros like that, I think.
|
|
|
|
regards, tom lane
|
|
|
|
From owner-pgsql-hackers@hub.org Wed Sep 22 20:31:02 1999
|
|
Received: from renoir.op.net (root@renoir.op.net [209.152.193.4])
|
|
by candle.pha.pa.us (8.9.0/8.9.0) with ESMTP id UAA15611
|
|
for <maillist@candle.pha.pa.us>; Wed, 22 Sep 1999 20:31:01 -0400 (EDT)
|
|
Received: from hub.org (hub.org [216.126.84.1]) by renoir.op.net (o1/$ Revision: 1.18 $) with ESMTP id UAA02926 for <maillist@candle.pha.pa.us>; Wed, 22 Sep 1999 20:21:24 -0400 (EDT)
|
|
Received: from hub.org (hub.org [216.126.84.1])
|
|
by hub.org (8.9.3/8.9.3) with ESMTP id UAA75413;
|
|
Wed, 22 Sep 1999 20:09:35 -0400 (EDT)
|
|
(envelope-from owner-pgsql-hackers@hub.org)
|
|
Received: by hub.org (TLB v0.10a (1.23 tibbs 1997/01/09 00:29:32)); Wed, 22 Sep 1999 20:08:50 +0000 (EDT)
|
|
Received: (from majordom@localhost)
|
|
by hub.org (8.9.3/8.9.3) id UAA75058
|
|
for pgsql-hackers-outgoing; Wed, 22 Sep 1999 20:06:58 -0400 (EDT)
|
|
(envelope-from owner-pgsql-hackers@postgreSQL.org)
|
|
Received: from sss.sss.pgh.pa.us (sss.pgh.pa.us [209.114.166.2])
|
|
by hub.org (8.9.3/8.9.3) with ESMTP id UAA74982
|
|
for <pgsql-hackers@postgreSQL.org>; Wed, 22 Sep 1999 20:06:25 -0400 (EDT)
|
|
(envelope-from tgl@sss.pgh.pa.us)
|
|
Received: from sss.sss.pgh.pa.us (localhost [127.0.0.1])
|
|
by sss.sss.pgh.pa.us (8.9.1/8.9.1) with ESMTP id UAA06411
|
|
for <pgsql-hackers@postgreSQL.org>; Wed, 22 Sep 1999 20:05:40 -0400 (EDT)
|
|
To: pgsql-hackers@postgreSQL.org
|
|
Subject: [HACKERS] Progress report: buffer refcount bugs and SQL functions
|
|
Date: Wed, 22 Sep 1999 20:05:39 -0400
|
|
Message-ID: <6408.938045139@sss.pgh.pa.us>
|
|
From: Tom Lane <tgl@sss.pgh.pa.us>
|
|
Sender: owner-pgsql-hackers@postgreSQL.org
|
|
Precedence: bulk
|
|
Status: RO
|
|
|
|
I have been finding a lot of interesting stuff while looking into
|
|
the buffer reference count/leakage issue.
|
|
|
|
It turns out that there were two specific things that were camouflaging
|
|
the existence of bugs in this area:
|
|
|
|
1. The BufferLeakCheck routine that's run at transaction commit was
|
|
only looking for nonzero PrivateRefCount to indicate a missing unpin.
|
|
It failed to notice nonzero LastRefCount --- which meant that an
|
|
error in refcount save/restore usage could leave a buffer pinned,
|
|
and BufferLeakCheck wouldn't notice.
|
|
|
|
2. The BufferIsValid macro, which you'd think just checks whether
|
|
it's handed a valid buffer identifier or not, actually did more:
|
|
it only returned true if the buffer ID was valid *and* the buffer
|
|
had positive PrivateRefCount. That meant that the common pattern
|
|
if (BufferIsValid(buf))
|
|
ReleaseBuffer(buf);
|
|
wouldn't complain if it were handed a valid but already unpinned buffer.
|
|
And that behavior masks bugs that result in buffers being unpinned too
|
|
early. For example, consider a sequence like
|
|
|
|
1. LockBuffer (buffer now has refcount 1). Store reference to
|
|
a tuple on that buffer page in a tuple table slot.
|
|
2. Copy buffer reference to a second tuple-table slot, but forget to
|
|
increment buffer's refcount.
|
|
3. Release second tuple table slot. Buffer refcount drops to 0,
|
|
so it's unpinned.
|
|
4. Release original tuple slot. Because of BufferIsValid behavior,
|
|
no assert happens here; in fact nothing at all happens.
|
|
|
|
This is, of course, buggy code: during the interval from 3 to 4 you
|
|
still have an apparently valid tuple reference in the original slot,
|
|
which someone might try to use; but the buffer it points to is unpinned
|
|
and could be replaced at any time by another backend.
|
|
|
|
In short, we had errors that would mask both missing-pin bugs and
|
|
missing-unpin bugs. And naturally there were a few such bugs lurking
|
|
behind them...
|
|
|
|
3. The buffer refcount save/restore stuff, which I had suspected
|
|
was useless, is not only useless but also buggy. The reason it's
|
|
buggy is that it only works if used in a nested fashion. You could
|
|
save state A, pin some buffers, save state B, pin some more
|
|
buffers, restore state B (thereby unpinning what you pinned since
|
|
the save), and finally restore state A (unpinning the earlier stuff).
|
|
What you could not do is save state A, pin, save B, pin more, then
|
|
restore state A --- that might unpin some of A's buffers, or some
|
|
of B's buffers, or some unforeseen combination thereof. If you
|
|
restore A and then restore B, you do not necessarily return to a zero-
|
|
pins state, either. And it turns out the actual usage pattern was a
|
|
nearly random sequence of saves and restores, compounded by a failure to
|
|
do all of the restores reliably (which was masked by the oversight in
|
|
BufferLeakCheck).
|
|
|
|
|
|
What I have done so far is to rip out the buffer refcount save/restore
|
|
support (including LastRefCount), change BufferIsValid to a simple
|
|
validity check (so that you get an assert if you unpin something that
|
|
was pinned), change ExecStoreTuple so that it increments the refcount
|
|
when it is handed a buffer reference (for symmetry with ExecClearTuple's
|
|
decrement of the refcount), and fix about a dozen bugs exposed by these
|
|
changes.
|
|
|
|
I am still getting Buffer Leak notices in the "misc" regression test,
|
|
specifically in the queries that invoke more than one SQL function.
|
|
What I find there is that SQL functions are not always run to
|
|
completion. Apparently, when a function can return multiple tuples,
|
|
it won't necessarily be asked to produce them all. And when it isn't,
|
|
postquel_end() isn't invoked for the function's current query, so its
|
|
tuple table isn't cleared, so we have dangling refcounts if any of the
|
|
tuples involved are in disk buffers.
|
|
|
|
It may be that the save/restore code was a misguided attempt to fix
|
|
this problem. I can't tell. But I think what we really need to do is
|
|
find some way of ensuring that Postquel function execution contexts
|
|
always get shut down by the end of the query, so that they don't leak
|
|
resources.
|
|
|
|
I suppose a straightforward approach would be to keep a list of open
|
|
function contexts somewhere (attached to the outer execution context,
|
|
perhaps), and clean them up at outer-plan shutdown.
|
|
|
|
What I am wondering, though, is whether this addition is actually
|
|
necessary, or is it a bug that the functions aren't run to completion
|
|
in the first place? I don't really understand the semantics of this
|
|
"nested dot notation". I suppose it is a Berkeleyism; I can't find
|
|
anything about it in the SQL92 document. The test cases shown in the
|
|
misc regress test seem peculiar, not to say wrong. For example:
|
|
|
|
regression=> SELECT p.hobbies.equipment.name, p.hobbies.name, p.name FROM person p;
|
|
name |name |name
|
|
-------------+-----------+-----
|
|
advil |posthacking|mike
|
|
peet's coffee|basketball |joe
|
|
hightops |basketball |sally
|
|
(3 rows)
|
|
|
|
which doesn't appear to agree with the contents of the underlying
|
|
relations:
|
|
|
|
regression=> SELECT * FROM hobbies_r;
|
|
name |person
|
|
-----------+------
|
|
posthacking|mike
|
|
posthacking|jeff
|
|
basketball |joe
|
|
basketball |sally
|
|
skywalking |
|
|
(5 rows)
|
|
|
|
regression=> SELECT * FROM equipment_r;
|
|
name |hobby
|
|
-------------+-----------
|
|
advil |posthacking
|
|
peet's coffee|posthacking
|
|
hightops |basketball
|
|
guts |skywalking
|
|
(4 rows)
|
|
|
|
I'd have expected an output along the lines of
|
|
|
|
advil |posthacking|mike
|
|
peet's coffee|posthacking|mike
|
|
hightops |basketball |joe
|
|
hightops |basketball |sally
|
|
|
|
Is the regression test's expected output wrong, or am I misunderstanding
|
|
what this query is supposed to do? Is there any documentation anywhere
|
|
about how SQL functions returning multiple tuples are supposed to
|
|
behave?
|
|
|
|
regards, tom lane
|
|
|
|
************
|
|
|
|
|
|
From owner-pgsql-hackers@hub.org Thu Sep 23 11:03:19 1999
|
|
Received: from hub.org (hub.org [216.126.84.1])
|
|
by candle.pha.pa.us (8.9.0/8.9.0) with ESMTP id LAA16211
|
|
for <maillist@candle.pha.pa.us>; Thu, 23 Sep 1999 11:03:17 -0400 (EDT)
|
|
Received: from hub.org (hub.org [216.126.84.1])
|
|
by hub.org (8.9.3/8.9.3) with ESMTP id KAA58151;
|
|
Thu, 23 Sep 1999 10:53:46 -0400 (EDT)
|
|
(envelope-from owner-pgsql-hackers@hub.org)
|
|
Received: by hub.org (TLB v0.10a (1.23 tibbs 1997/01/09 00:29:32)); Thu, 23 Sep 1999 10:53:05 +0000 (EDT)
|
|
Received: (from majordom@localhost)
|
|
by hub.org (8.9.3/8.9.3) id KAA57948
|
|
for pgsql-hackers-outgoing; Thu, 23 Sep 1999 10:52:23 -0400 (EDT)
|
|
(envelope-from owner-pgsql-hackers@postgreSQL.org)
|
|
Received: from sss.sss.pgh.pa.us (sss.pgh.pa.us [209.114.166.2])
|
|
by hub.org (8.9.3/8.9.3) with ESMTP id KAA57841
|
|
for <hackers@postgreSQL.org>; Thu, 23 Sep 1999 10:51:50 -0400 (EDT)
|
|
(envelope-from tgl@sss.pgh.pa.us)
|
|
Received: from sss.sss.pgh.pa.us (localhost [127.0.0.1])
|
|
by sss.sss.pgh.pa.us (8.9.1/8.9.1) with ESMTP id KAA14211;
|
|
Thu, 23 Sep 1999 10:51:10 -0400 (EDT)
|
|
To: Andreas Zeugswetter <andreas.zeugswetter@telecom.at>
|
|
cc: hackers@postgreSQL.org
|
|
Subject: Re: [HACKERS] Progress report: buffer refcount bugs and SQL functions
|
|
In-reply-to: Your message of Thu, 23 Sep 1999 10:07:24 +0200
|
|
<37E9DFBC.5C0978F@telecom.at>
|
|
Date: Thu, 23 Sep 1999 10:51:10 -0400
|
|
Message-ID: <14209.938098270@sss.pgh.pa.us>
|
|
From: Tom Lane <tgl@sss.pgh.pa.us>
|
|
Sender: owner-pgsql-hackers@postgreSQL.org
|
|
Precedence: bulk
|
|
Status: RO
|
|
|
|
Andreas Zeugswetter <andreas.zeugswetter@telecom.at> writes:
|
|
> That is what I use it for. I have never used it with a
|
|
> returns setof function, but reading the comments in the regression test,
|
|
> -- mike needs advil and peet's coffee,
|
|
> -- joe and sally need hightops, and
|
|
> -- everyone else is fine.
|
|
> it looks like the results you expected are correct, and currently the
|
|
> wrong result is given.
|
|
|
|
Yes, I have concluded the same (and partially fixed it, per my previous
|
|
message).
|
|
|
|
> Those that don't have a hobbie should return name|NULL|NULL. A hobbie
|
|
> that does'nt need equipment name|hobbie|NULL.
|
|
|
|
That's a good point. Currently (both with and without my uncommitted
|
|
fix) you get *no* rows out from ExecTargetList if there are any Iters
|
|
that return empty result sets. It might be more reasonable to treat an
|
|
empty result set as if it were NULL, which would give the behavior you
|
|
suggest.
|
|
|
|
This would be an easy change to my current patch, and I'm prepared to
|
|
make it before committing what I have, if people agree that that's a
|
|
more reasonable definition. Comments?
|
|
|
|
regards, tom lane
|
|
|
|
************
|
|
|
|
|
|
From owner-pgsql-hackers@hub.org Thu Sep 23 04:31:15 1999
|
|
Received: from renoir.op.net (root@renoir.op.net [209.152.193.4])
|
|
by candle.pha.pa.us (8.9.0/8.9.0) with ESMTP id EAA11344
|
|
for <maillist@candle.pha.pa.us>; Thu, 23 Sep 1999 04:31:15 -0400 (EDT)
|
|
Received: from hub.org (hub.org [216.126.84.1]) by renoir.op.net (o1/$ Revision: 1.18 $) with ESMTP id EAA05350 for <maillist@candle.pha.pa.us>; Thu, 23 Sep 1999 04:24:29 -0400 (EDT)
|
|
Received: from hub.org (hub.org [216.126.84.1])
|
|
by hub.org (8.9.3/8.9.3) with ESMTP id EAA85679;
|
|
Thu, 23 Sep 1999 04:16:26 -0400 (EDT)
|
|
(envelope-from owner-pgsql-hackers@hub.org)
|
|
Received: by hub.org (TLB v0.10a (1.23 tibbs 1997/01/09 00:29:32)); Thu, 23 Sep 1999 04:09:52 +0000 (EDT)
|
|
Received: (from majordom@localhost)
|
|
by hub.org (8.9.3/8.9.3) id EAA84708
|
|
for pgsql-hackers-outgoing; Thu, 23 Sep 1999 04:08:57 -0400 (EDT)
|
|
(envelope-from owner-pgsql-hackers@postgreSQL.org)
|
|
Received: from gandalf.telecom.at (gandalf.telecom.at [194.118.26.84])
|
|
by hub.org (8.9.3/8.9.3) with ESMTP id EAA84632
|
|
for <hackers@postgresql.org>; Thu, 23 Sep 1999 04:08:03 -0400 (EDT)
|
|
(envelope-from andreas.zeugswetter@telecom.at)
|
|
Received: from telecom.at (w0188000580.f000.d0188.sd.spardat.at [172.18.65.249])
|
|
by gandalf.telecom.at (xxx/xxx) with ESMTP id KAA195294
|
|
for <hackers@postgresql.org>; Thu, 23 Sep 1999 10:07:27 +0200
|
|
Message-ID: <37E9DFBC.5C0978F@telecom.at>
|
|
Date: Thu, 23 Sep 1999 10:07:24 +0200
|
|
From: Andreas Zeugswetter <andreas.zeugswetter@telecom.at>
|
|
X-Mailer: Mozilla 4.61 [en] (Win95; I)
|
|
X-Accept-Language: en
|
|
MIME-Version: 1.0
|
|
To: hackers@postgreSQL.org
|
|
Subject: Re: [HACKERS] Progress report: buffer refcount bugs and SQL functions
|
|
Content-Type: text/plain; charset=us-ascii
|
|
Content-Transfer-Encoding: 7bit
|
|
Sender: owner-pgsql-hackers@postgreSQL.org
|
|
Precedence: bulk
|
|
Status: RO
|
|
|
|
> Is the regression test's expected output wrong, or am I
|
|
> misunderstanding
|
|
> what this query is supposed to do? Is there any
|
|
> documentation anywhere
|
|
> about how SQL functions returning multiple tuples are supposed to
|
|
> behave?
|
|
|
|
They are supposed to behave somewhat like a view.
|
|
Not all rows are necessarily fetched.
|
|
If used in a context that needs a single row answer,
|
|
and the answer has multiple rows it is supposed to
|
|
runtime elog. Like in:
|
|
|
|
select * from tbl where col=funcreturningmultipleresults();
|
|
-- this must elog
|
|
|
|
while this is ok:
|
|
select * from tbl where col in (select funcreturningmultipleresults());
|
|
|
|
But the caller could only fetch the first row if he wanted.
|
|
|
|
The nested notation is supposed to call the function passing it the tuple
|
|
as the first argument. This is what can be used to "fake" a column
|
|
onto a table (computed column).
|
|
That is what I use it for. I have never used it with a
|
|
returns setof function, but reading the comments in the regression test,
|
|
-- mike needs advil and peet's coffee,
|
|
-- joe and sally need hightops, and
|
|
-- everyone else is fine.
|
|
it looks like the results you expected are correct, and currently the
|
|
wrong result is given.
|
|
|
|
But I think this query could also elog whithout removing substantial
|
|
functionality.
|
|
|
|
SELECT p.name, p.hobbies.name, p.hobbies.equipment.name FROM person p;
|
|
|
|
Actually for me it would be intuitive, that this query return one row per
|
|
person, but elog on those that have more than one hobbie or a hobbie that
|
|
needs more than one equipment. Those that don't have a hobbie should
|
|
return name|NULL|NULL. A hobbie that does'nt need equipment name|hobbie|NULL.
|
|
|
|
Andreas
|
|
|
|
************
|
|
|
|
|
|
From owner-pgsql-hackers@hub.org Wed Sep 22 22:01:07 1999
|
|
Received: from renoir.op.net (root@renoir.op.net [209.152.193.4])
|
|
by candle.pha.pa.us (8.9.0/8.9.0) with ESMTP id WAA16360
|
|
for <maillist@candle.pha.pa.us>; Wed, 22 Sep 1999 22:01:05 -0400 (EDT)
|
|
Received: from hub.org (hub.org [216.126.84.1]) by renoir.op.net (o1/$ Revision: 1.18 $) with ESMTP id VAA08386 for <maillist@candle.pha.pa.us>; Wed, 22 Sep 1999 21:37:24 -0400 (EDT)
|
|
Received: from hub.org (hub.org [216.126.84.1])
|
|
by hub.org (8.9.3/8.9.3) with ESMTP id VAA88083;
|
|
Wed, 22 Sep 1999 21:28:11 -0400 (EDT)
|
|
(envelope-from owner-pgsql-hackers@hub.org)
|
|
Received: by hub.org (TLB v0.10a (1.23 tibbs 1997/01/09 00:29:32)); Wed, 22 Sep 1999 21:27:48 +0000 (EDT)
|
|
Received: (from majordom@localhost)
|
|
by hub.org (8.9.3/8.9.3) id VAA87938
|
|
for pgsql-hackers-outgoing; Wed, 22 Sep 1999 21:26:52 -0400 (EDT)
|
|
(envelope-from owner-pgsql-hackers@postgreSQL.org)
|
|
Received: from orion.SAPserv.Hamburg.dsh.de (Tpolaris2.sapham.debis.de [53.2.131.8])
|
|
by hub.org (8.9.3/8.9.3) with SMTP id VAA87909
|
|
for <pgsql-hackers@postgresql.org>; Wed, 22 Sep 1999 21:26:36 -0400 (EDT)
|
|
(envelope-from wieck@debis.com)
|
|
Received: by orion.SAPserv.Hamburg.dsh.de
|
|
for pgsql-hackers@postgresql.org
|
|
id m11TxXw-0003kLC; Thu, 23 Sep 99 03:19 MET DST
|
|
Message-Id: <m11TxXw-0003kLC@orion.SAPserv.Hamburg.dsh.de>
|
|
From: wieck@debis.com (Jan Wieck)
|
|
Subject: Re: [HACKERS] Progress report: buffer refcount bugs and SQL functions
|
|
To: tgl@sss.pgh.pa.us (Tom Lane)
|
|
Date: Thu, 23 Sep 1999 03:19:39 +0200 (MET DST)
|
|
Cc: pgsql-hackers@postgreSQL.org
|
|
Reply-To: wieck@debis.com (Jan Wieck)
|
|
In-Reply-To: <6408.938045139@sss.pgh.pa.us> from "Tom Lane" at Sep 22, 99 08:05:39 pm
|
|
X-Mailer: ELM [version 2.4 PL25]
|
|
Content-Type: text
|
|
Sender: owner-pgsql-hackers@postgreSQL.org
|
|
Precedence: bulk
|
|
Status: RO
|
|
|
|
Tom Lane wrote:
|
|
|
|
> [...]
|
|
>
|
|
> What I am wondering, though, is whether this addition is actually
|
|
> necessary, or is it a bug that the functions aren't run to completion
|
|
> in the first place? I don't really understand the semantics of this
|
|
> "nested dot notation". I suppose it is a Berkeleyism; I can't find
|
|
> anything about it in the SQL92 document. The test cases shown in the
|
|
> misc regress test seem peculiar, not to say wrong. For example:
|
|
>
|
|
> [...]
|
|
>
|
|
> Is the regression test's expected output wrong, or am I misunderstanding
|
|
> what this query is supposed to do? Is there any documentation anywhere
|
|
> about how SQL functions returning multiple tuples are supposed to
|
|
> behave?
|
|
|
|
I've said some time (maybe too long) ago, that SQL functions
|
|
returning tuple sets are broken in general. This nested dot
|
|
notation (which I think is an artefact from the postquel
|
|
querylanguage) is implemented via set functions.
|
|
|
|
Set functions have total different semantics from all other
|
|
functions. First they don't really return a tuple set as
|
|
someone might think - all that screwed up code instead
|
|
simulates that they return something you could consider a
|
|
scan of the last SQL statement in the function. Then, on
|
|
each subsequent call inside of the same command, they return
|
|
a "tupletable slot" containing the next found tuple (that's
|
|
why their Func node is mangled up after the first call).
|
|
|
|
Second they have a targetlist what I think was originally
|
|
intended to extract attributes out of the tuples returned
|
|
when the above scan is asked to get the next tuple. But as I
|
|
read the code it invokes the function again and this might
|
|
cause the resource leakage you see.
|
|
|
|
Third, all this seems to never have been implemented
|
|
(thought?) to the end. A targetlist doesn't make sense at
|
|
this place because it could at max contain a single attribute
|
|
- so a single attno would have the same power. And if set
|
|
functions could appear in the rangetable (FROM clause), than
|
|
they would be treated as that and regular Var nodes in the
|
|
query would do it.
|
|
|
|
I think you shouldn't really care for that regression test
|
|
and maybe we should disable set functions until we really
|
|
implement stored procedures returning sets in the rangetable.
|
|
|
|
Set functions where planned by Stonebraker's team as
|
|
something that today is called stored procedures. But AFAIK
|
|
they never reached the useful state because even in Postgres
|
|
4.2 you haven't been able to get more than one attribute out
|
|
of a set function. It was a feature of the postquel
|
|
querylanguage that you could get one attribute from a set
|
|
function via
|
|
|
|
RETRIEVE (attributename(setfuncname()))
|
|
|
|
While working on the constraint triggers I've came across
|
|
another regression test (triggers :-) that's errorneous too.
|
|
The funny_dup17 trigger proc executes an INSERT into the same
|
|
relation where it get fired for by a previous INSERT. And it
|
|
stops this recursion only if it reaches a nesting level of
|
|
17, which could only occur if it is fired DURING the
|
|
execution of it's own SPI_exec(). After Vadim quouted some
|
|
SQL92 definitions about when constraint checks and triggers
|
|
are to be executed, I decided to fire regular triggers at the
|
|
end of a query too. Thus, there is absolutely no nesting
|
|
possible for AFTER triggers resulting in an endless loop.
|
|
|
|
|
|
Jan
|
|
|
|
--
|
|
|
|
#======================================================================#
|
|
# It's easier to get forgiveness for being wrong than for being right. #
|
|
# Let's break this rule - forgive me. #
|
|
#========================================= wieck@debis.com (Jan Wieck) #
|
|
|
|
|
|
|
|
************
|
|
|
|
|
|
From owner-pgsql-hackers@hub.org Thu Sep 23 11:01:06 1999
|
|
Received: from renoir.op.net (root@renoir.op.net [209.152.193.4])
|
|
by candle.pha.pa.us (8.9.0/8.9.0) with ESMTP id LAA16162
|
|
for <maillist@candle.pha.pa.us>; Thu, 23 Sep 1999 11:01:04 -0400 (EDT)
|
|
Received: from hub.org (hub.org [216.126.84.1]) by renoir.op.net (o1/$ Revision: 1.18 $) with ESMTP id KAA28544 for <maillist@candle.pha.pa.us>; Thu, 23 Sep 1999 10:45:54 -0400 (EDT)
|
|
Received: from hub.org (hub.org [216.126.84.1])
|
|
by hub.org (8.9.3/8.9.3) with ESMTP id KAA52943;
|
|
Thu, 23 Sep 1999 10:20:51 -0400 (EDT)
|
|
(envelope-from owner-pgsql-hackers@hub.org)
|
|
Received: by hub.org (TLB v0.10a (1.23 tibbs 1997/01/09 00:29:32)); Thu, 23 Sep 1999 10:19:58 +0000 (EDT)
|
|
Received: (from majordom@localhost)
|
|
by hub.org (8.9.3/8.9.3) id KAA52472
|
|
for pgsql-hackers-outgoing; Thu, 23 Sep 1999 10:19:03 -0400 (EDT)
|
|
(envelope-from owner-pgsql-hackers@postgreSQL.org)
|
|
Received: from sss.sss.pgh.pa.us (sss.pgh.pa.us [209.114.166.2])
|
|
by hub.org (8.9.3/8.9.3) with ESMTP id KAA52431
|
|
for <pgsql-hackers@postgresql.org>; Thu, 23 Sep 1999 10:18:47 -0400 (EDT)
|
|
(envelope-from tgl@sss.pgh.pa.us)
|
|
Received: from sss.sss.pgh.pa.us (localhost [127.0.0.1])
|
|
by sss.sss.pgh.pa.us (8.9.1/8.9.1) with ESMTP id KAA13253;
|
|
Thu, 23 Sep 1999 10:18:02 -0400 (EDT)
|
|
To: wieck@debis.com (Jan Wieck)
|
|
cc: pgsql-hackers@postgreSQL.org
|
|
Subject: Re: [HACKERS] Progress report: buffer refcount bugs and SQL functions
|
|
In-reply-to: Your message of Thu, 23 Sep 1999 03:19:39 +0200 (MET DST)
|
|
<m11TxXw-0003kLC@orion.SAPserv.Hamburg.dsh.de>
|
|
Date: Thu, 23 Sep 1999 10:18:01 -0400
|
|
Message-ID: <13251.938096281@sss.pgh.pa.us>
|
|
From: Tom Lane <tgl@sss.pgh.pa.us>
|
|
Sender: owner-pgsql-hackers@postgreSQL.org
|
|
Precedence: bulk
|
|
Status: RO
|
|
|
|
wieck@debis.com (Jan Wieck) writes:
|
|
> Tom Lane wrote:
|
|
>> What I am wondering, though, is whether this addition is actually
|
|
>> necessary, or is it a bug that the functions aren't run to completion
|
|
>> in the first place?
|
|
|
|
> I've said some time (maybe too long) ago, that SQL functions
|
|
> returning tuple sets are broken in general.
|
|
|
|
Indeed they are. Try this on for size (using the regression database):
|
|
|
|
SELECT p.name, p.hobbies.equipment.name FROM person p;
|
|
SELECT p.hobbies.equipment.name, p.name FROM person p;
|
|
|
|
You get different result sets!?
|
|
|
|
The problem in this example is that ExecTargetList returns the isDone
|
|
flag from the last targetlist entry, regardless of whether there are
|
|
incomplete iterations in previous entries. More generally, the buffer
|
|
leak problem that I started with only occurs if some Iter nodes are not
|
|
run to completion --- but execQual.c has no mechanism to make sure that
|
|
they have all reached completion simultaneously.
|
|
|
|
What we really need to make functions-returning-sets work properly is
|
|
an implementation somewhat like aggregate functions. We need to make
|
|
a list of all the Iter nodes present in a targetlist and cycle through
|
|
the values returned by each in a methodical fashion (run the rightmost
|
|
through its full cycle, then advance the next-to-rightmost one value,
|
|
run the rightmost through its cycle again, etc etc). Also there needs
|
|
to be an understanding of the hierarchy when an Iter appears in the
|
|
arguments of another Iter's function. (You cycle the upper one for
|
|
*each* set of arguments created by cycling its sub-Iters.)
|
|
|
|
I am not particularly interested in working on this feature right now,
|
|
since AFAIK it's a Berkeleyism not found in SQL92. What I've done
|
|
is to hack ExecTargetList so that it behaves semi-sanely when there's
|
|
more than one Iter at the top level of the target list --- it still
|
|
doesn't really give the right answer, but at least it will keep
|
|
generating tuples until all the Iters are done at the same time.
|
|
It happens that that's enough to give correct answers for the examples
|
|
shown in the misc regress test. Even when it fails to generate all
|
|
the possible combinations, there will be no buffer leaks.
|
|
|
|
So, I'm going to declare victory and go home ;-). We ought to add a
|
|
TODO item along the lines of
|
|
* Functions returning sets don't really work right
|
|
in hopes that someone will feel like tackling this someday.
|
|
|
|
regards, tom lane
|
|
|
|
************
|
|
|
|
|