Re: pl/pgperl Patch for adding $_FN detail just like triggers have for $_TD

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Mark Murawski <markm-lists(at)intellasoft(dot)net>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: pl/pgperl Patch for adding $_FN detail just like triggers have for $_TD
Date: 2024-08-30 19:49:57
Message-ID: e4f8c4c3-e98f-45ef-a579-15ee76f75be5@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On 2024-08-29 Th 5:50 PM, Mark Murawski wrote:
>
>
> On 8/29/24 16:54, Andrew Dunstan wrote:
>>
>> On 2024-08-29 Th 1:01 PM, Mark Murawski wrote:
>>>
>>>
>>> On 8/29/24 11:56, Andrew Dunstan wrote:
>>>>
>>>> On 2024-08-28 We 5:53 PM, Mark Murawski wrote:
>>>>> Hi Hackers!
>>>>>
>>>>> This would be version v1 of this feature
>>>>>
>>>>> Basically, the subject says it all: pl/pgperl Patch for being able
>>>>> to tell which function you're in.
>>>>> This is a hashref so it will be possible to populate new and
>>>>> exciting other details in the future as the need arises
>>>>>
>>>>> This also greatly improves logging capabilities for things like
>>>>> catching warnings,  Because as it stands right now, there's no
>>>>> information that can assist with locating the source of a warning
>>>>> like this:
>>>>>
>>>>> # tail -f /var/log/postgresql.log
>>>>> ******* GOT A WARNING - Use of uninitialized value $prefix in
>>>>> concatenation (.) or string at (eval 531) line 48.
>>>>>
>>>>> Now, with $_FN you can do this:
>>>>>
>>>>>
>>>>> CREATE OR REPLACE FUNCTION throw_warning() RETURNS text LANGUAGE
>>>>> plperlu AS $function$
>>>>>
>>>>> use warnings;
>>>>> use strict;
>>>>> use Data::Dumper;
>>>>>
>>>>> $SIG{__WARN__} = sub {
>>>>>   elog(NOTICE, Dumper($_FN));
>>>>>
>>>>>   print STDERR "In Function: $_FN->{name}: $_[0]\n";
>>>>> };
>>>>>
>>>>> my $a;
>>>>> print "$a"; # uninit!
>>>>>
>>>>> return undef;
>>>>>
>>>>> $function$
>>>>> ;
>>>>>
>>>>> This patch is against 12 which is still our production branch.
>>>>> This could easily be also patched against newer releases as well.
>>>>>
>>>>> I've been using this code in production now for about 3 years,
>>>>> it's greatly helped track down issues.  And there shouldn't be
>>>>> anything platform-specific here, it's all regular perl API
>>>>>
>>>>> I'm not sure about adding testing.  This is my first postgres
>>>>> patch, so any guidance on adding regression testing would be
>>>>> appreciated.
>>>>>
>>>>> The rationale for this has come from the need to know the source
>>>>> function name, and we've typically resorted to things like this in
>>>>> the past:
>>>>>
>>>>> CREATE OR REPLACE FUNCTION throw_warning() RETURNS text LANGUAGE
>>>>> plperlu AS $function$
>>>>> my $function_name = 'throw_warning';
>>>>> $SIG{__WARN__} = sub { print STDERR "In Function: $function_name:
>>>>> $_[0]\n"; }
>>>>> $function$
>>>>> ;
>>>>>
>>>>> We've literally had to copy/paste this all over and it's something
>>>>> that postgres should just 'give you' since it knows the name
>>>>> already, just like when triggers pass you $_TD with all the
>>>>> pertinent information
>>>>>
>>>>> A wishlist item would be for postgres plperl to automatically
>>>>> prepend the function name and schema when throwing perl warnings
>>>>> so you don't have to do your own __WARN__ handler, but this is the
>>>>> next best thing.
>>>>
>>>>
>>>>
>>>> I'm not necessarily opposed to this, but the analogy to $_TD isn't
>>>> really apt.  You can't know the trigger data at compile time,
>>>> whereas you can know the function's name at compile time, using
>>>> just the mechanism you find irksome.
>>>>
>>>> And if we're going to do it for plperl, shouldn't we do it for
>>>> other PLs?
>>>>
>>>>
>>>>
>>>
>>> Hi Andrew,
>>>
>>>
>>> Thanks for the feedback.
>>>
>>>
>>> 1) Why is this not similar to _TD?  It literally operates
>>> identically. At run-time it passes you $_TD  for triggers. Same her
>>> for functions.  This is all run-time.   What exactly is the issue
>>> you're trying to point out?
>>
>>
>> It's not the same as the trigger data case because the function name
>> is knowable at compile time, as in fact you have demonstrated. You
>> just find it a bit inconvenient to have to code for that knowledge.
>> By contrast, trigger data is ONLY knowable at run time.
>>
>> But I don't see that it is such a heavy burden to have to write
>>
>>   my $funcname = "foo";
>>
>> or similar in your function.
>>
>>
>> cheers
>>
>>
>> andrew
>>
>> --
>> Andrew Dunstan
>> EDB: https://www.enterprisedb.com
>>
>>
>>
>
> Understood, regarding knowability.  Trigger data is definitely going
> to be very dynamic in that regard.
>
> No, It's not a heavy burden to hard code the function name.  But what
> my ideal goal would be is this:
>
> CREATE OR REPLACE FUNCTION throw_warning() RETURNS text LANGUAGE
> plperlu AS $function$
> use 'PostgresWarnHandler'; # <--- imagine this registers a WARN
> handler and outputs $_FN->{name} for you as part of the final warning
>
> my $a;
> print $a;
>
> .... etc
>
>
> and then there's nothing 'hard coded' regarding the name of the
> function, anywhere.  It just seems nonsensical that postgres plperl
> can't send you the name of your registered function and there's
> absolutely no way to get it other than hard coding the name
> (redundantly, considering you're already know the name when you're
> defining the function in the first place)
>
> But even better would be catching the warn at the plperl level,
> prepending the function name to the warn message, and then you only need:
>
> CREATE OR REPLACE FUNCTION throw_warning() RETURNS text LANGUAGE
> plperlu AS $function$
>
> my $a;
> print $a;
>
> .... etc
>
> And then in this hypothetical situation -- magic ensues, and you're
> left with this:
> # tail -f /var/log/postgresql.log
> ******* GOT A WARNING - Use of uninitialized value $a in concatenation
> (.) or string in function public.throw_warning() line 1
>
>
>
>
>
>
>
--
Andrew Dunstan
EDB: https://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2024-08-30 20:04:42 Re: allowing extensions to control planner behavior
Previous Message Jelte Fennema-Nio 2024-08-30 19:49:44 Re: [EXTERNAL] Re: Add non-blocking version of PQcancel