Re: PL/PGSQL: Dynamic Record Introspection

From: uol(at)freenet(dot)de
To: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, 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 15:23:42
Message-ID: 447C637E.3050809@freenet.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

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.

Regards
Titus

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 <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

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2006-05-30 15:29:20 Re: Problem building initdb on sparc10
Previous Message Andrew Dunstan 2006-05-30 15:16:59 Re: plperl's ppport.h out of date?

Browse pgsql-patches by date

  From Date Subject
Next Message Andrew Dunstan 2006-05-30 16:41:39 Re: [PATCH] Add support for GnuTLS
Previous Message Tom Lane 2006-05-30 14:13:00 Re: PL/PGSQL: Dynamic Record Introspection