Thread: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

From
Pavel Stehule
Date:
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


Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

From
Kirk Wolak
Date:


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


Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

From
Julien Rouhaud
Date:
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.



Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

From
Pavel Stehule
Date:
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
Attachment

Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

From
Kirk Wolak
Date:
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   

Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

From
Kirk Wolak
Date:
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

Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

From
Kirk Wolak
Date:
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
 
Attachment

Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

From
Pavel Stehule
Date:
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
 

Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

From
Tom Lane
Date:
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



Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

From
Pavel Stehule
Date:
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
Attachment

Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

From
Tom Lane
Date:
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



Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

From
Pavel Stehule
Date:


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

Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

From
Tom Lane
Date:
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



Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID

From
Pavel Stehule
Date:


ú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