Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.300.41.1.7\))
Subject: Re: splitting pg_resetwal output strings
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: <202601311502.ohp4cx7hfms2@alvherre.pgsql>
Date: Mon, 2 Feb 2026 11:58:10 +0800
Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <A2A10B88-A7B1-4757-A64F-94E769C91F28@gmail.com>
References: <202601311502.ohp4cx7hfms2@alvherre.pgsql>
To: =?utf-8?Q?=C3=81lvaro_Herrera?= <alvherre@kurilemu.de>
Archived-At: <https://www.postgresql.org/message-id/A2A10B88-A7B1-4757-A64F-94E769C91F28%40gmail.com>
Precedence: bulk


> On Jan 31, 2026, at 23:08, =C3=81lvaro Herrera <alvherre@kurilemu.de> =
wrote:
>=20
> On 2026-Jan-31, =C3=81lvaro Herrera wrote:
>=20
>> This is not complete.  It modifies PrintControlValues(), which is =
easy
>> because I just print each item in the entries.h file in the same =
order
>> they appear there.   But for PrintNewControlValues() I'll need to add =
a
>> symbolic identifier to each string, that the code can use to scan the
>> array and print just those elements.  I'll do that if there are no
>> objections to this idea here.
>=20
> It looks more or less like this.  Patch 0001 is the same as before, =
and
> 0002 adds the symbolic names, which is used to create an enum and then
> to search for the correct lines to print in PrintNewControlValues.
>=20
> I decided to reuse simple_oid_list to store the integers values, which
> is kinda icky, so getting to something committable I think would have =
me
> add simple_int_list.  Also, there's a few blocks that are duplicate
> cases of the same line measuring and printing logic; I suppose I =
should
> have a routine to simplify.
>=20

Hi Alvaro,

I don't think we necessarily need to add a simple_int_list. Since OIDs =
are not globally unique across all object types, we can reasonably treat =
ControlData items as "objects" and assign them OIDs within this context.

If we take that approach, the enum could be named ControlDataOid:
```
/* Define the string enums */
#define CONTROLDATA_LINE(symbol, description, fmt, ...) \
symbol,
enum {
#include "entries.h=E2=80=9D
} ControlDataOid;
#undef CONTROLDATA_LINE
```

This makes reusing simple_oid_list feel appropriate. If you aren't fond =
of that idea, I=E2=80=99d suggest naming the enum ControlDataSymbol =
rather than ControlDataStrings, as the former better reflects what the =
members actually are.

Regarding patch 0002:
```
+#define CONTROLDATA_LINE(symbol, description, fmt, ...)			=
\
+	if (simple_oid_list_member(&toprint, symbol))				=
\
+	{									=
						\
+	thislen =3D internal_wcswidth(_(description),				=
	\
+								=
strlen(_(description)),			\
+								=
encoding);						\
+	if (thislen > maxlen)							=
			\
+		maxlen =3D thislen;						=
				\
 	}
+#include "entries.h"
+#undef CONTROLDATA_LINE
```
The code block inside the first if statement is missing an indentation =
level.

Otherwise, the patch looks good to me. I've tested it and confirmed that =
calculating the padding at runtime generates better-looking output.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/