pgsql: Refer to replication origin roident as "ID" in user facing messa

Started by John Nayloralmost 4 years ago5 messagescomitters
Jump to latest
#1John Naylor
john.naylor@enterprisedb.com

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: /messages/by-id/3437166.1659620465@sss.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(-)

#2Peter Eisentraut
peter_e@gmx.net
In reply to: John Naylor (#1)
Re: pgsql: Refer to replication origin roident as "ID" in user facing messa

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.

#3John Naylor
john.naylor@enterprisedb.com
In reply to: Peter Eisentraut (#2)
Re: pgsql: Refer to replication origin roident as "ID" in user facing messa

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

#4Peter Eisentraut
peter_e@gmx.net
In reply to: John Naylor (#3)
Re: pgsql: Refer to replication origin roident as "ID" in user facing messa

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.

#5John Naylor
john.naylor@enterprisedb.com
In reply to: Peter Eisentraut (#4)
Re: pgsql: Refer to replication origin roident as "ID" in user facing messa

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