No, the author would not.

Tom did participate in the discussion at the time
when the original patch was developed.

The patch was now hanging around more than 9 months,
and it was already accepted during the
original discussion by Neil Conway. Please figure out
by yourselves who might be the one to really, really, finally
accept the patch.

A word about the discussion style in general:
I personally don't like general objections on the code
*after* I - again - spent some hours of work to comply to Bruce's
wish to rewrite the patch for HEAD. After all, Tom, you
don't pay me?
And I don't like quoting my comments while at the same
time complaining about "poorly coded" software. This simply is nonsense.
The other attributes you gave to the patch are simply
bad discussion style and at the same time nonsense as well:
The patch tries to *minimize* changes to plpgsql and does not
deal with "fundamental" problems of the interpreter.
For this reason, of course the patch is more or less a hack.

Apparently, the RECORD construct without that patch
does not really make much sense; ROWTYPE would suffice.
This is why the patch is a "cool feature".

If now Tom or the community (being identical?)
in general have the feeling
to start some kind of complete or otherwise "fundamental"
rewrite of plpgsql to handle constructs like the one implemented by me,
I suggest someone else should do that. I do not have
the time to rewrite plpgsql or to participate
in this kind of discussions.


Bruce Momjian wrote:
OK, patch reverted, and attached. Would the author please revise? Thanks.

It seems like a cool feature.


Tom Lane wrote:
Bruce Momjian <> 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 3: Have you checked our extensive FAQ?


Reply via email to