From: Tom Lane <tgl@sss.pgh.pa.us>
To: exclusion@gmail.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17811: Replacing an underlying view breaks OLD/NEW tuple when accessing it via upper-level view
Date: Tue, 07 Mar 2023 13:43:05 -0500
Message-ID: <4022282.1678214585@sss.pgh.pa.us> (raw)
In-Reply-To: <17811-d31686b78f0dffc9@postgresql.org>
References: <17811-d31686b78f0dffc9@postgresql.org>
PG Bug reporting form <noreply@postgresql.org> writes:
> I've discovered an issue with replacing a view when there is another
> updatable view defined on top of it and the new underlying view has
> more columns than the previous one.
I've looked into this a bit more, and both of these symptoms reduce
to the same thing: after we've plugged the modified view's query
into the upper query, we have an RTE_SUBQUERY RTE whose eref->colnames
list is shorter than the number of columns actually available from the
subquery. This breaks assorted code that is expecting that it can
use list_length(eref->colnames) as a quick guide to the number of
output columns.
We have seen this before (bug #14876, commit d5b760ecb) and at the
time our response was to patch up the code making such an assumption.
But you've just demonstrated at least two other places with the same
issue, so now I'm feeling that we'd be best advised to fix it
centrally in the rewriter code that plugs in the view definition.
The attached fix causes a change in the regression test output for
bug #14876's test case, but AFAICS the change is a strict improvement:
we get something sane, not a NULL, for the added column.
I'm not sure whether to change the code added to expandRTE in
d5b760ecb or leave it be. We could make it into elog(ERROR) now,
perhaps.
regards, tom lane
Attachments:fix-bug-17811.patch (text/x-diff, 2.5K) downloaddiff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.cindex a614e3f5bd..980dc1816f 100644--- a/src/backend/rewrite/rewriteHandler.c+++ b/src/backend/rewrite/rewriteHandler.c@@ -26,6 +26,7 @@
#include "catalog/dependency.h"
#include "catalog/pg_type.h"
#include "commands/trigger.h"
+#include "executor/executor.h"
#include "foreign/fdwapi.h"
#include "miscadmin.h"
#include "nodes/makefuncs.h"
@@ -1730,6 +1731,7 @@ ApplyRetrieveRule(Query *parsetree,
Query *rule_action;
RangeTblEntry *rte;
RowMarkClause *rc;
+ int numCols;
if (list_length(rule->actions) != 1)
elog(ERROR, "expected just one rule action");
@@ -1855,6 +1857,20 @@ ApplyRetrieveRule(Query *parsetree,
rte->tablesample = NULL;
rte->inh = false; /* must not be set for a subquery */
+ /*+ * Since we allow CREATE OR REPLACE VIEW to add columns to a view, the+ * rule_action might emit more columns than we expected when the current+ * query was parsed. Various places expect rte->eref->colnames to be+ * consistent with the non-junk output columns of the subquery, so patch+ * things up if necessary by adding some dummy column names.+ */+ numCols = ExecCleanTargetListLength(rule_action->targetList);+ while (list_length(rte->eref->colnames) < numCols)+ {+ rte->eref->colnames = lappend(rte->eref->colnames,+ makeString(pstrdup("?column?")));+ }+
return parsetree;
}
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.outindex 97bfc3475b..5e5b31f6ed 100644--- a/src/test/regress/expected/alter_table.out+++ b/src/test/regress/expected/alter_table.out@@ -2551,16 +2551,16 @@ View definition:
FROM at_view_1 v1;
explain (verbose, costs off) select * from at_view_2;
- QUERY PLAN -----------------------------------------------------------------+ QUERY PLAN +-------------------------------------------------------------
Seq Scan on public.at_base_table bt
- Output: bt.id, bt.stuff, to_json(ROW(bt.id, bt.stuff, NULL))+ Output: bt.id, bt.stuff, to_json(ROW(bt.id, bt.stuff, 4))
(2 rows)
select * from at_view_2;
- id | stuff | j -----+--------+----------------------------------------- 23 | skidoo | {"id":23,"stuff":"skidoo","more":null}+ id | stuff | j +----+--------+-------------------------------------+ 23 | skidoo | {"id":23,"stuff":"skidoo","more":4}
(1 row)
drop view at_view_2;
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17811: Replacing an underlying view breaks OLD/NEW tuple when accessing it via upper-level view
In-Reply-To: <4022282.1678214585@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox