On 28.02.23 07:15, Bharath Rupireddy wrote:
Going through the remaining report_invalid_record() calls I then
adjusted the use of "invalid" vs. "incorrect" in one case. The message
"record with invalid length" makes it sound like the length was
something like -5, but really we know what the length should be and what
we got wasn't it, so "incorrect" sounded better and is also used in
other error messages in that file.
I have no strong opinion about this change. We seem to be using
"invalid length" and "incorrect length" interchangeably [1] without
distinguishing between "invalid" if length is < 0 and "incorrect" if
length >= 0 and not something we're expecting.
Right, this isn't handled very consistently. I did a pass across all
"{invalid|incorrect|wrong} {length|size}" messages and tried to make
them more precise by adding more detail and using the appropriate word.
What do you think about the attached patch?
From 24876cad15d43fde310ad12a801aec09c35e18a5 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 2 Mar 2023 08:54:22 +0100
Subject: [PATCH v1] Improve various error messages
Update error messages "{invalid|incorrect|wrong} {length|size}" to be
more precise by using the appropriate adjective and adding more detail
if available.
---
src/backend/access/transam/twophase.c | 10 ++++------
src/backend/access/transam/xlogreader.c | 2 +-
src/backend/access/transam/xlogrecovery.c | 5 ++++-
src/backend/postmaster/postmaster.c | 8 ++++++--
src/backend/replication/logical/worker.c | 2 +-
src/backend/statistics/mcv.c | 19 +++++++------------
src/backend/utils/adt/network.c | 6 ++++--
src/backend/utils/adt/tsquery.c | 2 +-
src/backend/utils/adt/tsvector.c | 2 +-
src/backend/utils/adt/varbit.c | 6 ++++--
src/pl/plpython/expected/plpython_types.out | 2 +-
src/pl/plpython/plpy_typeio.c | 2 +-
src/test/modules/test_rbtree/test_rbtree.c | 2 +-
13 files changed, 36 insertions(+), 32 deletions(-)
diff --git a/src/backend/access/transam/twophase.c
b/src/backend/access/transam/twophase.c
index 068e59bec0..e644a0c4d3 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1319,10 +1319,8 @@ ReadTwoPhaseFile(TransactionId xid, bool missing_ok)
stat.st_size > MaxAllocSize)
ereport(ERROR,
(errcode(ERRCODE_DATA_CORRUPTED),
- errmsg_plural("incorrect size of file \"%s\":
%lld byte",
- "incorrect size of
file \"%s\": %lld bytes",
- (long long int)
stat.st_size, path,
- (long long int)
stat.st_size)));
+ errmsg("size of file \"%s\" (%lld) out of
valid range",
+ path, (long long int)
stat.st_size)));
crc_offset = stat.st_size - sizeof(pg_crc32c);
if (crc_offset != MAXALIGN(crc_offset))
@@ -1367,8 +1365,8 @@ ReadTwoPhaseFile(TransactionId xid, bool missing_ok)
if (hdr->total_len != stat.st_size)
ereport(ERROR,
(errcode(ERRCODE_DATA_CORRUPTED),
- errmsg("invalid size stored in file \"%s\"",
- path)));
+ errmsg("incorrect size stored in file \"%s\"",
path),
+ errdetail("Expected %lld, got %u.", (long long
int) stat.st_size, hdr->total_len)));
INIT_CRC32C(calc_crc);
COMP_CRC32C(calc_crc, buf, crc_offset);
diff --git a/src/backend/access/transam/xlogreader.c
b/src/backend/access/transam/xlogreader.c
index aa6c929477..f431a84fbc 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1942,7 +1942,7 @@ DecodeXLogRecord(XLogReaderState *state,
shortdata_err:
report_invalid_record(state,
- "record with invalid length
at %X/%X",
+ "record with incorrect length
at %X/%X",
LSN_FORMAT_ARGS(state->ReadRecPtr));
err:
*errormsg = state->errormsg_buf;
diff --git a/src/backend/access/transam/xlogrecovery.c
b/src/backend/access/transam/xlogrecovery.c
index dbe9394762..7d5b8b9775 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -3984,7 +3984,10 @@ ReadCheckpointRecord(XLogPrefetcher *xlogprefetcher,
XLogRecPtr RecPtr,
if (record->xl_tot_len != SizeOfXLogRecord +
SizeOfXLogRecordDataHeaderShort + sizeof(CheckPoint))
{
ereport(LOG,
- (errmsg("invalid length of checkpoint
record")));
+ (errmsg("incorrect length of checkpoint
record"),
+ errdetail("Expected %zu, got %u.",
+ SizeOfXLogRecord +
SizeOfXLogRecordDataHeaderShort + sizeof(CheckPoint),
+ record->xl_tot_len)));
return NULL;
}
return record;
diff --git a/src/backend/postmaster/postmaster.c
b/src/backend/postmaster/postmaster.c
index 2552327d90..1080a72c8d 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1993,7 +1993,9 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool
gss_done)
{
ereport(COMMERROR,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
- errmsg("invalid length of startup packet")));
+ errmsg("invalid length of startup packet"),
+ errdetail("Expected %d..%d, got %d.",
+ (int32)
sizeof(ProtocolVersion), MAX_STARTUP_PACKET_LENGTH, len)));
return STATUS_ERROR;
}
@@ -2026,7 +2028,9 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool
gss_done)
{
ereport(COMMERROR,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
- errmsg("invalid length of startup
packet")));
+ errmsg("incorrect length of cancel
request packet"),
+ errdetail("Expected %zu, got %d.",
+
sizeof(CancelRequestPacket), len)));
return STATUS_ERROR;
}
processCancelRequest(port, buf);
diff --git a/src/backend/replication/logical/worker.c
b/src/backend/replication/logical/worker.c
index cfb2ab6248..62b3fb56f6 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -2090,7 +2090,7 @@ apply_spooled_messages(FileSet *stream_fileset,
TransactionId xid,
/* do we have a correct length? */
if (len <= 0)
- elog(ERROR, "incorrect length %d in streaming
transaction's changes file \"%s\"",
+ elog(ERROR, "invalid length %d in streaming
transaction's changes file \"%s\"",
len, path);
/* make sure we have sufficiently large buffer */
diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c
index 03b9f04bb5..5590f78c6d 100644
--- a/src/backend/statistics/mcv.c
+++ b/src/backend/statistics/mcv.c
@@ -1064,18 +1064,13 @@ statext_mcv_deserialize(bytea *data)
elog(ERROR, "invalid MCV type %u (expected %u)",
mcvlist->type, STATS_MCV_TYPE_BASIC);
- if (mcvlist->ndimensions == 0)
- elog(ERROR, "invalid zero-length dimension array in MCVList");
- else if ((mcvlist->ndimensions > STATS_MAX_DIMENSIONS) ||
- (mcvlist->ndimensions < 0))
- elog(ERROR, "invalid length (%d) dimension array in MCVList",
- mcvlist->ndimensions);
-
- if (mcvlist->nitems == 0)
- elog(ERROR, "invalid zero-length item array in MCVList");
- else if (mcvlist->nitems > STATS_MCVLIST_MAX_ITEMS)
- elog(ERROR, "invalid length (%u) item array in MCVList",
- mcvlist->nitems);
+ if (mcvlist->ndimensions < 1 || mcvlist->ndimensions >
STATS_MAX_DIMENSIONS)
+ elog(ERROR, "length of dimension array in MCVList (%d) out of
valid range (%d..%d)",
+ mcvlist->ndimensions, 1, STATS_MAX_DIMENSIONS);
+
+ if (mcvlist->nitems < 1 || mcvlist->nitems > STATS_MCVLIST_MAX_ITEMS)
+ elog(ERROR, "length of item array in MCVList (%u) out of valid
range (%d..%d)",
+ mcvlist->nitems, 1, STATS_MCVLIST_MAX_ITEMS);
nitems = mcvlist->nitems;
ndims = mcvlist->ndimensions;
diff --git a/src/backend/utils/adt/network.c b/src/backend/utils/adt/network.c
index 42a4d9d44e..361e6e19af 100644
--- a/src/backend/utils/adt/network.c
+++ b/src/backend/utils/adt/network.c
@@ -222,8 +222,10 @@ network_recv(StringInfo buf, bool is_cidr)
ereport(ERROR,
(errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
/* translator: %s is inet or cidr */
- errmsg("invalid length in external \"%s\"
value",
- is_cidr ? "cidr" : "inet")));
+ errmsg("incorrect length in external \"%s\"
value",
+ is_cidr ? "cidr" : "inet"),
+ errdetail("Expected %d, got %d.",
+ ip_addrsize(addr), nb)));
addrptr = (char *) ip_addr(addr);
for (i = 0; i < nb; i++)
diff --git a/src/backend/utils/adt/tsquery.c b/src/backend/utils/adt/tsquery.c
index 67ad876a27..a6fa770c95 100644
--- a/src/backend/utils/adt/tsquery.c
+++ b/src/backend/utils/adt/tsquery.c
@@ -1242,7 +1242,7 @@ tsqueryrecv(PG_FUNCTION_ARGS)
size = pq_getmsgint(buf, sizeof(uint32));
if (size > (MaxAllocSize / sizeof(QueryItem)))
- elog(ERROR, "invalid size of tsquery");
+ elog(ERROR, "size of tsquery (%u) out of valid range", size);
/* Allocate space to temporarily hold operand strings */
operands = palloc(size * sizeof(char *));
diff --git a/src/backend/utils/adt/tsvector.c b/src/backend/utils/adt/tsvector.c
index 0e66f362c3..c0ce9282b9 100644
--- a/src/backend/utils/adt/tsvector.c
+++ b/src/backend/utils/adt/tsvector.c
@@ -462,7 +462,7 @@ tsvectorrecv(PG_FUNCTION_ARGS)
nentries = pq_getmsgint(buf, sizeof(int32));
if (nentries < 0 || nentries > (MaxAllocSize / sizeof(WordEntry)))
- elog(ERROR, "invalid size of tsvector");
+ elog(ERROR, "size of tsvector (%d) out of valid range",
nentries);
hdrlen = DATAHDRSIZE + sizeof(WordEntry) * nentries;
diff --git a/src/backend/utils/adt/varbit.c b/src/backend/utils/adt/varbit.c
index 3dbbd1207f..2da5d6ebea 100644
--- a/src/backend/utils/adt/varbit.c
+++ b/src/backend/utils/adt/varbit.c
@@ -344,7 +344,8 @@ bit_recv(PG_FUNCTION_ARGS)
if (bitlen < 0 || bitlen > VARBITMAXLEN)
ereport(ERROR,
(errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
- errmsg("invalid length in external bit
string")));
+ errmsg("length in external bit string (%d) out
of valid range (%d..%d)",
+ bitlen, 0, VARBITMAXLEN)));
/*
* Sometimes atttypmod is not supplied. If it is supplied we need to
make
@@ -649,7 +650,8 @@ varbit_recv(PG_FUNCTION_ARGS)
if (bitlen < 0 || bitlen > VARBITMAXLEN)
ereport(ERROR,
(errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
- errmsg("invalid length in external bit
string")));
+ errmsg("length in external bit string (%d) out
of valid range (%d..%d)",
+ bitlen, 0, VARBITMAXLEN)));
/*
* Sometimes atttypmod is not supplied. If it is supplied we need to
make
diff --git a/src/pl/plpython/expected/plpython_types.out
b/src/pl/plpython/expected/plpython_types.out
index a470911c2e..fc451d86b4 100644
--- a/src/pl/plpython/expected/plpython_types.out
+++ b/src/pl/plpython/expected/plpython_types.out
@@ -691,7 +691,7 @@ CREATE FUNCTION test_type_conversion_mdarray_malformed()
RETURNS int[] AS $$
return [[1,2,3],[4,5]]
$$ LANGUAGE plpython3u;
SELECT * FROM test_type_conversion_mdarray_malformed();
-ERROR: wrong length of inner sequence: has length 2, but 3 was expected
+ERROR: incorrect length of inner sequence: has length 2, but 3 was expected
DETAIL: To construct a multidimensional array, the inner sequences must all
have the same length.
CONTEXT: while creating return value
PL/Python function "test_type_conversion_mdarray_malformed"
diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c
index 7018c9d404..66850a00cb 100644
--- a/src/pl/plpython/plpy_typeio.c
+++ b/src/pl/plpython/plpy_typeio.c
@@ -1258,7 +1258,7 @@ PLySequence_ToArray_recurse(PLyObToDatum *elm, PyObject
*list,
if (PySequence_Length(list) != dims[dim])
ereport(ERROR,
(errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR),
- errmsg("wrong length of inner sequence: has
length %d, but %d was expected",
+ errmsg("incorrect length of inner sequence:
has length %d, but %d was expected",
(int) PySequence_Length(list),
dims[dim]),
(errdetail("To construct a multidimensional
array, the inner sequences must all have the same length."))));
diff --git a/src/test/modules/test_rbtree/test_rbtree.c
b/src/test/modules/test_rbtree/test_rbtree.c
index e4eb154378..99c288571a 100644
--- a/src/test/modules/test_rbtree/test_rbtree.c
+++ b/src/test/modules/test_rbtree/test_rbtree.c
@@ -505,7 +505,7 @@ test_rb_tree(PG_FUNCTION_ARGS)
int size = PG_GETARG_INT32(0);
if (size <= 0 || size > MaxAllocSize / sizeof(int))
- elog(ERROR, "invalid size for test_rb_tree: %d", size);
+ elog(ERROR, "size for test_rb_tree out of valid range: %d",
size);
testleftright(size);
testrightleft(size);
testfind(size);
--
2.39.2
