Обсуждение: pgsql: Refer to replication origin roident as "ID" in user facing messa

Refer to replication origin roident as "ID" in user facing messages and docs

The table column that stores this is of type oid, but is actually limited
to uint16 and has a different path for creating new values. Some of
the documentation already referred to it as an ID, so let's standardize
on that.

While at it, most format strings already use %u, so for consintency
change the remaining stragglers using %d.

Per suggestions from Tom Lane and Justin Pryzby
Discussion: https://www.postgresql.org/message-id/3437166.1659620465%40sss.pgh.pa.us
Backpatch to v15

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/c920fe48181d81ce218faa746f8af92c41d03b2e

Modified Files
--------------
doc/src/sgml/replication-origins.sgml    |  4 ++--
src/backend/replication/logical/origin.c | 16 ++++++++--------
2 files changed, 10 insertions(+), 10 deletions(-)

On 18.08.22 04:10, John Naylor wrote:
> While at it, most format strings already use %u, so for consintency
> change the remaining stragglers using %d.

This is incorrect.  Replication origin IDs are of type uint16, which 
gets promoted to int in a variable arguments list, so %d is the correct 
placeholder.

On Thu, Aug 18, 2022 at 4:49 PM Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:
>
> On 18.08.22 04:10, John Naylor wrote:
> > While at it, most format strings already use %u, so for consintency
> > change the remaining stragglers using %d.
>
> This is incorrect.  Replication origin IDs are of type uint16, which
> gets promoted to int in a variable arguments list, so %d is the correct
> placeholder.

I would think that for uint16, either %d or %u would have the same
result. Is there some other consideration I'm not aware of?

-- 
John Naylor
EDB: http://www.enterprisedb.com

On 19.08.22 03:06, John Naylor wrote:
> On Thu, Aug 18, 2022 at 4:49 PM Peter Eisentraut
> <peter.eisentraut@enterprisedb.com> wrote:
>>
>> On 18.08.22 04:10, John Naylor wrote:
>>> While at it, most format strings already use %u, so for consintency
>>> change the remaining stragglers using %d.
>>
>> This is incorrect.  Replication origin IDs are of type uint16, which
>> gets promoted to int in a variable arguments list, so %d is the correct
>> placeholder.
> 
> I would think that for uint16, either %d or %u would have the same
> result. Is there some other consideration I'm not aware of?

Every once in a while, I build PostgreSQL with -Wformat-signedness, 
which often finds issues with OIDs and timeline IDs printed with the 
wrong placeholder.  There is a lot of noise to skip past when doing 
that, but in the long run it would be nice if we didn't add more of it.

On Mon, Aug 22, 2022 at 9:35 PM Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:
>
> On 19.08.22 03:06, John Naylor wrote:
> > On Thu, Aug 18, 2022 at 4:49 PM Peter Eisentraut
> > <peter.eisentraut@enterprisedb.com> wrote:
> >>
> >> On 18.08.22 04:10, John Naylor wrote:
> >>> While at it, most format strings already use %u, so for consintency
> >>> change the remaining stragglers using %d.
> >>
> >> This is incorrect.  Replication origin IDs are of type uint16, which
> >> gets promoted to int in a variable arguments list, so %d is the correct
> >> placeholder.
> >
> > I would think that for uint16, either %d or %u would have the same
> > result. Is there some other consideration I'm not aware of?
>
> Every once in a while, I build PostgreSQL with -Wformat-signedness,
> which often finds issues with OIDs and timeline IDs printed with the
> wrong placeholder.  There is a lot of noise to skip past when doing
> that, but in the long run it would be nice if we didn't add more of it.

Thanks for the info; I've pushed a patch.

-- 
John Naylor
EDB: http://www.enterprisedb.com