Date: Fri, 26 Jun 2026 16:15:53 +0900 (JST)
Message-Id: <20260626.161553.236415645647775260.horikyota.ntt@gmail.com>
To: michael@paquier.xyz
Cc: sjh910805@gmail.com, alvherre@kurilemu.de, scott@scottray.io,
 masao.fujii@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: [PATCH] Don't call ereport(ERROR) from recovery target GUC
 assign hooks
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
In-Reply-To: <ajzgIRrl8378bnlt@paquier.xyz>
References: <ajfWZqpzgziKNzX5@alvherre.pgsql>
	<CACSdjfMt5obQc0QCX8E+v-pxROWFQxmeW6Ki9X16F60D3tSJgg@mail.gmail.com>
	<ajzgIRrl8378bnlt@paquier.xyz>
User-Agent: Mew version 6.8 on Emacs 29.4
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20260626.161553.236415645647775260.horikyota.ntt%40gmail.com>
Precedence: bulk

Hello,

I spent some time thinking about this issue and the proposed patch,
and I wonder if we can simplify it a bit further.

If we don't want to rely on GUC hooks for cross-checking multiple
recovery target settings, perhaps the assign hooks could stop updating
the shared recoveryTarget state altogether and only maintain their own
underlying variables.

As far as I can tell, the existing recovery target variables already
seem sufficient to determine whether each target is configured. The
only exception appears to be recovery_target = 'immediate', which
would probably need an additional boolean flag (or similar state) to
represent whether it has been specified.

Then validateRecoveryParameters() could determine which recovery
target is configured by inspecting those variables directly, report an
error if more than one target is set, and reconstruct the same
recoveryTarget state that the current code expects.

Does that make sense?

Regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center