Re: Sequence Access Methods, round two

On Fri, Mar 28, 2025 at 12:40:17PM +0500, Kirill Reshke wrote:
> Looks like we entered a rebase loop here. I'm really interested in
> moving this forward; furthermore, I am inclined to include these
> patches into our PostgreSQL fork with modifications that we use for
> sharding and cloud.

Yes. I do have some folks poking at me about this patch set on a
regular basis, mostly people working on cluster-like applications
where they want to control the source of the sequence computation, and
because they need to maintain some custom forking chirurgy around the
sequence area. So I'd like to get something done for v19 in this
area, but well, few people need to be interested in the whole topic
for the moment. There was an unconference session last year about
that, but there was no consensus reached with the topic drifting
around the copy of sequence data in logical replication setups. What
I have here is different. It cannot come down to me to take a
decision and do something; that's not the way things work around here.

> At first, general thought: Access Method is a term we use for
> relations. Table access method is something that stores data and
> knowledge on how to read this data, and index access method is
> something that hints what data we should retrieve using some predicate
> (or quals). But sequence... is generating data primitive? So, maybe
> not Sequence access method but Sequence generate method? OTOH this
> patch provides a way for generating sequence values by _accessing_
> remote storage or procedures, so maybe we are fine with this wording.

"Access method" is kind of fit for me here. We have all the
infrastructure in place for this purpose, with the utility commands
and such. Duplicating all that seems a waste, and sequences are
relations.

> patches:
>
> 0003:
> Patch uses "local" wording for build-in sequence, which is not
> precise. As I understand, snowflake sequence state is local too.

Yes, I am not wedded to this name.

>> + /*
>> + * Retrieve table access method used by a sequence to store its metadata.
>> + */
>> + const char *(*get_table_am) (void);
>
> Looks like we force the sequence to have some table storage. I can
> imagine cases where local storage is not used (and thus, not logged),
> so can we have this optional?

The patch set gives the option for sequences to define the set of
attributes they want, and it's up to the callbacks to decide if they
want to interact with the storage. An implementation can also choose
to not WAL-log anything if they want; the callbacks have been designed
to allow this case (see the in-memory sequence computation from
upthread). The point is that not supporting a NULL makes all the
logic around the relcache much easier to think about, for little gain,
actually. And we still need to support the default case where a heap
relation is required for the current in-core "local" sequences. So
I'm not really excited about such cases as a whole.

> 0007:
> So, we use generic xlog for logging very small actual changes. I
> understand this is out of topic here, but can we add a second option
> for generic log to store FormData_snowflake_data which is a few bytes?
> Maybe we should start different thread for that

Without an agreement about how the basic callbacks should be shaped, I
don't think that this is worth the time at this stage.

Spoiler: I'm pretty happy with the way things are done in the patch
as with 64-bit fields for sequences mapping only to integers (no 128b
or more, no custom types). One is already set for life with 8 bytes.

I am rebasing the patch set, there were a few things with the removal
of the %lld and their associated (long long) casts.
--
Michael