From: Michael Paquier <michael@paquier.xyz>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Aleksander Alekseev <aleksander@timescale.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Cc: Sven Klemm <sven@timescale.com>
Subject: Re: Fix dropped object handling in pg_event_trigger_ddl_commands
Date: Fri, 11 Jun 2021 12:46:56 +0900
Message-ID: <YMLcsJrjWAYs/7SL@paquier.xyz> (raw)
In-Reply-To: <YMHIQAdUA7LkrsFZ@paquier.xyz>
References: <YMBRvAIZxJyQE5fv@paquier.xyz>
<202106091355.sphciftmvqds@alvherre.pgsql>
<YMHIQAdUA7LkrsFZ@paquier.xyz>
On Thu, Jun 10, 2021 at 05:07:28PM +0900, Michael Paquier wrote:
> Except that these syscache lookups need to be done on an object-type
> basis, which is basically what getObjectDescription() & friends now do
> where the logic makes sure that we have a correct objectId <-> cacheId
> mapping for the syscache lookups. So that would be roughly copying
> into event_trigger.c what objectaddress.c does now, but for the back
> branches. It would be better to just backport the changes to support
> missing_ok in objectaddress.c if we go down this road, but the
> invasiveness makes that much more complicated.
I have been looking at that more this morning, and I have convinced
myself that skipping objects should work fine. The test added at the
bottom of event_trigger.sql was making the file a bit messier though,
and there are already tests for relations when it comes to dropped
objects. So let's do a bit of consolidation while on it with an extra
event trigger on ddl_command_end and relations on the schema evttrig.
This one already included some cases for serial columns, so that's
natural to me to extend the area for identity columns. I have also
added a case for a serial column dropped, while on it. The last thing
is the addition of r.object_identity from
pg_event_trigger_ddl_commands() in the data generated for the output
messages, so as the output is as complete as possible.
Regarding the back-branches, I am tempted to do nothing. The APIs are
just not here to do the job. On top of being an invasive change, it
took 4 years for somebody to complain on this matter, as this exists
since 10. That's not worth the risk/cost.
--
Michael
Attachments:altertab-eventtrigger-v3.patch (text/x-diff, 10.6K) downloaddiff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.cindex 5bde507c75..d9d1b28401 100644--- a/src/backend/commands/event_trigger.c+++ b/src/backend/commands/event_trigger.c@@ -1936,8 +1936,19 @@ pg_event_trigger_ddl_commands(PG_FUNCTION_ARGS)
else if (cmd->type == SCT_AlterTSConfig)
addr = cmd->d.atscfg.address;
- type = getObjectTypeDescription(&addr, false);- identity = getObjectIdentity(&addr, false);+ /*+ * If an object was dropped in the same command we may end+ * up in a situation where we generated a message but can+ * no longer look for the object information, so skip it+ * rather than failing. This can happen for example with+ * some subcommand combinations of ALTER TABLE.+ */+ identity = getObjectIdentity(&addr, true);+ if (identity == NULL)+ continue;++ /* The type can never be NULL */+ type = getObjectTypeDescription(&addr, true);
/*
* Obtain schema name, if any ("pg_temp" if a temp
diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.outindex 369f3d7d84..44d545de25 100644--- a/src/test/regress/expected/event_trigger.out+++ b/src/test/regress/expected/event_trigger.out@@ -366,6 +366,7 @@ SELECT * FROM dropped_objects WHERE type = 'schema';
DROP ROLE regress_evt_user;
DROP EVENT TRIGGER regress_event_trigger_drop_objects;
DROP EVENT TRIGGER undroppable;
+-- Event triggers on relations.
CREATE OR REPLACE FUNCTION event_trigger_report_dropped()
RETURNS event_trigger
LANGUAGE plpgsql
@@ -384,41 +385,90 @@ BEGIN
END; $$;
CREATE EVENT TRIGGER regress_event_trigger_report_dropped ON sql_drop
EXECUTE PROCEDURE event_trigger_report_dropped();
+CREATE OR REPLACE FUNCTION event_trigger_report_end()+ RETURNS event_trigger+ LANGUAGE plpgsql+AS $$+DECLARE r RECORD;+BEGIN+ FOR r IN SELECT * FROM pg_event_trigger_ddl_commands()+ LOOP+ RAISE NOTICE 'END: command_tag=% type=% identity=%',+ r.command_tag, r.object_type, r.object_identity;+ END LOOP;+END; $$;+CREATE EVENT TRIGGER regress_event_trigger_report_end ON ddl_command_end+ EXECUTE PROCEDURE event_trigger_report_end();
CREATE SCHEMA evttrig
- CREATE TABLE one (col_a SERIAL PRIMARY KEY, col_b text DEFAULT 'forty two')+ CREATE TABLE one (col_a SERIAL PRIMARY KEY, col_b text DEFAULT 'forty two', col_c SERIAL)
CREATE INDEX one_idx ON one (col_b)
- CREATE TABLE two (col_c INTEGER CHECK (col_c > 0) REFERENCES one DEFAULT 42);+ CREATE TABLE two (col_c INTEGER CHECK (col_c > 0) REFERENCES one DEFAULT 42)+ CREATE TABLE id (col_d int NOT NULL GENERATED ALWAYS AS IDENTITY);+NOTICE: END: command_tag=CREATE SCHEMA type=schema identity=evttrig+NOTICE: END: command_tag=CREATE SEQUENCE type=sequence identity=evttrig.one_col_a_seq+NOTICE: END: command_tag=CREATE SEQUENCE type=sequence identity=evttrig.one_col_c_seq+NOTICE: END: command_tag=CREATE TABLE type=table identity=evttrig.one+NOTICE: END: command_tag=CREATE INDEX type=index identity=evttrig.one_pkey+NOTICE: END: command_tag=ALTER SEQUENCE type=sequence identity=evttrig.one_col_a_seq+NOTICE: END: command_tag=ALTER SEQUENCE type=sequence identity=evttrig.one_col_c_seq+NOTICE: END: command_tag=CREATE TABLE type=table identity=evttrig.two+NOTICE: END: command_tag=ALTER TABLE type=table identity=evttrig.two+NOTICE: END: command_tag=CREATE SEQUENCE type=sequence identity=evttrig.id_col_d_seq+NOTICE: END: command_tag=CREATE TABLE type=table identity=evttrig.id+NOTICE: END: command_tag=ALTER SEQUENCE type=sequence identity=evttrig.id_col_d_seq+NOTICE: END: command_tag=CREATE INDEX type=index identity=evttrig.one_idx
-- Partitioned tables with a partitioned index
CREATE TABLE evttrig.parted (
id int PRIMARY KEY)
PARTITION BY RANGE (id);
+NOTICE: END: command_tag=CREATE TABLE type=table identity=evttrig.parted+NOTICE: END: command_tag=CREATE INDEX type=index identity=evttrig.parted_pkey
CREATE TABLE evttrig.part_1_10 PARTITION OF evttrig.parted (id)
FOR VALUES FROM (1) TO (10);
+NOTICE: END: command_tag=CREATE TABLE type=table identity=evttrig.part_1_10
CREATE TABLE evttrig.part_10_20 PARTITION OF evttrig.parted (id)
FOR VALUES FROM (10) TO (20) PARTITION BY RANGE (id);
+NOTICE: END: command_tag=CREATE TABLE type=table identity=evttrig.part_10_20
CREATE TABLE evttrig.part_10_15 PARTITION OF evttrig.part_10_20 (id)
FOR VALUES FROM (10) TO (15);
+NOTICE: END: command_tag=CREATE TABLE type=table identity=evttrig.part_10_15
CREATE TABLE evttrig.part_15_20 PARTITION OF evttrig.part_10_20 (id)
FOR VALUES FROM (15) TO (20);
+NOTICE: END: command_tag=CREATE TABLE type=table identity=evttrig.part_15_20
ALTER TABLE evttrig.two DROP COLUMN col_c;
NOTICE: NORMAL: orig=t normal=f istemp=f type=table column identity=evttrig.two.col_c name={evttrig,two,col_c} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=table constraint identity=two_col_c_check on evttrig.two name={evttrig,two,two_col_c_check} args={}
+NOTICE: END: command_tag=ALTER TABLE type=table identity=evttrig.two
ALTER TABLE evttrig.one ALTER COLUMN col_b DROP DEFAULT;
NOTICE: NORMAL: orig=t normal=f istemp=f type=default value identity=for evttrig.one.col_b name={evttrig,one,col_b} args={}
+NOTICE: END: command_tag=ALTER TABLE type=table identity=evttrig.one
ALTER TABLE evttrig.one DROP CONSTRAINT one_pkey;
NOTICE: NORMAL: orig=t normal=f istemp=f type=table constraint identity=one_pkey on evttrig.one name={evttrig,one,one_pkey} args={}
+NOTICE: END: command_tag=ALTER TABLE type=table identity=evttrig.one+ALTER TABLE evttrig.one DROP COLUMN col_c;+NOTICE: NORMAL: orig=t normal=f istemp=f type=table column identity=evttrig.one.col_c name={evttrig,one,col_c} args={}+NOTICE: NORMAL: orig=f normal=t istemp=f type=default value identity=for evttrig.one.col_c name={evttrig,one,col_c} args={}+NOTICE: END: command_tag=ALTER TABLE type=table identity=evttrig.one+ALTER TABLE evttrig.id ALTER COLUMN col_d SET DATA TYPE bigint;+NOTICE: END: command_tag=ALTER SEQUENCE type=sequence identity=evttrig.id_col_d_seq+NOTICE: END: command_tag=ALTER TABLE type=table identity=evttrig.id+ALTER TABLE evttrig.id ALTER COLUMN col_d DROP IDENTITY,+ ALTER COLUMN col_d SET DATA TYPE int;+NOTICE: END: command_tag=ALTER TABLE type=table identity=evttrig.id
DROP INDEX evttrig.one_idx;
NOTICE: NORMAL: orig=t normal=f istemp=f type=index identity=evttrig.one_idx name={evttrig,one_idx} args={}
DROP SCHEMA evttrig CASCADE;
-NOTICE: drop cascades to 3 other objects+NOTICE: drop cascades to 4 other objects
DETAIL: drop cascades to table evttrig.one
drop cascades to table evttrig.two
+drop cascades to table evttrig.id
drop cascades to table evttrig.parted
NOTICE: NORMAL: orig=t normal=f istemp=f type=schema identity=evttrig name={evttrig} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.one name={evttrig,one} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=sequence identity=evttrig.one_col_a_seq name={evttrig,one_col_a_seq} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=default value identity=for evttrig.one.col_a name={evttrig,one,col_a} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.two name={evttrig,two} args={}
+NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.id name={evttrig,id} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.parted name={evttrig,parted} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.part_1_10 name={evttrig,part_1_10} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.part_10_20 name={evttrig,part_10_20} args={}
@@ -427,6 +477,7 @@ NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.part_15_20
DROP TABLE a_temp_tbl;
NOTICE: NORMAL: orig=t normal=f istemp=t type=table identity=pg_temp.a_temp_tbl name={pg_temp,a_temp_tbl} args={}
DROP EVENT TRIGGER regress_event_trigger_report_dropped;
+DROP EVENT TRIGGER regress_event_trigger_report_end;
-- only allowed from within an event trigger function, should fail
select pg_event_trigger_table_rewrite_oid();
ERROR: pg_event_trigger_table_rewrite_oid() can only be called in a table_rewrite event trigger function
diff --git a/src/test/regress/sql/event_trigger.sql b/src/test/regress/sql/event_trigger.sqlindex e79c5f0b5d..1446cf8cc8 100644--- a/src/test/regress/sql/event_trigger.sql+++ b/src/test/regress/sql/event_trigger.sql@@ -273,6 +273,7 @@ DROP ROLE regress_evt_user;
DROP EVENT TRIGGER regress_event_trigger_drop_objects;
DROP EVENT TRIGGER undroppable;
+-- Event triggers on relations.
CREATE OR REPLACE FUNCTION event_trigger_report_dropped()
RETURNS event_trigger
LANGUAGE plpgsql
@@ -291,10 +292,26 @@ BEGIN
END; $$;
CREATE EVENT TRIGGER regress_event_trigger_report_dropped ON sql_drop
EXECUTE PROCEDURE event_trigger_report_dropped();
+CREATE OR REPLACE FUNCTION event_trigger_report_end()+ RETURNS event_trigger+ LANGUAGE plpgsql+AS $$+DECLARE r RECORD;+BEGIN+ FOR r IN SELECT * FROM pg_event_trigger_ddl_commands()+ LOOP+ RAISE NOTICE 'END: command_tag=% type=% identity=%',+ r.command_tag, r.object_type, r.object_identity;+ END LOOP;+END; $$;+CREATE EVENT TRIGGER regress_event_trigger_report_end ON ddl_command_end+ EXECUTE PROCEDURE event_trigger_report_end();+
CREATE SCHEMA evttrig
- CREATE TABLE one (col_a SERIAL PRIMARY KEY, col_b text DEFAULT 'forty two')+ CREATE TABLE one (col_a SERIAL PRIMARY KEY, col_b text DEFAULT 'forty two', col_c SERIAL)
CREATE INDEX one_idx ON one (col_b)
- CREATE TABLE two (col_c INTEGER CHECK (col_c > 0) REFERENCES one DEFAULT 42);+ CREATE TABLE two (col_c INTEGER CHECK (col_c > 0) REFERENCES one DEFAULT 42)+ CREATE TABLE id (col_d int NOT NULL GENERATED ALWAYS AS IDENTITY);
-- Partitioned tables with a partitioned index
CREATE TABLE evttrig.parted (
@@ -312,11 +329,16 @@ CREATE TABLE evttrig.part_15_20 PARTITION OF evttrig.part_10_20 (id)
ALTER TABLE evttrig.two DROP COLUMN col_c;
ALTER TABLE evttrig.one ALTER COLUMN col_b DROP DEFAULT;
ALTER TABLE evttrig.one DROP CONSTRAINT one_pkey;
+ALTER TABLE evttrig.one DROP COLUMN col_c;+ALTER TABLE evttrig.id ALTER COLUMN col_d SET DATA TYPE bigint;+ALTER TABLE evttrig.id ALTER COLUMN col_d DROP IDENTITY,+ ALTER COLUMN col_d SET DATA TYPE int;
DROP INDEX evttrig.one_idx;
DROP SCHEMA evttrig CASCADE;
DROP TABLE a_temp_tbl;
DROP EVENT TRIGGER regress_event_trigger_report_dropped;
+DROP EVENT TRIGGER regress_event_trigger_report_end;
-- only allowed from within an event trigger function, should fail
select pg_event_trigger_table_rewrite_oid();
signature.asc (application/pgp-signature, 832B) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: michael@paquier.xyz, alvherre@alvh.no-ip.org, aleksander@timescale.com, pgsql-hackers@lists.postgresql.org, sven@timescale.com
Subject: Re: Fix dropped object handling in pg_event_trigger_ddl_commands
In-Reply-To: <YMLcsJrjWAYs/7SL@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox