Re: pgsql: Fix gratuitous error message variation

2019-11-09 Thread Euler Taveira
Em sáb., 9 de nov. de 2019 às 05:02, Peter Eisentraut
 escreveu:
>
> On 2019-11-09 02:20, Tom Lane wrote:
> > Peter Eisentraut  writes:
> >> Fix gratuitous error message variation
> >
> > Hm, as long as you're touching that ... OIDs should be formatted
> > with %u not %d.
>
> This is just a fixup of the recent patch
> 8f75e8e44609335e6bdd73123284682235f242a2.
>
> Replication origin IDs are actually
>
> typedef uint16 RepOriginId;
>
... that is different from OIDs.

> so using the term OID is probably wrong altogether.  I'm not sure what
> the overall intent was here.
>
According to the following comment:

 * We need the numeric replication origin to be 16bit wide, so we cannot
 * rely on the normal oid allocation. Instead we simply scan
 * pg_replication_origin for the first unused id. That's not particularly
 * efficient, but this should be a fairly infrequent operation - we can
 * easily spend a bit more code on this when it turns out it needs to be
 * faster.

... it seems the author wants to use OID infrastructure for a type
(RepOriginId) that is similar to Oid.

Although, it is a different representation from OID, it uses OID
infrastructure in its API. Indeed RepOriginId is a subset of Oid. If
terminology "replication origin OID" is used, users can complain that
it will fail to allocate more than 65k replication origins (OID range
is much larger).Frankly, I don't foresee it happen in decades. I think
"replication origin" shouldn't use OID datatype, however, that ship
has sailed. I propose to use %u instead of %d (same as we do with
OIDs). I also included a s/oid/OID/. Patch is attached.


-- 
   Euler Taveira   Timbira -
http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
index fad2194418..8af20a4496 100644
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -367,7 +367,7 @@ restart:
 if (nowait)
 	ereport(ERROR,
 			(errcode(ERRCODE_OBJECT_IN_USE),
-			 errmsg("could not drop replication origin with OID %d, in use by PID %d",
+			 errmsg("could not drop replication origin with OID %u, in use by PID %d",
 	state->roident,
 	state->acquired_by)));
 
@@ -411,7 +411,7 @@ restart:
 	 */
 	tuple = SearchSysCache1(REPLORIGIDENT, ObjectIdGetDatum(roident));
 	if (!HeapTupleIsValid(tuple))
-		elog(ERROR, "cache lookup failed for replication origin with oid %u",
+		elog(ERROR, "cache lookup failed for replication origin with OID %u",
 			 roident);
 
 	CatalogTupleDelete(rel, >t_self);
@@ -914,7 +914,7 @@ replorigin_advance(RepOriginId node,
 		{
 			ereport(ERROR,
 	(errcode(ERRCODE_OBJECT_IN_USE),
-	 errmsg("replication origin with OID %d is already active for PID %d",
+	 errmsg("replication origin with OID %u is already active for PID %d",
 			replication_state->roident,
 			replication_state->acquired_by)));
 		}
@@ -1100,7 +1100,7 @@ replorigin_session_setup(RepOriginId node)
 		{
 			ereport(ERROR,
 	(errcode(ERRCODE_OBJECT_IN_USE),
-	 errmsg("replication origin with OID %d is already active for PID %d",
+	 errmsg("replication origin with OID %u is already active for PID %d",
 			curstate->roident, curstate->acquired_by)));
 		}
 


Re: pgsql: Fix gratuitous error message variation

2019-11-09 Thread Peter Eisentraut

On 2019-11-09 02:20, Tom Lane wrote:

Peter Eisentraut  writes:

Fix gratuitous error message variation


Hm, as long as you're touching that ... OIDs should be formatted
with %u not %d.


This is just a fixup of the recent patch 
8f75e8e44609335e6bdd73123284682235f242a2.


Replication origin IDs are actually

typedef uint16 RepOriginId;

so using the term OID is probably wrong altogether.  I'm not sure what 
the overall intent was here.


--
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services




Re: pgsql: Fix gratuitous error message variation

2019-11-08 Thread Tom Lane
Peter Eisentraut  writes:
> Fix gratuitous error message variation

Hm, as long as you're touching that ... OIDs should be formatted
with %u not %d.

regards, tom lane