Thread: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID
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
On Tue, Feb 7, 2023 at 2:49 PM Pavel Stehule <pavel.stehule@gmail.com> wrote:
HiI 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?RegardsPavel
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.
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)
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
On Wed, Feb 8, 2023 at 3:08 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:
hist 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 thatcreate 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 exceptionRegardsPavel
I agree that the name changed to pg_current_routine_... makes the most sense, great call...
+1
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:hist 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 thatcreate 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 exceptionRegardsPavelI 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
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:hist 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 thatcreate 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 exceptionRegardsPavelI agree that the name changed to pg_current_routine_... makes the most sense, great call...+1Okay, 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!
Thanks for your patience. I now have a workable hacking environment!
Regards - Kirk
Attachment
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:hist 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 thatcreate 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 exceptionRegardsPavelI agree that the name changed to pg_current_routine_... makes the most sense, great call...+1Okay, 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 - KirkOkay, 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
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
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
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
ú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
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
ú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