Re: Logical Replication of sequences

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Euler Taveira <euler(at)eulerto(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Hou, Zhijie/侯 志杰 <houzj(dot)fnst(at)fujitsu(dot)com>, "Jonathan S(dot) Katz" <jkatz(at)postgresql(dot)org>
Subject: Re: Logical Replication of sequences
Date: 2024-12-18 02:41:57
Message-ID: CAHut+Psk1V2eUL_nreWp1trO1iSDqhDtBnfu65PrsoorpuNzKA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Vignesh.

Here are some review comments for patch v20241211-0001.

======
src/backend/commands/sequence.c

pg_sequence_state:

1.
+ TupleDesc tupdesc;
+ HeapTuple tuple;
+ Datum values[4];
+ bool nulls[4] = {false, false, false, false};

SUGGESTION
bool nulls[4] = {0};

The above achieves the same thing, but more succinctly, and I think it
is a common enough pattern in the PG source code.

~~~

2.
+ seq = read_seq_tuple(seqrel, &buf, &seqtuple);
+ page = BufferGetPage(buf);
+ lsn = PageGetLSN(page);
+
+ last_value = seq->last_value;
+ log_cnt = seq->log_cnt;
+ is_called = seq->is_called;

Move the blank line, so the 'lsn' assignment will be grouped with the
other 3 assignments which are part of the return tuple.

~~~

3.
+ /* How many fetches remain before a new WAL record has to be written */
+ values[2] = Int64GetDatum(log_cnt);

Trivial change to use the same wording as the documentation.

/has to/must/

======

(The attached NITPICKS patch includes the above suggestions)

======
Kind Regards,
Peter Smith.
Fujitsu Australia

Attachment Content-Type Size
PS_NITPICKS_20241218_SEQ_0001.txt text/plain 1.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message jian he 2024-12-18 03:00:01 Re: proposal: schema variables
Previous Message Tom Lane 2024-12-18 02:28:57 Re: Pg18 Recursive Crash