On Fri, May 03, 2019 at 09:42:17PM +0200, Tomas Vondra wrote:
On Fri, May 03, 2019 at 12:21:36PM -0400, Tom Lane wrote:
Alvaro Herrera <alvhe...@2ndquadrant.com> writes:
Error reporting in extended statistics is inconsistent -- many messages
that are ereport() in mvdistinct.c are elog() in the other modules.
...
I think this should be cleaned up, while at the same time not giving too
much hassle for translators; for example, this message
dependencies.c: elog(ERROR, "invalid MVDependencies size %zd (expected at least
%zd)",
should not only be turned into an ereport(), but also the MVDependencies
part turned into a %s. (Alternatively, we could decide I was wrong and
turn them all back into elogs, but I obviously vote against that.)
FWIW, I'd vote the other way: that seems like a clear "internal error",
so making translators deal with it is just make-work. It should be an
elog. If there's a reasonably plausible way for a user to trigger an
error condition, then yes ereport, but if we're reporting situations
that couldn't happen without a server bug then elog seems fine.
Yeah, I agree. Most of (peshaps all) those errors are internal errors,
and thus should be elog. I'll take care of clening this up a bit.
OK, so here is a patch, using elog() for all places except for the
input function, where we simply report we don't accept those values.
I agree those are internal errors, usually meaning the statistics object
was either corrupted or there's a bug in how it's built/serialized.
Users should not be able to trigger those cases (the only thing I can
think of is sending a bogus value through send/recv functions, that
simply do byteaout/byteain).
Now, what about backpatch? It's a small tweak, but it makes the life a
bit easier for translators ...
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/statistics/dependencies.c
b/src/backend/statistics/dependencies.c
index 0b26e4166d..d2464381c4 100644
--- a/src/backend/statistics/dependencies.c
+++ b/src/backend/statistics/dependencies.c
@@ -534,9 +534,9 @@ statext_dependencies_deserialize(bytea *data)
dependencies->type, STATS_DEPS_TYPE_BASIC);
if (dependencies->ndeps == 0)
- ereport(ERROR,
- (errcode(ERRCODE_DATA_CORRUPTED),
- errmsg("invalid zero-length item array in
MVDependencies")));
+ elog(ERROR,
+ (errcode(ERRCODE_DATA_CORRUPTED),
+ errmsg("invalid zero-length item array in
MVDependencies")));
/* what minimum bytea size do we expect for those parameters */
min_expected_size = SizeOfItem(dependencies->ndeps);
diff --git a/src/backend/statistics/mvdistinct.c
b/src/backend/statistics/mvdistinct.c
index 133503cb9b..fa410c95c6 100644
--- a/src/backend/statistics/mvdistinct.c
+++ b/src/backend/statistics/mvdistinct.c
@@ -274,27 +274,27 @@ statext_ndistinct_deserialize(bytea *data)
tmp += sizeof(uint32);
if (ndist.magic != STATS_NDISTINCT_MAGIC)
- ereport(ERROR,
- (errcode(ERRCODE_DATA_CORRUPTED),
- errmsg("invalid ndistinct magic %08x (expected
%08x)",
- ndist.magic,
STATS_NDISTINCT_MAGIC)));
+ elog(ERROR,
+ (errcode(ERRCODE_DATA_CORRUPTED),
+ errmsg("invalid ndistinct magic %08x (expected %08x)",
+ ndist.magic, STATS_NDISTINCT_MAGIC)));
if (ndist.type != STATS_NDISTINCT_TYPE_BASIC)
- ereport(ERROR,
- (errcode(ERRCODE_DATA_CORRUPTED),
- errmsg("invalid ndistinct type %d (expected
%d)",
- ndist.type,
STATS_NDISTINCT_TYPE_BASIC)));
+ elog(ERROR,
+ (errcode(ERRCODE_DATA_CORRUPTED),
+ errmsg("invalid ndistinct type %d (expected %d)",
+ ndist.type,
STATS_NDISTINCT_TYPE_BASIC)));
if (ndist.nitems == 0)
- ereport(ERROR,
- (errcode(ERRCODE_DATA_CORRUPTED),
- errmsg("invalid zero-length item array in
MVNDistinct")));
+ elog(ERROR,
+ (errcode(ERRCODE_DATA_CORRUPTED),
+ errmsg("invalid zero-length item array in
MVNDistinct")));
/* what minimum bytea size do we expect for those parameters */
minimum_size = MinSizeOfItems(ndist.nitems);
if (VARSIZE_ANY_EXHDR(data) < minimum_size)
- ereport(ERROR,
- (errcode(ERRCODE_DATA_CORRUPTED),
- errmsg("invalid MVNDistinct size %zd (expected
at least %zd)",
- VARSIZE_ANY_EXHDR(data),
minimum_size)));
+ elog(ERROR,
+ (errcode(ERRCODE_DATA_CORRUPTED),
+ errmsg("invalid MVNDistinct size %zd (expected at
least %zd)",
+ VARSIZE_ANY_EXHDR(data),
minimum_size)));
/*
* Allocate space for the ndistinct items (no space for each item's
