Re: PL/PGSQL: Dynamic Record Introspection

OK, patch reverted, and attached. Would the author please revise? 
Thanks.

It seems like a cool feature.

---------------------------------------------------------------------------

Tom Lane wrote:
> 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
> 

-- 
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +