On 12.03.24 00:44, Michael Paquier wrote:
> Anyway, there is one piece of this patch set that I think has a lot of
> value outside of the discussion with access methods, which is to
> redesign pg_sequence_last_value so as it returns a (last_value,
> is_called) tuple rather than a (last_value). This has the benefit of
> switching pg_dump to use this function rather than relying on a scan
> of the heap table used by a sequence to retrieve the state of a
> sequence dumped. This is the main diff:
> - appendPQExpBuffer(query,
> - "SELECT last_value, is_called FROM %s",
> - fmtQualifiedDumpable(tbinfo));
> + /*
> + * In versions 17 and up, pg_sequence_last_value() has been switched to
> + * return a tuple with last_value and is_called.
> + */
> + if (fout->remoteVersion >= 170000)
> + appendPQExpBuffer(query,
> + "SELECT last_value, is_called "
> + "FROM pg_sequence_last_value('%s')",
> + fmtQualifiedDumpable(tbinfo));
> + else
> + appendPQExpBuffer(query,
> + "SELECT last_value, is_called FROM %s",
> + fmtQualifiedDumpable(tbinfo));
>
> Are there any objections to that? pg_sequence_last_value() is
> something that we've only been relying on internally for the catalog
> pg_sequences.
I don't understand what the overall benefit of this change is supposed
to be.
If this route were to be pursued, it should be a different function
name. We shouldn't change the signature of an existing function.
