mirror of
https://github.com/postgres/postgres.git
synced 2025-07-27 12:41:57 +03:00
Improve error messages about mismatching relkind
Most error messages about a relkind that was not supported or appropriate for the command was of the pattern "relation \"%s\" is not a table, foreign table, or materialized view" This style can become verbose and tedious to maintain. Moreover, it's not very helpful: If I'm trying to create a comment on a TOAST table, which is not supported, then the information that I could have created a comment on a materialized view is pointless. Instead, write the primary error message shorter and saying more directly that what was attempted is not possible. Then, in the detail message, explain that the operation is not supported for the relkind the object was. To simplify that, add a new function errdetail_relkind_not_supported() that does this. In passing, make use of RELKIND_HAS_STORAGE() where appropriate, instead of listing out the relkinds individually. Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://www.postgresql.org/message-id/flat/dc35a398-37d0-75ce-07ea-1dd71d98f8ec@2ndquadrant.com
This commit is contained in:
@ -128,8 +128,8 @@ typedef struct HashIndexStat
|
||||
} HashIndexStat;
|
||||
|
||||
static Datum pgstatindex_impl(Relation rel, FunctionCallInfo fcinfo);
|
||||
static int64 pg_relpages_impl(Relation rel);
|
||||
static void GetHashPageStats(Page page, HashIndexStat *stats);
|
||||
static void check_relation_relkind(Relation rel);
|
||||
|
||||
/* ------------------------------------------------------
|
||||
* pgstatindex()
|
||||
@ -383,7 +383,6 @@ Datum
|
||||
pg_relpages(PG_FUNCTION_ARGS)
|
||||
{
|
||||
text *relname = PG_GETARG_TEXT_PP(0);
|
||||
int64 relpages;
|
||||
Relation rel;
|
||||
RangeVar *relrv;
|
||||
|
||||
@ -395,16 +394,7 @@ pg_relpages(PG_FUNCTION_ARGS)
|
||||
relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
|
||||
rel = relation_openrv(relrv, AccessShareLock);
|
||||
|
||||
/* only some relkinds have storage */
|
||||
check_relation_relkind(rel);
|
||||
|
||||
/* note: this will work OK on non-local temp tables */
|
||||
|
||||
relpages = RelationGetNumberOfBlocks(rel);
|
||||
|
||||
relation_close(rel, AccessShareLock);
|
||||
|
||||
PG_RETURN_INT64(relpages);
|
||||
PG_RETURN_INT64(pg_relpages_impl(rel));
|
||||
}
|
||||
|
||||
/* No need for superuser checks in v1.5, see above */
|
||||
@ -412,23 +402,13 @@ Datum
|
||||
pg_relpages_v1_5(PG_FUNCTION_ARGS)
|
||||
{
|
||||
text *relname = PG_GETARG_TEXT_PP(0);
|
||||
int64 relpages;
|
||||
Relation rel;
|
||||
RangeVar *relrv;
|
||||
|
||||
relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
|
||||
rel = relation_openrv(relrv, AccessShareLock);
|
||||
|
||||
/* only some relkinds have storage */
|
||||
check_relation_relkind(rel);
|
||||
|
||||
/* note: this will work OK on non-local temp tables */
|
||||
|
||||
relpages = RelationGetNumberOfBlocks(rel);
|
||||
|
||||
relation_close(rel, AccessShareLock);
|
||||
|
||||
PG_RETURN_INT64(relpages);
|
||||
PG_RETURN_INT64(pg_relpages_impl(rel));
|
||||
}
|
||||
|
||||
/* Must keep superuser() check, see above. */
|
||||
@ -436,7 +416,6 @@ Datum
|
||||
pg_relpagesbyid(PG_FUNCTION_ARGS)
|
||||
{
|
||||
Oid relid = PG_GETARG_OID(0);
|
||||
int64 relpages;
|
||||
Relation rel;
|
||||
|
||||
if (!superuser())
|
||||
@ -446,16 +425,7 @@ pg_relpagesbyid(PG_FUNCTION_ARGS)
|
||||
|
||||
rel = relation_open(relid, AccessShareLock);
|
||||
|
||||
/* only some relkinds have storage */
|
||||
check_relation_relkind(rel);
|
||||
|
||||
/* note: this will work OK on non-local temp tables */
|
||||
|
||||
relpages = RelationGetNumberOfBlocks(rel);
|
||||
|
||||
relation_close(rel, AccessShareLock);
|
||||
|
||||
PG_RETURN_INT64(relpages);
|
||||
PG_RETURN_INT64(pg_relpages_impl(rel));
|
||||
}
|
||||
|
||||
/* No need for superuser checks in v1.5, see above */
|
||||
@ -463,13 +433,24 @@ Datum
|
||||
pg_relpagesbyid_v1_5(PG_FUNCTION_ARGS)
|
||||
{
|
||||
Oid relid = PG_GETARG_OID(0);
|
||||
int64 relpages;
|
||||
Relation rel;
|
||||
|
||||
rel = relation_open(relid, AccessShareLock);
|
||||
|
||||
/* only some relkinds have storage */
|
||||
check_relation_relkind(rel);
|
||||
PG_RETURN_INT64(pg_relpages_impl(rel));
|
||||
}
|
||||
|
||||
static int64
|
||||
pg_relpages_impl(Relation rel)
|
||||
{
|
||||
int64 relpages;
|
||||
|
||||
if (!RELKIND_HAS_STORAGE(rel->rd_rel->relkind))
|
||||
ereport(ERROR,
|
||||
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
|
||||
errmsg("cannot get page count of relation \"%s\"",
|
||||
RelationGetRelationName(rel)),
|
||||
errdetail_relkind_not_supported(rel->rd_rel->relkind)));
|
||||
|
||||
/* note: this will work OK on non-local temp tables */
|
||||
|
||||
@ -477,7 +458,7 @@ pg_relpagesbyid_v1_5(PG_FUNCTION_ARGS)
|
||||
|
||||
relation_close(rel, AccessShareLock);
|
||||
|
||||
PG_RETURN_INT64(relpages);
|
||||
return relpages;
|
||||
}
|
||||
|
||||
/* ------------------------------------------------------
|
||||
@ -754,21 +735,3 @@ GetHashPageStats(Page page, HashIndexStat *stats)
|
||||
}
|
||||
stats->free_space += PageGetExactFreeSpace(page);
|
||||
}
|
||||
|
||||
/*
|
||||
* check_relation_relkind - convenience routine to check that relation
|
||||
* is of the relkind supported by the callers
|
||||
*/
|
||||
static void
|
||||
check_relation_relkind(Relation rel)
|
||||
{
|
||||
if (rel->rd_rel->relkind != RELKIND_RELATION &&
|
||||
rel->rd_rel->relkind != RELKIND_INDEX &&
|
||||
rel->rd_rel->relkind != RELKIND_MATVIEW &&
|
||||
rel->rd_rel->relkind != RELKIND_SEQUENCE &&
|
||||
rel->rd_rel->relkind != RELKIND_TOASTVALUE)
|
||||
ereport(ERROR,
|
||||
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
|
||||
errmsg("\"%s\" is not a table, index, materialized view, sequence, or TOAST table",
|
||||
RelationGetRelationName(rel))));
|
||||
}
|
||||
|
Reference in New Issue
Block a user