Bruce Momjian <pgman@candle.pha.pa.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 ---------------------------(end of broadcast)--------------------------- TIP 5: don't forget to increase your free space map settings