Hello, here's a pretty trivial cleanup.
Currently, you have to pass the errmsg text to convert_tuples_by_name
and convert_tuples_by_position that's going to be raised if the tuple
descriptors don't match. In the latter's case that makes sense, as each
case is pretty specific and tailored messages can be offered, so this is
useful.
However, in the case of convert_tuples_by_name, it seems we don't have
enough control over what is being called, so there's no way to
produce tailored messages -- all the callers are using the same generic
wording: "could not convert row type".
This code was introduced by dcb2bda9b704; I think back then we were
thinking that it would be possible to give different error messages for
different cases (as convert_tuples_by_position was already doing then),
however it seems clear now that that'll never happen.
I propose we get rid of it by having convert_tuples_by_name supply the
error message by itself, as in the attached patch.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From eb4da7ddd5c596ec8585761f02e55d2f03262f62 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Wed, 26 Jun 2019 16:36:50 -0400
Subject: [PATCH] Remove 'msg' param from convert_tuples_by_name and friends
---
src/backend/access/common/tupconvert.c | 17 +++++++----------
src/backend/catalog/partition.c | 3 +--
src/backend/commands/analyze.c | 3 +--
src/backend/commands/indexcmds.c | 3 +--
src/backend/commands/tablecmds.c | 24 ++++++++----------------
src/backend/executor/execExprInterp.c | 4 +---
src/backend/executor/execMain.c | 12 ++++--------
src/backend/executor/execPartition.c | 18 ++++++------------
src/backend/executor/nodeModifyTable.c | 3 +--
src/include/access/tupconvert.h | 9 +++------
10 files changed, 33 insertions(+), 63 deletions(-)
diff --git a/src/backend/access/common/tupconvert.c b/src/backend/access/common/tupconvert.c
index 8cda16431c..0ec9cd5870 100644
--- a/src/backend/access/common/tupconvert.c
+++ b/src/backend/access/common/tupconvert.c
@@ -203,15 +203,14 @@ convert_tuples_by_position(TupleDesc indesc,
*/
TupleConversionMap *
convert_tuples_by_name(TupleDesc indesc,
- TupleDesc outdesc,
- const char *msg)
+ TupleDesc outdesc)
{
TupleConversionMap *map;
AttrNumber *attrMap;
int n = outdesc->natts;
/* Verify compatibility and prepare attribute-number map */
- attrMap = convert_tuples_by_name_map_if_req(indesc, outdesc, msg);
+ attrMap = convert_tuples_by_name_map_if_req(indesc, outdesc);
if (attrMap == NULL)
{
@@ -244,8 +243,7 @@ convert_tuples_by_name(TupleDesc indesc,
*/
AttrNumber *
convert_tuples_by_name_map(TupleDesc indesc,
- TupleDesc outdesc,
- const char *msg)
+ TupleDesc outdesc)
{
AttrNumber *attrMap;
int outnatts;
@@ -299,7 +297,7 @@ convert_tuples_by_name_map(TupleDesc indesc,
if (atttypid != inatt->atttypid || atttypmod != inatt->atttypmod)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg_internal("%s", _(msg)),
+ errmsg("could not convert row type"),
errdetail("Attribute \"%s\" of type %s does not match corresponding attribute of type %s.",
attname,
format_type_be(outdesc->tdtypeid),
@@ -311,7 +309,7 @@ convert_tuples_by_name_map(TupleDesc indesc,
if (attrMap[i] == 0)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg_internal("%s", _(msg)),
+ errmsg("could not convert row type"),
errdetail("Attribute \"%s\" of type %s does not exist in type %s.",
attname,
format_type_be(outdesc->tdtypeid),
@@ -327,8 +325,7 @@ convert_tuples_by_name_map(TupleDesc indesc,
*/
AttrNumber *
convert_tuples_by_name_map_if_req(TupleDesc indesc,
- TupleDesc outdesc,
- const char *msg)
+ TupleDesc outdesc)
{
AttrNumber *attrMap;
int n = outdesc->natts;
@@ -336,7 +333,7 @@ convert_tuples_by_name_map_if_req(TupleDesc indesc,
bool same;
/* Verify compatibility and prepare attribute-number map */
- attrMap = convert_tuples_by_name_map(indesc, outdesc, msg);
+ attrMap = convert_tuples_by_name_map(indesc, outdesc);
/*
* Check to see if the map is one-to-one, in which case we need not do a
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 07b8223348..53af14e939 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -210,8 +210,7 @@ map_partition_varattnos(List *expr, int fromrel_varno,
AttrNumber *part_attnos;
part_attnos = convert_tuples_by_name_map(RelationGetDescr(to_rel),
- RelationGetDescr(from_rel),
- gettext_noop("could not convert row type"));
+ RelationGetDescr(from_rel));
expr = (List *) map_variable_attnos((Node *) expr,
fromrel_varno, 0,
part_attnos,
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 92285e848f..d2fdf447b6 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -1356,8 +1356,7 @@ acquire_inherited_sample_rows(Relation onerel, int elevel,
TupleConversionMap *map;
map = convert_tuples_by_name(RelationGetDescr(childrel),
- RelationGetDescr(onerel),
- gettext_noop("could not convert row type"));
+ RelationGetDescr(onerel));
if (map != NULL)
{
int j;
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8dc7a39870..03a2598912 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1086,8 +1086,7 @@ DefineIndex(Oid relationId,
childidxs = RelationGetIndexList(childrel);
attmap =
convert_tuples_by_name_map(RelationGetDescr(childrel),
- parentDesc,
- gettext_noop("could not convert row type"));
+ parentDesc);
maplen = parentDesc->natts;
foreach(cell, childidxs)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index fb2be10794..11c9f6eabe 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1087,8 +1087,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
}
attmap = convert_tuples_by_name_map(RelationGetDescr(rel),
- RelationGetDescr(parent),
- gettext_noop("could not convert row type"));
+ RelationGetDescr(parent));
idxstmt =
generateClonedIndexStmt(NULL, idxRel,
attmap, RelationGetDescr(rel)->natts,
@@ -8171,8 +8170,7 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel,
* definition to match the partition's column layout.
*/
map = convert_tuples_by_name_map_if_req(RelationGetDescr(partRel),
- RelationGetDescr(pkrel),
- gettext_noop("could not convert row type"));
+ RelationGetDescr(pkrel));
if (map)
{
mapped_pkattnum = palloc(sizeof(AttrNumber) * numfks);
@@ -8314,8 +8312,7 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel,
CheckTableNotInUse(partition, "ALTER TABLE");
attmap = convert_tuples_by_name_map(RelationGetDescr(partition),
- RelationGetDescr(rel),
- gettext_noop("could not convert row type"));
+ RelationGetDescr(rel));
for (int j = 0; j < numfks; j++)
mapped_fkattnum[j] = attmap[fkattnum[j] - 1];
@@ -8505,8 +8502,7 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
table_close(pg_constraint, RowShareLock);
attmap = convert_tuples_by_name_map(RelationGetDescr(partitionRel),
- RelationGetDescr(parentRel),
- gettext_noop("could not convert row type"));
+ RelationGetDescr(parentRel));
foreach(cell, clone)
{
Oid constrOid = lfirst_oid(cell);
@@ -8642,8 +8638,7 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel)
* different. This map is used to convert them.
*/
attmap = convert_tuples_by_name_map(RelationGetDescr(partRel),
- RelationGetDescr(parentRel),
- gettext_noop("could not convert row type"));
+ RelationGetDescr(parentRel));
partFKs = copyObject(RelationGetFKeyList(partRel));
@@ -10440,8 +10435,7 @@ ATPrepAlterColumnType(List **wqueue,
cmd = copyObject(cmd);
attmap = convert_tuples_by_name_map(RelationGetDescr(childrel),
- RelationGetDescr(rel),
- gettext_noop("could not convert row type"));
+ RelationGetDescr(rel));
((ColumnDef *) cmd->def)->cooked_default =
map_variable_attnos(def->cooked_default,
1, 0,
@@ -15812,8 +15806,7 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel)
/* construct an indexinfo to compare existing indexes against */
info = BuildIndexInfo(idxRel);
attmap = convert_tuples_by_name_map(RelationGetDescr(attachrel),
- RelationGetDescr(rel),
- gettext_noop("could not convert row type"));
+ RelationGetDescr(rel));
constraintOid = get_relation_idx_constraint_oid(RelationGetRelid(rel), idx);
/*
@@ -16389,8 +16382,7 @@ ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name)
childInfo = BuildIndexInfo(partIdx);
parentInfo = BuildIndexInfo(parentIdx);
attmap = convert_tuples_by_name_map(RelationGetDescr(partTbl),
- RelationGetDescr(parentTbl),
- gettext_noop("could not convert row type"));
+ RelationGetDescr(parentTbl));
if (!CompareIndexInfo(childInfo, parentInfo,
partIdx->rd_indcollation,
parentIdx->rd_indcollation,
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 66a67c72b2..5d8bd7aec5 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -3298,9 +3298,7 @@ ExecEvalConvertRowtype(ExprState *state, ExprEvalStep *op, ExprContext *econtext
old_cxt = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
/* prepare map from old to new attribute numbers */
- op->d.convert_rowtype.map =
- convert_tuples_by_name(indesc, outdesc,
- gettext_noop("could not convert row type"));
+ op->d.convert_rowtype.map = convert_tuples_by_name(indesc, outdesc);
op->d.convert_rowtype.initialized = true;
MemoryContextSwitchTo(old_cxt);
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index dbd7dd9bcd..3af15f4636 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1861,8 +1861,7 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
old_tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
/* a reverse map */
- map = convert_tuples_by_name_map_if_req(old_tupdesc, tupdesc,
- gettext_noop("could not convert row type"));
+ map = convert_tuples_by_name_map_if_req(old_tupdesc, tupdesc);
/*
* Partition-specific slot's tupdesc can't be changed, so allocate a
@@ -1947,8 +1946,7 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
tupdesc = RelationGetDescr(rel);
/* a reverse map */
map = convert_tuples_by_name_map_if_req(orig_tupdesc,
- tupdesc,
- gettext_noop("could not convert row type"));
+ tupdesc);
/*
* Partition-specific slot's tupdesc can't be changed, so
@@ -1997,8 +1995,7 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
tupdesc = RelationGetDescr(rel);
/* a reverse map */
map = convert_tuples_by_name_map_if_req(old_tupdesc,
- tupdesc,
- gettext_noop("could not convert row type"));
+ tupdesc);
/*
* Partition-specific slot's tupdesc can't be changed, so
@@ -2105,8 +2102,7 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
tupdesc = RelationGetDescr(rel);
/* a reverse map */
map = convert_tuples_by_name_map_if_req(old_tupdesc,
- tupdesc,
- gettext_noop("could not convert row type"));
+ tupdesc);
/*
* Partition-specific slot's tupdesc can't be changed,
diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index 729dc396a9..d23f292cb0 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -586,8 +586,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
*/
part_attnos =
convert_tuples_by_name_map(RelationGetDescr(partrel),
- RelationGetDescr(firstResultRel),
- gettext_noop("could not convert row type"));
+ RelationGetDescr(firstResultRel));
wcoList = (List *)
map_variable_attnos((Node *) wcoList,
firstVarno, 0,
@@ -646,8 +645,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
if (part_attnos == NULL)
part_attnos =
convert_tuples_by_name_map(RelationGetDescr(partrel),
- RelationGetDescr(firstResultRel),
- gettext_noop("could not convert row type"));
+ RelationGetDescr(firstResultRel));
returningList = (List *)
map_variable_attnos((Node *) returningList,
firstVarno, 0,
@@ -790,8 +788,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
if (part_attnos == NULL)
part_attnos =
convert_tuples_by_name_map(RelationGetDescr(partrel),
- RelationGetDescr(firstResultRel),
- gettext_noop("could not convert row type"));
+ RelationGetDescr(firstResultRel));
onconflset = (List *)
map_variable_attnos((Node *) onconflset,
INNER_VAR, 0,
@@ -904,8 +901,7 @@ ExecInitRoutingInfo(ModifyTableState *mtstate,
*/
partrouteinfo->pi_RootToPartitionMap =
convert_tuples_by_name(RelationGetDescr(partRelInfo->ri_PartitionRoot),
- RelationGetDescr(partRelInfo->ri_RelationDesc),
- gettext_noop("could not convert row type"));
+ RelationGetDescr(partRelInfo->ri_RelationDesc));
/*
* If a partition has a different rowtype than the root parent, initialize
@@ -937,8 +933,7 @@ ExecInitRoutingInfo(ModifyTableState *mtstate,
{
partrouteinfo->pi_PartitionToRootMap =
convert_tuples_by_name(RelationGetDescr(partRelInfo->ri_RelationDesc),
- RelationGetDescr(partRelInfo->ri_PartitionRoot),
- gettext_noop("could not convert row type"));
+ RelationGetDescr(partRelInfo->ri_PartitionRoot));
}
else
partrouteinfo->pi_PartitionToRootMap = NULL;
@@ -1042,8 +1037,7 @@ ExecInitPartitionDispatchInfo(EState *estate,
* routing.
*/
pd->tupmap = convert_tuples_by_name_map_if_req(RelationGetDescr(parent_pd->reldesc),
- tupdesc,
- gettext_noop("could not convert row type"));
+ tupdesc);
pd->tupslot = pd->tupmap ?
MakeSingleTupleTableSlot(tupdesc, &TTSOpsVirtual) : NULL;
}
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 9e0c8794c4..01fe11aa68 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1959,8 +1959,7 @@ ExecSetupChildParentMapForSubplan(ModifyTableState *mtstate)
{
mtstate->mt_per_subplan_tupconv_maps[i] =
convert_tuples_by_name(RelationGetDescr(resultRelInfos[i].ri_RelationDesc),
- outdesc,
- gettext_noop("could not convert row type"));
+ outdesc);
}
}
diff --git a/src/include/access/tupconvert.h b/src/include/access/tupconvert.h
index 509e5c44ef..6d095f8e0d 100644
--- a/src/include/access/tupconvert.h
+++ b/src/include/access/tupconvert.h
@@ -36,15 +36,12 @@ extern TupleConversionMap *convert_tuples_by_position(TupleDesc indesc,
const char *msg);
extern TupleConversionMap *convert_tuples_by_name(TupleDesc indesc,
- TupleDesc outdesc,
- const char *msg);
+ TupleDesc outdesc);
extern AttrNumber *convert_tuples_by_name_map(TupleDesc indesc,
- TupleDesc outdesc,
- const char *msg);
+ TupleDesc outdesc);
extern AttrNumber *convert_tuples_by_name_map_if_req(TupleDesc indesc,
- TupleDesc outdesc,
- const char *msg);
+ TupleDesc outdesc);
extern HeapTuple execute_attr_map_tuple(HeapTuple tuple, TupleConversionMap *map);
extern TupleTableSlot *execute_attr_map_slot(AttrNumber *attrMap,
--
2.17.1
