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 |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers pgsql-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
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2006-05-30 13:39:18 | Re: PL/PGSQL: Dynamic Record Introspection |
Previous Message | Andy Shellam | 2006-05-30 13:35:27 | Re: Problem building initdb on sparc10 |
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2006-05-30 13:39:18 | Re: PL/PGSQL: Dynamic Record Introspection |
Previous Message | Bruce Momjian | 2006-05-30 13:30:13 | Re: archiver.pid |