Kevin Grittner <kgri...@ymail.com> wrote:
> Michael Paquier <michael.paqu...@gmail.com> wrote:
>
>> I am not sure that adding a boolean flag introducing a concept
>> related to matview inside checkRuleResultList is the best
>> approach to solve that. checkRuleResultList is something related
>> only to rules, and has nothing related to matviews in it yet.
>
> Well, I was tempted to keep that concept in DefineQueryRewrite()
> where the call is made, by calling the new bool something like
> requireColumnNameMatch and not having checkRuleResultList()
> continue to use isSelect for that purpose at all.
> DefineQueryRewrite() already references RELKIND_RELATION once,
> RELKIND_MATVIEW twice, and RELKIND_VIEW three times, so it would
> hardly be introducing a new concept there.
Upon reflection, that seemed to be cleaner. Pushed fix that way.
Not much of a change from the previously posted patch, but attached
here in case anyone wants to argue against this approach.
Thanks for the report!
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
*** a/src/backend/rewrite/rewriteDefine.c
--- b/src/backend/rewrite/rewriteDefine.c
***************
*** 44,50 ****
static void checkRuleResultList(List *targetList, TupleDesc resultDesc,
! bool isSelect);
static bool setRuleCheckAsUser_walker(Node *node, Oid *context);
static void setRuleCheckAsUser_Query(Query *qry, Oid userid);
--- 44,50 ----
static void checkRuleResultList(List *targetList, TupleDesc resultDesc,
! bool isSelect, bool requireColumnNameMatch);
static bool setRuleCheckAsUser_walker(Node *node, Oid *context);
static void setRuleCheckAsUser_Query(Query *qry, Oid userid);
***************
*** 355,361 **** DefineQueryRewrite(char *rulename,
*/
checkRuleResultList(query->targetList,
RelationGetDescr(event_relation),
! true);
/*
* ... there must not be another ON SELECT rule already ...
--- 355,363 ----
*/
checkRuleResultList(query->targetList,
RelationGetDescr(event_relation),
! true,
! event_relation->rd_rel->relkind !=
! RELKIND_MATVIEW);
/*
* ... there must not be another ON SELECT rule already ...
***************
*** 484,490 **** DefineQueryRewrite(char *rulename,
errmsg("RETURNING lists are not supported in non-INSTEAD rules")));
checkRuleResultList(query->returningList,
RelationGetDescr(event_relation),
! false);
}
}
--- 486,492 ----
errmsg("RETURNING lists are not supported in non-INSTEAD rules")));
checkRuleResultList(query->returningList,
RelationGetDescr(event_relation),
! false, false);
}
}
***************
*** 613,627 **** DefineQueryRewrite(char *rulename,
* Verify that targetList produces output compatible with a tupledesc
*
* The targetList might be either a SELECT targetlist, or a RETURNING list;
! * isSelect tells which. (This is mostly used for choosing error messages,
! * but also we don't enforce column name matching for RETURNING.)
*/
static void
! checkRuleResultList(List *targetList, TupleDesc resultDesc, bool isSelect)
{
ListCell *tllist;
int i;
i = 0;
foreach(tllist, targetList)
{
--- 615,634 ----
* Verify that targetList produces output compatible with a tupledesc
*
* The targetList might be either a SELECT targetlist, or a RETURNING list;
! * isSelect tells which. This is used for choosing error messages.
! *
! * A SELECT targetlist may optionally require that column names match.
*/
static void
! checkRuleResultList(List *targetList, TupleDesc resultDesc, bool isSelect,
! bool requireColumnNameMatch)
{
ListCell *tllist;
int i;
+ /* Only a SELECT may require a column name match. */
+ Assert(isSelect || !requireColumnNameMatch);
+
i = 0;
foreach(tllist, targetList)
{
***************
*** 657,663 **** checkRuleResultList(List *targetList, TupleDesc resultDesc, bool isSelect)
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot convert relation containing dropped columns to view")));
! if (isSelect && strcmp(tle->resname, attname) != 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
errmsg("SELECT rule's target entry %d has different column name from \"%s\"", i, attname)));
--- 664,670 ----
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot convert relation containing dropped columns to view")));
! if (requireColumnNameMatch && strcmp(tle->resname, attname) != 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
errmsg("SELECT rule's target entry %d has different column name from \"%s\"", i, attname)));
*** a/src/test/regress/expected/matview.out
--- b/src/test/regress/expected/matview.out
***************
*** 450,452 **** SELECT * FROM boxmv ORDER BY id;
--- 450,475 ----
DROP TABLE boxes CASCADE;
NOTICE: drop cascades to materialized view boxmv
+ -- make sure that column names are handled correctly
+ CREATE TABLE v (i int, j int);
+ CREATE MATERIALIZED VIEW mv_v (ii) AS SELECT i, j AS jj FROM v;
+ ALTER TABLE v RENAME COLUMN i TO x;
+ INSERT INTO v values (1, 2);
+ CREATE UNIQUE INDEX mv_v_ii ON mv_v (ii);
+ REFRESH MATERIALIZED VIEW mv_v;
+ UPDATE v SET j = 3 WHERE x = 1;
+ REFRESH MATERIALIZED VIEW CONCURRENTLY mv_v;
+ SELECT * FROM v;
+ x | j
+ ---+---
+ 1 | 3
+ (1 row)
+
+ SELECT * FROM mv_v;
+ ii | jj
+ ----+----
+ 1 | 3
+ (1 row)
+
+ DROP TABLE v CASCADE;
+ NOTICE: drop cascades to materialized view mv_v
*** a/src/test/regress/sql/matview.sql
--- b/src/test/regress/sql/matview.sql
***************
*** 173,175 **** UPDATE boxes SET b = '(2,2),(1,1)' WHERE id = 2;
--- 173,188 ----
REFRESH MATERIALIZED VIEW CONCURRENTLY boxmv;
SELECT * FROM boxmv ORDER BY id;
DROP TABLE boxes CASCADE;
+
+ -- make sure that column names are handled correctly
+ CREATE TABLE v (i int, j int);
+ CREATE MATERIALIZED VIEW mv_v (ii) AS SELECT i, j AS jj FROM v;
+ ALTER TABLE v RENAME COLUMN i TO x;
+ INSERT INTO v values (1, 2);
+ CREATE UNIQUE INDEX mv_v_ii ON mv_v (ii);
+ REFRESH MATERIALIZED VIEW mv_v;
+ UPDATE v SET j = 3 WHERE x = 1;
+ REFRESH MATERIALIZED VIEW CONCURRENTLY mv_v;
+ SELECT * FROM v;
+ SELECT * FROM mv_v;
+ DROP TABLE v CASCADE;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
