On 2018-Jul-05, Michael Paquier wrote:
> On Wed, Jul 04, 2018 at 09:57:31AM -0400, Alvaro Herrera wrote:
> > None from me.
>
> Thanks Alvaro. For now the patch uses the following error message:
> +SELECT pg_replication_slot_advance('regression_slot3', '0/1'); -- error
> +ERROR: cannot move slot with non-reserved restart_lsn
>
> Mentioning directly the column name of pg_replication_slots is confusing
> I think. Here are some suggestions of perhaps better error messages:
> 1) cannot move unreserved slot.
> 2) cannot move slot which has never been reserved.
Yeah, I don't like it very much. Let's avoid having an obscure column
name in there.
Do we use the term "reserved" anywhere else? It just doesn't click for
me. Other than "All rights reserved", that is ...
As for the patch itself: why is the problem that the slot is "not
reserved" in the first place? I think what we should be actually
checking is that the target LSN is within valid limits, ie. the end
state of the slot after the operation, rather than the initial state of
the slot before the operation.
If we made this code check the end state, we could make the error
message be something like "target LSN is not within the allocated range"
or something like that.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services