Horton

Thread

Commits

  1. Add a way to get the current function's OID in pl/pgsql.

  1. possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

    Pavel Stehule <pavel.stehule@gmail.com> — 2023-02-07T19:48:22Z

    Hi

I have a question about the possibility of simply getting the name of the
currently executed function. The reason for this request is simplification
of writing debug messages.

GET DIAGNOSTICS _oid = PG_ROUTINE_OID;
RAISE NOTICE '... % ... %', _oid, _oid::regproc::text;

The advantage of this dynamic access to function name is always valid value
not sensitive to some renaming or moving between schemas.

I am able to separate a name from context, but it can be harder to write
this separation really robustly. It can be very easy to enhance the GET
DIAGNOSTICS statement to return the oid of currently executed function.

Do you think it can be useful feature?

The implementation should be trivial.

Comments, notes?

Regards

Pavel

  2. Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

    Kirk Wolak <wolakk@gmail.com> — 2023-02-07T21:08:02Z

    On Tue, Feb 7, 2023 at 2:49 PM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

> Hi
>
> I have a question about the possibility of simply getting the name of the
> currently executed function. The reason for this request is simplification
> of writing debug messages.
>
> GET DIAGNOSTICS _oid = PG_ROUTINE_OID;
> RAISE NOTICE '... % ... %', _oid, _oid::regproc::text;
>
> The advantage of this dynamic access to function name is always valid
> value not sensitive to some renaming or moving between schemas.
>
> I am able to separate a name from context, but it can be harder to write
> this separation really robustly. It can be very easy to enhance the GET
> DIAGNOSTICS statement to return the oid of currently executed function.
>
> Do you think it can be useful feature?
>

I was hoping it could be a CONSTANT like TG_OP (so the extra GET
DIAGNOSTICS wasn't invoked, but I have no idea the weight of that CODE
CHANGE)

Regardless, this concept is what we are looking for.  We prefer to leave
some debugging scaffolding in our DB Procedures, but disable it by default.
We are looking for a way to add something like this as a filter on the
level of output.

Our Current USE CASE is
  CALL LOGGING('Msg');  -- And by default nothing happens, unless we set
some session variables appropriately

We are looking for
  CALL LOGGING('Msg', __PG_ROUTINE_OID );  -- Now we can enable logging by
the routine we are interested in!

The LOGGING routine currently checks a session variable to see if logging
is EVEN Desired, if not it exits (eg PRODUCTION).

Now we can add a single line check, if p_funcoid  is IN my list of routines
I am debugging, send the output.

I will gladly work on the documentation side to help this happen!

+10




>
> The implementation should be trivial.
>
> Comments, notes?
>
> Regards
>
> Pavel
>
>
>

  3. Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

    Julien Rouhaud <rjuju123@gmail.com> — 2023-02-08T06:33:16Z

    On Tue, Feb 07, 2023 at 08:48:22PM +0100, Pavel Stehule wrote:
>
> I have a question about the possibility of simply getting the name of the
> currently executed function. The reason for this request is simplification
> of writing debug messages.
>
> GET DIAGNOSTICS _oid = PG_ROUTINE_OID;
> RAISE NOTICE '... % ... %', _oid, _oid::regproc::text;
>
> The advantage of this dynamic access to function name is always valid value
> not sensitive to some renaming or moving between schemas.
>
> I am able to separate a name from context, but it can be harder to write
> this separation really robustly. It can be very easy to enhance the GET
> DIAGNOSTICS statement to return the oid of currently executed function.
>
> Do you think it can be useful feature?

+1, it would have been quite handy in a few of my projects.

  4. Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

    Pavel Stehule <pavel.stehule@gmail.com> — 2023-02-08T08:07:27Z

    hi



st 8. 2. 2023 v 7:33 odesílatel Julien Rouhaud <rjuju123@gmail.com> napsal:

> On Tue, Feb 07, 2023 at 08:48:22PM +0100, Pavel Stehule wrote:
> >
> > I have a question about the possibility of simply getting the name of the
> > currently executed function. The reason for this request is
> simplification
> > of writing debug messages.
> >
> > GET DIAGNOSTICS _oid = PG_ROUTINE_OID;
> > RAISE NOTICE '... % ... %', _oid, _oid::regproc::text;
> >
> > The advantage of this dynamic access to function name is always valid
> value
> > not sensitive to some renaming or moving between schemas.
> >
> > I am able to separate a name from context, but it can be harder to write
> > this separation really robustly. It can be very easy to enhance the GET
> > DIAGNOSTICS statement to return the oid of currently executed function.
> >
> > Do you think it can be useful feature?
>
> +1, it would have been quite handy in a few of my projects.
>

it can looks like that

create or replace function foo(a int)
returns int as $$
declare s text; n text; o oid;
begin
  get diagnostics s = pg_current_routine_signature,
                  n = pg_current_routine_name,
                  o = pg_current_routine_oid;
  raise notice 'sign:%,  name:%,  oid:%', s, n, o;
  return a;
end;
$$ language plpgsql;
CREATE FUNCTION
(2023-02-08 09:04:03) postgres=# select foo(10);
NOTICE:  sign:foo(integer),  name:foo,  oid:16392
┌─────┐
│ foo │
╞═════╡
│  10 │
└─────┘
(1 row)

The name - pg_routine_oid can be confusing, because there is not clean if
it is oid of currently executed routine or routine from top of exception

Regards

Pavel

  5. Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

    Kirk Wolak <wolakk@gmail.com> — 2023-02-08T15:56:04Z

    On Wed, Feb 8, 2023 at 3:08 AM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

> hi
>
> st 8. 2. 2023 v 7:33 odesílatel Julien Rouhaud <rjuju123@gmail.com>
> napsal:
>
>> On Tue, Feb 07, 2023 at 08:48:22PM +0100, Pavel Stehule wrote:
>> >
>> > GET DIAGNOSTICS _oid = PG_ROUTINE_OID;
>> > RAISE NOTICE '... % ... %', _oid, _oid::regproc::text;
>> >
>> > Do you think it can be useful feature?
>>
>> +1, it would have been quite handy in a few of my projects.
>>
>
> it can looks like that
>
> create or replace function foo(a int)
> returns int as $$
> declare s text; n text; o oid;
> begin
>   get diagnostics s = pg_current_routine_signature,
>                   n = pg_current_routine_name,
>                   o = pg_current_routine_oid;
>   raise notice 'sign:%,  name:%,  oid:%', s, n, o;
>   return a;
> end;
> $$ language plpgsql;
> CREATE FUNCTION
> (2023-02-08 09:04:03) postgres=# select foo(10);
> NOTICE:  sign:foo(integer),  name:foo,  oid:16392
> ┌─────┐
> │ foo │
> ╞═════╡
> │  10 │
> └─────┘
> (1 row)
>
> The name - pg_routine_oid can be confusing, because there is not clean if
> it is oid of currently executed routine or routine from top of exception
>
> Regards
>
> Pavel
>

I agree that the name changed to pg_current_routine_...  makes the most
sense, great call...

+1

  6. Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

    Kirk Wolak <wolakk@gmail.com> — 2023-03-26T21:37:36Z

    On Wed, Feb 8, 2023 at 10:56 AM Kirk Wolak <wolakk@gmail.com> wrote:

> On Wed, Feb 8, 2023 at 3:08 AM Pavel Stehule <pavel.stehule@gmail.com>
> wrote:
>
>> hi
>>
>> st 8. 2. 2023 v 7:33 odesílatel Julien Rouhaud <rjuju123@gmail.com>
>> napsal:
>>
>>> On Tue, Feb 07, 2023 at 08:48:22PM +0100, Pavel Stehule wrote:
>>> >
>>> > GET DIAGNOSTICS _oid = PG_ROUTINE_OID;
>>> > RAISE NOTICE '... % ... %', _oid, _oid::regproc::text;
>>> >
>>> > Do you think it can be useful feature?
>>>
>>> +1, it would have been quite handy in a few of my projects.
>>>
>>
>> it can looks like that
>>
>> create or replace function foo(a int)
>> returns int as $$
>> declare s text; n text; o oid;
>> begin
>>   get diagnostics s = pg_current_routine_signature,
>>                   n = pg_current_routine_name,
>>                   o = pg_current_routine_oid;
>>   raise notice 'sign:%,  name:%,  oid:%', s, n, o;
>>   return a;
>> end;
>> $$ language plpgsql;
>> CREATE FUNCTION
>> (2023-02-08 09:04:03) postgres=# select foo(10);
>> NOTICE:  sign:foo(integer),  name:foo,  oid:16392
>> ┌─────┐
>> │ foo │
>> ╞═════╡
>> │  10 │
>> └─────┘
>> (1 row)
>>
>> The name - pg_routine_oid can be confusing, because there is not clean if
>> it is oid of currently executed routine or routine from top of exception
>>
>> Regards
>>
>> Pavel
>>
>
> I agree that the name changed to pg_current_routine_...  makes the most
> sense, great call...
>
> +1
>

Okay, I reviewed this.  I tested it (allocating too small of
varchar's for values, various "signature types"),
and also a performance test... Wow, on my VM, 10,000 Calls in a loop was
2-4ms...

The names are clear.  Again, I tested with various options, and including
ROW_COUNT, or not.

This functions PERFECTLY....  Except there are no documentation changes.
Because of that, I set it to Waiting on Author.
Which might be unfair, because I could take a stab at doing the
documentation (but docs are not compiling on my setup yet).

The documentation changes are simple enough.
If I can get the docs compiled on my rig, I will see if I can make the
changes, and post an updated patch,
that contains both...

But I don't want to be stepping on toes, or having it look like I am taking
credit.

Regards - Kirk

  7. Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

    Kirk Wolak <wolakk@gmail.com> — 2023-03-27T03:36:10Z

    On Sun, Mar 26, 2023 at 5:37 PM Kirk Wolak <wolakk@gmail.com> wrote:

> On Wed, Feb 8, 2023 at 10:56 AM Kirk Wolak <wolakk@gmail.com> wrote:
>
>> On Wed, Feb 8, 2023 at 3:08 AM Pavel Stehule <pavel.stehule@gmail.com>
>> wrote:
>>
>>> hi
>>>
>>> st 8. 2. 2023 v 7:33 odesílatel Julien Rouhaud <rjuju123@gmail.com>
>>> napsal:
>>>
>>>> On Tue, Feb 07, 2023 at 08:48:22PM +0100, Pavel Stehule wrote:
>>>> >
>>>> > GET DIAGNOSTICS _oid = PG_ROUTINE_OID;
>>>> > RAISE NOTICE '... % ... %', _oid, _oid::regproc::text;
>>>> >
>>>> > Do you think it can be useful feature?
>>>>
>>>> +1, it would have been quite handy in a few of my projects.
>>>>
>>>
>>> it can looks like that
>>>
>>> create or replace function foo(a int)
>>> returns int as $$
>>> declare s text; n text; o oid;
>>> begin
>>>   get diagnostics s = pg_current_routine_signature,
>>>                   n = pg_current_routine_name,
>>>                   o = pg_current_routine_oid;
>>>   raise notice 'sign:%,  name:%,  oid:%', s, n, o;
>>>   return a;
>>> end;
>>> $$ language plpgsql;
>>> CREATE FUNCTION
>>> (2023-02-08 09:04:03) postgres=# select foo(10);
>>> NOTICE:  sign:foo(integer),  name:foo,  oid:16392
>>> ┌─────┐
>>> │ foo │
>>> ╞═════╡
>>> │  10 │
>>> └─────┘
>>> (1 row)
>>>
>>> The name - pg_routine_oid can be confusing, because there is not clean
>>> if it is oid of currently executed routine or routine from top of exception
>>>
>>> Regards
>>>
>>> Pavel
>>>
>>
>> I agree that the name changed to pg_current_routine_...  makes the most
>> sense, great call...
>>
>> +1
>>
>
> Okay, I reviewed this.  I tested it (allocating too small of
> varchar's for values, various "signature types"),
> and also a performance test... Wow, on my VM, 10,000 Calls in a loop was
> 2-4ms...
>
> The names are clear.  Again, I tested with various options, and including
> ROW_COUNT, or not.
>
> This functions PERFECTLY....  Except there are no documentation changes.
> Because of that, I set it to Waiting on Author.
> Which might be unfair, because I could take a stab at doing the
> documentation (but docs are not compiling on my setup yet).
>
> The documentation changes are simple enough.
> If I can get the docs compiled on my rig, I will see if I can make the
> changes, and post an updated patch,
> that contains both...
>
> But I don't want to be stepping on toes, or having it look like I am
> taking credit.
>
> Regards - Kirk
>

Okay, I have modified the documentation and made sure it compiles.  They
were simple enough changes.
I am attaching this updated patch.

I have marked the item Ready for Commiter...

Thanks for your patience.  I now have a workable hacking environment!

Regards - Kirk

  8. Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

    Pavel Stehule <pavel.stehule@gmail.com> — 2023-03-27T06:29:29Z

    Hi


po 27. 3. 2023 v 5:36 odesílatel Kirk Wolak <wolakk@gmail.com> napsal:

> On Sun, Mar 26, 2023 at 5:37 PM Kirk Wolak <wolakk@gmail.com> wrote:
>
>> On Wed, Feb 8, 2023 at 10:56 AM Kirk Wolak <wolakk@gmail.com> wrote:
>>
>>> On Wed, Feb 8, 2023 at 3:08 AM Pavel Stehule <pavel.stehule@gmail.com>
>>> wrote:
>>>
>>>> hi
>>>>
>>>> st 8. 2. 2023 v 7:33 odesílatel Julien Rouhaud <rjuju123@gmail.com>
>>>> napsal:
>>>>
>>>>> On Tue, Feb 07, 2023 at 08:48:22PM +0100, Pavel Stehule wrote:
>>>>> >
>>>>> > GET DIAGNOSTICS _oid = PG_ROUTINE_OID;
>>>>> > RAISE NOTICE '... % ... %', _oid, _oid::regproc::text;
>>>>> >
>>>>> > Do you think it can be useful feature?
>>>>>
>>>>> +1, it would have been quite handy in a few of my projects.
>>>>>
>>>>
>>>> it can looks like that
>>>>
>>>> create or replace function foo(a int)
>>>> returns int as $$
>>>> declare s text; n text; o oid;
>>>> begin
>>>>   get diagnostics s = pg_current_routine_signature,
>>>>                   n = pg_current_routine_name,
>>>>                   o = pg_current_routine_oid;
>>>>   raise notice 'sign:%,  name:%,  oid:%', s, n, o;
>>>>   return a;
>>>> end;
>>>> $$ language plpgsql;
>>>> CREATE FUNCTION
>>>> (2023-02-08 09:04:03) postgres=# select foo(10);
>>>> NOTICE:  sign:foo(integer),  name:foo,  oid:16392
>>>> ┌─────┐
>>>> │ foo │
>>>> ╞═════╡
>>>> │  10 │
>>>> └─────┘
>>>> (1 row)
>>>>
>>>> The name - pg_routine_oid can be confusing, because there is not clean
>>>> if it is oid of currently executed routine or routine from top of exception
>>>>
>>>> Regards
>>>>
>>>> Pavel
>>>>
>>>
>>> I agree that the name changed to pg_current_routine_...  makes the most
>>> sense, great call...
>>>
>>> +1
>>>
>>
>> Okay, I reviewed this.  I tested it (allocating too small of
>> varchar's for values, various "signature types"),
>> and also a performance test... Wow, on my VM, 10,000 Calls in a loop was
>> 2-4ms...
>>
>> The names are clear.  Again, I tested with various options, and including
>> ROW_COUNT, or not.
>>
>> This functions PERFECTLY....  Except there are no documentation changes.
>> Because of that, I set it to Waiting on Author.
>> Which might be unfair, because I could take a stab at doing the
>> documentation (but docs are not compiling on my setup yet).
>>
>> The documentation changes are simple enough.
>> If I can get the docs compiled on my rig, I will see if I can make the
>> changes, and post an updated patch,
>> that contains both...
>>
>> But I don't want to be stepping on toes, or having it look like I am
>> taking credit.
>>
>> Regards - Kirk
>>
>
> Okay, I have modified the documentation and made sure it compiles.  They
> were simple enough changes.
> I am attaching this updated patch.
>
> I have marked the item Ready for Commiter...
>

Thank you for doc and for review

Regards

Pavel


>
> Thanks for your patience.  I now have a workable hacking environment!
>
> Regards - Kirk
>
>

  9. Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-04-03T17:37:31Z

    Pavel Stehule <pavel.stehule@gmail.com> writes:
> po 27. 3. 2023 v 5:36 odesílatel Kirk Wolak <wolakk@gmail.com> napsal:
>> I have marked the item Ready for Commiter...

> Thank you for doc and for review

I'm kind of surprised there was any interest in this proposal at all,
TBH, but apparently there is some.  Still, I think you over-engineered
it by doing more than the original proposal of making the function OID
available.  The other things can be had by casting the OID to regproc
or regprocedure, so I'd be inclined to add just one new keyword not
three.  Besides, your implementation is a bit inconsistent: relying
on fn_signature could return a result that is stale or doesn't conform
to the current search_path.

			regards, tom lane

  10. Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

    Pavel Stehule <pavel.stehule@gmail.com> — 2023-04-03T18:49:35Z

    Hi


po 3. 4. 2023 v 19:37 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

> Pavel Stehule <pavel.stehule@gmail.com> writes:
> > po 27. 3. 2023 v 5:36 odesílatel Kirk Wolak <wolakk@gmail.com> napsal:
> >> I have marked the item Ready for Commiter...
>
> > Thank you for doc and for review
>
> I'm kind of surprised there was any interest in this proposal at all,
> TBH, but apparently there is some.  Still, I think you over-engineered
> it by doing more than the original proposal of making the function OID
> available.  The other things can be had by casting the OID to regproc
> or regprocedure, so I'd be inclined to add just one new keyword not
> three.  Besides, your implementation is a bit inconsistent: relying
> on fn_signature could return a result that is stale or doesn't conform
> to the current search_path.
>

ok

There is reduced patch + regress tests

Regards

Pavel



>                         regards, tom lane
>

  11. Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-04-04T14:20:00Z

    Pavel Stehule <pavel.stehule@gmail.com> writes:
> There is reduced patch + regress tests

One more thing: I do not think it's appropriate to allow this in
GET STACKED DIAGNOSTICS.  That's about reporting the place where
an error occurred, not the current location.  Eventually it might
be interesting to retrieve the OID of the function that contained
the error, but that would be a pretty complicated patch and I am
not sure it's worth it.  In the meantime I think we should just
forbid it.

If we do that, then the confusion you were concerned about upthread
goes away and we could shorten the keyword back down to "pg_routine_oid",
which seems like a good thing for our carpal tunnels.

Thoughts?

			regards, tom lane

  12. Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

    Pavel Stehule <pavel.stehule@gmail.com> — 2023-04-04T16:57:12Z

    út 4. 4. 2023 v 16:20 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

> Pavel Stehule <pavel.stehule@gmail.com> writes:
> > There is reduced patch + regress tests
>
> One more thing: I do not think it's appropriate to allow this in
> GET STACKED DIAGNOSTICS.  That's about reporting the place where
> an error occurred, not the current location.  Eventually it might
> be interesting to retrieve the OID of the function that contained
> the error, but that would be a pretty complicated patch and I am
> not sure it's worth it.  In the meantime I think we should just
> forbid it.
>
> If we do that, then the confusion you were concerned about upthread
> goes away and we could shorten the keyword back down to "pg_routine_oid",
> which seems like a good thing for our carpal tunnels.
>
> Thoughts?
>

has sense

updated patch attached

Regards

Pavel

>
>                         regards, tom lane
>

  13. Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-04-04T17:34:22Z

    Pavel Stehule <pavel.stehule@gmail.com> writes:
> út 4. 4. 2023 v 16:20 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:
>> If we do that, then the confusion you were concerned about upthread
>> goes away and we could shorten the keyword back down to "pg_routine_oid",
>> which seems like a good thing for our carpal tunnels.

> has sense

OK, pushed like that with some cosmetic adjustments (better test
case, mostly).

			regards, tom lane

  14. Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

    Pavel Stehule <pavel.stehule@gmail.com> — 2023-04-04T17:41:53Z

    út 4. 4. 2023 v 19:34 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

> Pavel Stehule <pavel.stehule@gmail.com> writes:
> > út 4. 4. 2023 v 16:20 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:
> >> If we do that, then the confusion you were concerned about upthread
> >> goes away and we could shorten the keyword back down to
> "pg_routine_oid",
> >> which seems like a good thing for our carpal tunnels.
>
> > has sense
>
> OK, pushed like that with some cosmetic adjustments (better test
> case, mostly).
>

Thank you very much

Regards

Pavel


>
>                         regards, tom lane
>