On Tue, Apr 30, 2024 at 10:26 AM David Steele <da...@pgmasters.net> wrote: > > Hackers, > > While testing pgAudit against PG17 I noticed the following behavioral > change: > > CREATE TABLE public.test > ( > id INT, > name TEXT, > description TEXT, > CONSTRAINT test_pkey PRIMARY KEY (id) > ); > NOTICE: AUDIT: SESSION,23,1,DDL,CREATE TABLE,TABLE,public.test,"CREATE > TABLE public.test > ( > id INT, > name TEXT, > description TEXT, > CONSTRAINT test_pkey PRIMARY KEY (id) > );",<none> > NOTICE: AUDIT: SESSION,23,1,DDL,ALTER TABLE,TABLE,public.test,"CREATE > TABLE public.test > ( > id INT, > name TEXT, > description TEXT, > CONSTRAINT test_pkey PRIMARY KEY (id) > );",<none> > NOTICE: AUDIT: SESSION,23,1,DDL,CREATE > INDEX,INDEX,public.test_pkey,"CREATE TABLE public.test > ( > id INT, > name TEXT, > description TEXT, > CONSTRAINT test_pkey PRIMARY KEY (id) > );",<none> > > Prior to PG17, ProcessUtility was not being called for ALTER TABLE in > this context. The change probably makes some sense, since the table is > being created then altered to add the primary key, but since it is a > behavioral change I wanted to bring it up. > > I attempted a bisect to identify the commit that caused this behavioral > change but pgAudit has been broken since at least November on master and > didn't get fixed again until bb766cde (April 8). Looking at that commit > I'm a bit baffled by how it fixed the issue I was seeing, which was that > an event trigger was firing in the wrong context in the pgAudit tests: > > CREATE TABLE tmp (id int, data text); > +ERROR: not fired by event trigger manager > > That error comes out of pgAudit itself: > > if (!CALLED_AS_EVENT_TRIGGER(fcinfo)) > elog(ERROR, "not fired by event trigger manager"); > > Since bb766cde cannot be readily applied to older commits in master I'm > unable to continue bisecting to find the ALTER TABLE behavioral change. > > This seems to leave me with manual code inspection and there have been a > lot of changes in this area, so I'm hoping somebody will know why this > ALTER TABLE change happened before I start digging into it.
probably this commit: https://git.postgresql.org/cgit/postgresql.git/commit/?id=0cd711271d42b0888d36f8eda50e1092c2fed4b3 especially: - * The parser will create AT_AttSetNotNull subcommands for - * columns of PRIMARY KEY indexes/constraints, but we need - * not do anything with them here, because the columns' - * NOT NULL marks will already have been propagated into - * the new table definition. + * We see this subtype when a primary key is created + * internally, for example when it is replaced with a new + * constraint (say because one of the columns changes + * type); in this case we need to reinstate attnotnull, + * because it was removed because of the drop of the old + * PK. Schedule this subcommand to an upcoming AT pass. */ + cmd->subtype = AT_SetAttNotNull; + tab->subcmds[AT_PASS_OLD_COL_ATTRS] = + lappend(tab->subcmds[AT_PASS_OLD_COL_ATTRS], cmd);
