Обсуждение: pg_identify_object_as_address() doesn't support pg_event_trigger oids

On 2021-Apr-22, Joel Jacobson wrote:

> Is $SUBJECT intentional, or would it be desirable to add support it?
> 
> Example:
> 
> SELECT * FROM pg_catalog.pg_event_trigger;
>     oid    |    evtname    |    evtevent     | evtowner |  evtfoid  | evtenabled | evttags
> -----------+---------------+-----------------+----------+-----------+------------+---------
> 289361636 | ddl_postgrest | ddl_command_end |    16696 | 289361635 | O          |
> (1 row)
> 
> SELECT * FROM pg_identify_object_as_address('pg_event_trigger'::regclass,289361636,0);
> ERROR:  requested object address for unsupported object class 32: text result "ddl_postgrest"

Hmm, I think this is an accidental omission and it should be supported.

-- 
Álvaro Herrera                            39°49'30"S 73°17'W
"In Europe they call me Niklaus Wirth; in the US they call me Nickel's worth.
 That's because in Europe they call me by name, and in the US by value!"

On 2021-Apr-22, Joel Jacobson wrote:

> SELECT * FROM pg_identify_object_as_address('pg_event_trigger'::regclass,289361636,0);

> ERROR:  requested object address for unsupported object class 32: text result "ddl_postgrest"

Hmm, I think this is an accidental omission and it should be supported.

pg_describe_object            | event trigger ddl_postgrest

pg_identify_object            | ("event trigger",,ddl_postgrest,ddl_postgrest)

pg_identify_object_as_address | ("event trigger",{},{})

I therefore think the evtname should be added to object_names.

fix-pg_identify_object_as_address-for-event-triggers.patch

On 2021-Apr-22, Joel Jacobson wrote:

> Is $SUBJECT intentional, or would it be desirable to add support it?

> 

> Example:

> 

> SELECT * FROM pg_catalog.pg_event_trigger;

>     oid    |    evtname    |    evtevent     | evtowner |  evtfoid  | evtenabled | evttags

> -----------+---------------+-----------------+----------+-----------+------------+---------

> 289361636 | ddl_postgrest | ddl_command_end |    16696 | 289361635 | O          |

> (1 row)

> 

> SELECT * FROM pg_identify_object_as_address('pg_event_trigger'::regclass,289361636,0);

> ERROR:  requested object address for unsupported object class 32: text result "ddl_postgrest"

Hmm, I think this is an accidental omission and it should be supported.

On Fri, Apr 23, 2021 at 09:33:36AM +0200, Joel Jacobson wrote:
> Also, since this is a problem also in v13 maybe this should also be
> back-ported?  I think it's a bug since both
> pg_identify_object_as_address() and event triggers exists in v13, so
> the function should work there as well, otherwise users would need
> to do work-arounds for event triggers.

No objections from here to do something in back-branches.  We cannot
have a test for event triggers in object_address.sql and it would be
better to keep it in a parallel set (see 676858b for example).  Could
you however add a small test for that in event_trigger.sql?  It would
be good to check after all three functions pg_identify_object(),
pg_identify_object_as_address() and pg_get_object_address().
--
Michael

On Fri, Apr 23, 2021 at 09:33:36AM +0200, Joel Jacobson wrote:

> Also, since this is a problem also in v13 maybe this should also be

> back-ported?  I think it's a bug since both

> pg_identify_object_as_address() and event triggers exists in v13, so

> the function should work there as well, otherwise users would need

> to do work-arounds for event triggers.  

No objections from here to do something in back-branches.  We cannot

have a test for event triggers in object_address.sql and it would be

better to keep it in a parallel set (see 676858b for example).  Could

you however add a small test for that in event_trigger.sql?  It would

be good to check after all three functions pg_identify_object(),

pg_identify_object_as_address() and pg_get_object_address().

--

Michael

On Tue, 27 Apr 2021 at 13:16, Joel Jacobson <joel@compiler.org> wrote:
> On Mon, Apr 26, 2021, at 10:30, Michael Paquier wrote:
>> On Fri, Apr 23, 2021 at 09:33:36AM +0200, Joel Jacobson wrote:
>> > Also, since this is a problem also in v13 maybe this should also be
>> > back-ported?  I think it's a bug since both
>> > pg_identify_object_as_address() and event triggers exists in v13, so
>> > the function should work there as well, otherwise users would need
>> > to do work-arounds for event triggers.  
>> 
>> No objections from here to do something in back-branches.  We cannot
>> have a test for event triggers in object_address.sql and it would be
>> better to keep it in a parallel set (see 676858b for example).  Could
>> you however add a small test for that in event_trigger.sql?  It would
>> be good to check after all three functions pg_identify_object(),
>> pg_identify_object_as_address() and pg_get_object_address().
>> --
>> Michael
>
> Thanks for the guidance in how to test.
>
> I've added a test at the end of event_trigger.sql,
> reusing the three event triggers already in existence,
> just before they are dropped.
>
> New patch attached.

IMO we should add a space between the parameters to keep the code
style consistently.

+SELECT
+  evtname,
+  pg_describe_object('pg_event_trigger'::regclass,oid,0),
+  pg_identify_object('pg_event_trigger'::regclass,oid,0),
+  pg_identify_object_as_address('pg_event_trigger'::regclass,oid,0)
+FROM pg_event_trigger
+WHERE evtname IN ('start_rls_command','end_rls_command','sql_drop_command')
+ORDER BY evtname;

-- 
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

On Tue, Apr 27, 2021 at 07:16:25AM +0200, Joel Jacobson wrote:
> I've added a test at the end of event_trigger.sql,
> reusing the three event triggers already in existence,
> just before they are dropped.

Cool, thanks.  I have been looking at it and I'd still like to
cross-check the output data of pg_get_object_address() to see if
pg_identify_object() remains consistent through it.  See for example
the attached that uses a trick based on LATERAL, a bit different than
what's done in object_address.sql but that gives the same amount of
coverage (I could also use two ROW()s and an equality, but well..).
--
Michael

On Tue, Apr 27, 2021 at 07:16:25AM +0200, Joel Jacobson wrote:

> I've added a test at the end of event_trigger.sql,

> reusing the three event triggers already in existence,

> just before they are dropped.

Cool, thanks.  I have been looking at it and I'd still like to

cross-check the output data of pg_get_object_address() to see if

pg_identify_object() remains consistent through it.  See for example

the attached that uses a trick based on LATERAL, a bit different than

what's done in object_address.sql but that gives the same amount of

coverage (I could also use two ROW()s and an equality, but well..).

On Tue, Apr 27, 2021 at 02:33:36PM +0200, Joel Jacobson wrote:
> I've successfully tested fix_event_trigger_pg_identify_object_as_address3.patch.

Thanks.  Applied down to 9.6 then.
--
Michael