Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 8d8ff5f

Browse files
committed
Improve error message for rejecting RETURNING clauses with dropped columns.
This error message was written with only ON SELECT rules in mind, but since then we also made RETURNING-clause targetlists go through the same logic. This means that you got a rather off-topic error message if you tried to add a rule with RETURNING to a table having dropped columns. Ideally we'd just support that, but some preliminary investigation says that it might be a significant amount of work. Seeing that Nicklas Avén's complaint is the first one we've gotten about this in the ten years or so that the code's been like that, I'm unwilling to put much time into it. Instead, improve the error report by issuing a different message for RETURNING cases, and revise the associated comment based on this investigation. Discussion: 1456176604.17219.9.camel@jordogskog.no
1 parent 5847397 commit 8d8ff5f

File tree

1 file changed

+19
-7
lines changed

1 file changed

+19
-7
lines changed

src/backend/rewrite/rewriteDefine.c

+19-7
Original file line numberDiff line numberDiff line change
@@ -671,17 +671,29 @@ checkRuleResultList(List *targetList, TupleDesc resultDesc, bool isSelect,
671671
attname = NameStr(attr->attname);
672672

673673
/*
674-
* Disallow dropped columns in the relation. This won't happen in the
675-
* cases we actually care about (namely creating a view via CREATE
676-
* TABLE then CREATE RULE, or adding a RETURNING rule to a view).
677-
* Trying to cope with it is much more trouble than it's worth,
678-
* because we'd have to modify the rule to insert dummy NULLs at the
679-
* right positions.
674+
* Disallow dropped columns in the relation. This is not really
675+
* expected to happen when creating an ON SELECT rule. It'd be
676+
* possible if someone tried to convert a relation with dropped
677+
* columns to a view, but the only case we care about supporting
678+
* table-to-view conversion for is pg_dump, and pg_dump won't do that.
679+
*
680+
* Unfortunately, the situation is also possible when adding a rule
681+
* with RETURNING to a regular table, and rejecting that case is
682+
* altogether more annoying. In principle we could support it by
683+
* modifying the targetlist to include dummy NULL columns
684+
* corresponding to the dropped columns in the tupdesc. However,
685+
* places like ruleutils.c would have to be fixed to not process such
686+
* entries, and that would take an uncertain and possibly rather large
687+
* amount of work. (Note we could not dodge that by marking the dummy
688+
* columns resjunk, since it's precisely the non-resjunk tlist columns
689+
* that are expected to correspond to table columns.)
680690
*/
681691
if (attr->attisdropped)
682692
ereport(ERROR,
683693
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
684-
errmsg("cannot convert relation containing dropped columns to view")));
694+
isSelect ?
695+
errmsg("cannot convert relation containing dropped columns to view") :
696+
errmsg("cannot create a RETURNING list for a relation containing dropped columns")));
685697

686698
/* Check name match if required; no need for two error texts here */
687699
if (requireColumnNameMatch && strcmp(tle->resname, attname) != 0)

0 commit comments

Comments
 (0)