Skip site navigation (1) Skip section navigation (2)

Re: PL/PGSQL: Dynamic Record Introspection

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: uol(at)freenet(dot)de, Neil Conway <neilc(at)samurai(dot)com>, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: PL/PGSQL: Dynamic Record Introspection
Date: 2006-05-30 13:35:38
Message-ID: 23092.1148996138@sss.pgh.pa.us (view raw or flat)
Thread:
Lists: pgsql-hackerspgsql-patches
Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Patch applied.  Thanks.

I wish to object to this patch; it's poorly designed, poorly coded, and
badly documented.  The philosophy seems to be "I want this feature
and I don't care what I have to do to the semantics and performance
of plpgsql to get it".  In particular I don't like the bits that go

                 /* Do not allow typeids to become "narrowed" by InvalidOids 
                 causing specialized typeids from the tuple restricting the destination */

The incoherence of the comment is a good guide to how poorly thought out
the code is.  These bits are positioned so that they change the language
semantics even when not using a dynamic field reference; what's more,
they don't make any sense when you *are* using a dynamic field
reference, because you need to keep the actual field type not try
(probably vainly) to cast it to whatever the previous field's type was.

I also dislike the loop added to exec_eval_cleanup(), which will impose
a significant performance penalty on every single expression evaluation
done by any plpgsql function, whether it's ever heard of dynamic field
references or not.  I doubt it's even necessary; I think the author
doesn't understand plpgsql's memory allocation strategy very well.

Lastly, and this is maybe a judgement call, I find the changes to
PLpgSQL_recfield fairly ugly.  It'd be better to make a separate
datum type PLpgSQL_dynamic_recfield or some such.

This needs to be bounced back for rework.  It looks like a first draft
that should have been rewritten once the author had learned enough to
make it sort-of-work.

			regards, tom lane

In response to

Responses

pgsql-hackers by date

Next:From: Bruce MomjianDate: 2006-05-30 13:39:18
Subject: Re: PL/PGSQL: Dynamic Record Introspection
Previous:From: Andy ShellamDate: 2006-05-30 13:35:27
Subject: Re: Problem building initdb on sparc10

pgsql-patches by date

Next:From: Bruce MomjianDate: 2006-05-30 13:39:18
Subject: Re: PL/PGSQL: Dynamic Record Introspection
Previous:From: Bruce MomjianDate: 2006-05-30 13:30:13
Subject: Re: archiver.pid

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group