Message-ID: <766da800-4884-41a9-8e8e-71d131dc599c@illuminatedcomputing.com>
Date: Wed, 26 Mar 2025 15:46:21 -0700
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [fixed] Trigger test
To: Dmitrii Bondar <d.bondar@postgrespro.ru>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <79755a2b18ed4fe5e29da6a87a1e00d1@postgrespro.ru>
 <7879fede654a17b9fcc8548de2cb3f69@postgrespro.ru>
 <174293570374.60294.13024599409215069922.pgcf@coridan.postgresql.org>
 <17a8a24f7295201873d105c5118024f5@postgrespro.ru>
Content-Language: en-US
From: Paul Jungwirth <pj@illuminatedcomputing.com>
Cc: Lilian <ontowhee@gmail.com>
In-Reply-To: <17a8a24f7295201873d105c5118024f5@postgrespro.ru>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/766da800-4884-41a9-8e8e-71d131dc599c%40illuminatedcomputing.com>
Precedence: bulk

Hi Dmitrii,

Thanks for the quick update!

On 3/26/25 02:45, Dmitrii Bondar wrote:
 >> 3. Consider updating documentation for doc/src/contrib-spi.sgml, or any file as appropriate, to
 >> reflect the changes.
 >
 > The changes have now been added to doc/src/contrib-spi.sgml. I also added a consideration note about
 > interactions with BEFORE triggers.

This looks good. I have a couple small grammar suggestions. This:

+   To use, create a <literal>AFTER INSERT OR UPDATE</literal> trigger using this

should be:

+   To use, create an <literal>AFTER INSERT OR UPDATE</literal> trigger using this

and this:

+   To use, create a <literal>AFTER DELETE OR UPDATE</literal> trigger using this

should be this:

+   To use, create an <literal>AFTER DELETE OR UPDATE</literal> trigger using this

Also re this part of the patch:

@@ -592,10 +598,15 @@ check_foreign_key(PG_FUNCTION_ARGS)
     }
     else
     {
+     const char* operation;
+
+     if (action == 'c')
+       operation = is_update ? "updated" : "deleted";
+     else
+       operation = "set to null";
  #ifdef REFINT_VERBOSE
       elog(NOTICE, "%s: " UINT64_FORMAT " tuple(s) of %s are %s",
-        trigger->tgname, SPI_processed, relname,
-        (action == 'c') ? "deleted" : "set to null");
+        trigger->tgname, SPI_processed, relname, operation);
  #endif
     }
     args += nkeys + 1;    /* to the next relation */

We can put all the new lines inside the #ifdef, can't we?

 > Can you also help me with the patch status? What status should I move the patch to?

I think if you make those changes we should mark this as Ready for Committer.

Yours,

-- 
Paul              ~{:-)
pj@illuminatedcomputing.com