public inbox for [email protected]
help / color / mirror / Atom feedFrom: Aleksander Alekseev <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Cc: Tom Lane <[email protected]>
Subject: [PATCH] Remove workarounds to format [u]int64's
Date: Mon, 21 Mar 2022 11:52:14 +0300
Message-ID: <CAJ7c6TMSKi3Xs8h5MP38XOnQQpBLazJvVxVfPn++roitDJcR7g@mail.gmail.com> (raw)
Hi hackers,
I learned from Tom [1] that we can simplify the code like:
```
char buff[32];
snprintf(buf, sizeof(buf), INT64_FORMAT, ...)
ereport(WARNING, (errmsg("%s ...", buf)));
```
... and rely on %lld/%llu now as long as we explicitly cast the
argument to long long int / unsigned long long. This was previously
addressed in 6a1cd8b9 and d914eb34, but I see more places where we
still use an old approach.
Suggested patch fixes this. Tested locally - no warnings; passes all the tests.
[1] https://www.postgresql.org/message-id/771048.1647528068%40sss.pgh.pa.us
--
Best regards,
Aleksander Alekseev
Attachments:
[application/octet-stream] v1-0001-Remove-workarounds-to-format-u-int64-s.patch (11.8K, ../CAJ7c6TMSKi3Xs8h5MP38XOnQQpBLazJvVxVfPn++roitDJcR7g@mail.gmail.com/2-v1-0001-Remove-workarounds-to-format-u-int64-s.patch)
download | inline diff:
From 152a9b9e8f1eaa9bd319daca7d1adff282c0c321 Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <[email protected]>
Date: Mon, 21 Mar 2022 11:01:57 +0300
Subject: [PATCH v1] Remove workarounds to format [u]int64's
Now that we always use our own sprintf(), we can rely on %lld and %llu to be
portable, so we can use those. This reduces the code verbosity.
Author: Aleksander Alekseev <[email protected]>
---
src/backend/access/brin/brin.c | 8 +--
src/backend/commands/copyfrom.c | 28 ++++-----
src/backend/commands/sequence.c | 95 ++++++++-----------------------
src/backend/utils/adt/xid8funcs.c | 5 +-
4 files changed, 39 insertions(+), 97 deletions(-)
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index ba78ecff66..c44b807619 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -1018,11 +1018,9 @@ brin_summarize_range(PG_FUNCTION_ARGS)
if (heapBlk64 > BRIN_ALL_BLOCKRANGES || heapBlk64 < 0)
{
- char *blk = psprintf(INT64_FORMAT, heapBlk64);
-
ereport(ERROR,
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
- errmsg("block number out of range: %s", blk)));
+ errmsg("block number out of range: %lld", (long long int)heapBlk64)));
}
heapBlk = (BlockNumber) heapBlk64;
@@ -1095,11 +1093,9 @@ brin_desummarize_range(PG_FUNCTION_ARGS)
if (heapBlk64 > MaxBlockNumber || heapBlk64 < 0)
{
- char *blk = psprintf(INT64_FORMAT, heapBlk64);
-
ereport(ERROR,
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
- errmsg("block number out of range: %s", blk)));
+ errmsg("block number out of range: %lld", (long long int)heapBlk64)));
}
heapBlk = (BlockNumber) heapBlk64;
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index 7b3f5a84b8..bab40c4201 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -115,21 +115,17 @@ void
CopyFromErrorCallback(void *arg)
{
CopyFromState cstate = (CopyFromState) arg;
- char curlineno_str[32];
-
- snprintf(curlineno_str, sizeof(curlineno_str), UINT64_FORMAT,
- cstate->cur_lineno);
if (cstate->opts.binary)
{
/* can't usefully display the data */
if (cstate->cur_attname)
- errcontext("COPY %s, line %s, column %s",
- cstate->cur_relname, curlineno_str,
+ errcontext("COPY %s, line %llu, column %s",
+ cstate->cur_relname, (unsigned long long)cstate->cur_lineno,
cstate->cur_attname);
else
- errcontext("COPY %s, line %s",
- cstate->cur_relname, curlineno_str);
+ errcontext("COPY %s, line %llu",
+ cstate->cur_relname, (unsigned long long)cstate->cur_lineno);
}
else
{
@@ -139,16 +135,16 @@ CopyFromErrorCallback(void *arg)
char *attval;
attval = limit_printout_length(cstate->cur_attval);
- errcontext("COPY %s, line %s, column %s: \"%s\"",
- cstate->cur_relname, curlineno_str,
+ errcontext("COPY %s, line %llu, column %s: \"%s\"",
+ cstate->cur_relname, (unsigned long long)cstate->cur_lineno,
cstate->cur_attname, attval);
pfree(attval);
}
else if (cstate->cur_attname)
{
/* error is relevant to a particular column, value is NULL */
- errcontext("COPY %s, line %s, column %s: null input",
- cstate->cur_relname, curlineno_str,
+ errcontext("COPY %s, line %llu, column %s: null input",
+ cstate->cur_relname, (unsigned long long)cstate->cur_lineno,
cstate->cur_attname);
}
else
@@ -163,14 +159,14 @@ CopyFromErrorCallback(void *arg)
char *lineval;
lineval = limit_printout_length(cstate->line_buf.data);
- errcontext("COPY %s, line %s: \"%s\"",
- cstate->cur_relname, curlineno_str, lineval);
+ errcontext("COPY %s, line %llu: \"%s\"",
+ cstate->cur_relname, (unsigned long long)cstate->cur_lineno, lineval);
pfree(lineval);
}
else
{
- errcontext("COPY %s, line %s",
- cstate->cur_relname, curlineno_str);
+ errcontext("COPY %s, line %llu",
+ cstate->cur_relname, (unsigned long long)cstate->cur_lineno);
}
}
}
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index ab592ce2f1..8db875c003 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -707,13 +707,10 @@ nextval_internal(Oid relid, bool check_permissions)
break; /* stop fetching */
if (!cycle)
{
- char buf[100];
-
- snprintf(buf, sizeof(buf), INT64_FORMAT, maxv);
ereport(ERROR,
(errcode(ERRCODE_SEQUENCE_GENERATOR_LIMIT_EXCEEDED),
- errmsg("nextval: reached maximum value of sequence \"%s\" (%s)",
- RelationGetRelationName(seqrel), buf)));
+ errmsg("nextval: reached maximum value of sequence \"%s\" (%lld)",
+ RelationGetRelationName(seqrel), (long long int)maxv)));
}
next = minv;
}
@@ -730,13 +727,10 @@ nextval_internal(Oid relid, bool check_permissions)
break; /* stop fetching */
if (!cycle)
{
- char buf[100];
-
- snprintf(buf, sizeof(buf), INT64_FORMAT, minv);
ereport(ERROR,
(errcode(ERRCODE_SEQUENCE_GENERATOR_LIMIT_EXCEEDED),
- errmsg("nextval: reached minimum value of sequence \"%s\" (%s)",
- RelationGetRelationName(seqrel), buf)));
+ errmsg("nextval: reached minimum value of sequence \"%s\" (%lld)",
+ RelationGetRelationName(seqrel), (long long int)minv)));
}
next = maxv;
}
@@ -976,18 +970,11 @@ do_setval(Oid relid, int64 next, bool iscalled)
if ((next < minv) || (next > maxv))
{
- char bufv[100],
- bufm[100],
- bufx[100];
-
- snprintf(bufv, sizeof(bufv), INT64_FORMAT, next);
- snprintf(bufm, sizeof(bufm), INT64_FORMAT, minv);
- snprintf(bufx, sizeof(bufx), INT64_FORMAT, maxv);
ereport(ERROR,
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
- errmsg("setval: value %s is out of bounds for sequence \"%s\" (%s..%s)",
- bufv, RelationGetRelationName(seqrel),
- bufm, bufx)));
+ errmsg("setval: value %lld is out of bounds for sequence \"%s\" (%lld..%lld)",
+ (long long int)next, RelationGetRelationName(seqrel),
+ (long long int)minv, (long long int)maxv)));
}
/* Set the currval() state only if iscalled = true */
@@ -1469,14 +1456,10 @@ init_params(ParseState *pstate, List *options, bool for_identity,
if ((seqform->seqtypid == INT2OID && (seqform->seqmax < PG_INT16_MIN || seqform->seqmax > PG_INT16_MAX))
|| (seqform->seqtypid == INT4OID && (seqform->seqmax < PG_INT32_MIN || seqform->seqmax > PG_INT32_MAX)))
{
- char bufx[100];
-
- snprintf(bufx, sizeof(bufx), INT64_FORMAT, seqform->seqmax);
-
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("MAXVALUE (%s) is out of range for sequence data type %s",
- bufx, format_type_be(seqform->seqtypid))));
+ errmsg("MAXVALUE (%lld) is out of range for sequence data type %s",
+ (long long int)seqform->seqmax, format_type_be(seqform->seqtypid))));
}
/* MINVALUE (null arg means NO MINVALUE) */
@@ -1506,28 +1489,19 @@ init_params(ParseState *pstate, List *options, bool for_identity,
if ((seqform->seqtypid == INT2OID && (seqform->seqmin < PG_INT16_MIN || seqform->seqmin > PG_INT16_MAX))
|| (seqform->seqtypid == INT4OID && (seqform->seqmin < PG_INT32_MIN || seqform->seqmin > PG_INT32_MAX)))
{
- char bufm[100];
-
- snprintf(bufm, sizeof(bufm), INT64_FORMAT, seqform->seqmin);
-
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("MINVALUE (%s) is out of range for sequence data type %s",
- bufm, format_type_be(seqform->seqtypid))));
+ errmsg("MINVALUE (%lld) is out of range for sequence data type %s",
+ (long long int)seqform->seqmin, format_type_be(seqform->seqtypid))));
}
/* crosscheck min/max */
if (seqform->seqmin >= seqform->seqmax)
{
- char bufm[100],
- bufx[100];
-
- snprintf(bufm, sizeof(bufm), INT64_FORMAT, seqform->seqmin);
- snprintf(bufx, sizeof(bufx), INT64_FORMAT, seqform->seqmax);
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("MINVALUE (%s) must be less than MAXVALUE (%s)",
- bufm, bufx)));
+ errmsg("MINVALUE (%lld) must be less than MAXVALUE (%lld)",
+ (long long int)seqform->seqmin, (long long int)seqform->seqmax)));
}
/* START WITH */
@@ -1546,27 +1520,17 @@ init_params(ParseState *pstate, List *options, bool for_identity,
/* crosscheck START */
if (seqform->seqstart < seqform->seqmin)
{
- char bufs[100],
- bufm[100];
-
- snprintf(bufs, sizeof(bufs), INT64_FORMAT, seqform->seqstart);
- snprintf(bufm, sizeof(bufm), INT64_FORMAT, seqform->seqmin);
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("START value (%s) cannot be less than MINVALUE (%s)",
- bufs, bufm)));
+ errmsg("START value (%lld) cannot be less than MINVALUE (%lld)",
+ (long long int)seqform->seqstart, (long long int)seqform->seqmin)));
}
if (seqform->seqstart > seqform->seqmax)
{
- char bufs[100],
- bufm[100];
-
- snprintf(bufs, sizeof(bufs), INT64_FORMAT, seqform->seqstart);
- snprintf(bufm, sizeof(bufm), INT64_FORMAT, seqform->seqmax);
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("START value (%s) cannot be greater than MAXVALUE (%s)",
- bufs, bufm)));
+ errmsg("START value (%lld) cannot be greater than MAXVALUE (%lld)",
+ (long long int)seqform->seqstart, (long long int)seqform->seqmax)));
}
/* RESTART [WITH] */
@@ -1588,27 +1552,17 @@ init_params(ParseState *pstate, List *options, bool for_identity,
/* crosscheck RESTART (or current value, if changing MIN/MAX) */
if (seqdataform->last_value < seqform->seqmin)
{
- char bufs[100],
- bufm[100];
-
- snprintf(bufs, sizeof(bufs), INT64_FORMAT, seqdataform->last_value);
- snprintf(bufm, sizeof(bufm), INT64_FORMAT, seqform->seqmin);
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("RESTART value (%s) cannot be less than MINVALUE (%s)",
- bufs, bufm)));
+ errmsg("RESTART value (%lld) cannot be less than MINVALUE (%lld)",
+ (long long int)seqdataform->last_value, (long long int)seqform->seqmin)));
}
if (seqdataform->last_value > seqform->seqmax)
{
- char bufs[100],
- bufm[100];
-
- snprintf(bufs, sizeof(bufs), INT64_FORMAT, seqdataform->last_value);
- snprintf(bufm, sizeof(bufm), INT64_FORMAT, seqform->seqmax);
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("RESTART value (%s) cannot be greater than MAXVALUE (%s)",
- bufs, bufm)));
+ errmsg("RESTART value (%lld) cannot be greater than MAXVALUE (%lld)",
+ (long long int)seqdataform->last_value, (long long int)seqform->seqmax)));
}
/* CACHE */
@@ -1617,13 +1571,10 @@ init_params(ParseState *pstate, List *options, bool for_identity,
seqform->seqcache = defGetInt64(cache_value);
if (seqform->seqcache <= 0)
{
- char buf[100];
-
- snprintf(buf, sizeof(buf), INT64_FORMAT, seqform->seqcache);
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("CACHE (%s) must be greater than zero",
- buf)));
+ errmsg("CACHE (%lld) must be greater than zero",
+ (long long int)seqform->seqcache)));
}
seqdataform->log_cnt = 0;
}
diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c
index 60e57876de..e321a70138 100644
--- a/src/backend/utils/adt/xid8funcs.c
+++ b/src/backend/utils/adt/xid8funcs.c
@@ -113,9 +113,8 @@ TransactionIdInRecentPast(FullTransactionId fxid, TransactionId *extracted_xid)
if (!FullTransactionIdPrecedes(fxid, now_fullxid))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("transaction ID %s is in the future",
- psprintf(UINT64_FORMAT,
- U64FromFullTransactionId(fxid)))));
+ errmsg("transaction ID %llu is in the future",
+ (unsigned long long)U64FromFullTransactionId(fxid))));
/*
* ShmemVariableCache->oldestClogXid is protected by XactTruncationLock,
--
2.35.1
view thread (3+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: [PATCH] Remove workarounds to format [u]int64's
In-Reply-To: <CAJ7c6TMSKi3Xs8h5MP38XOnQQpBLazJvVxVfPn++roitDJcR7g@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox