Skip to content

Commit 65da6dd

Browse files
tglsfdctglsfdc
committed
Further fix dumping of views that contain just VALUES(...).
It turns out that commit e9f1c01 missed a case: we must print a VALUES clause in long format if get_query_def is given a resultDesc that would require the query's output column name(s) to be different from what the bare VALUES clause would produce. This applies in case an ALTER ... RENAME COLUMN has been done to a view that formerly could be printed in simple format, as shown in the added regression test case. It also explains bug #16119 from Dmitry Telpt, because it turns out that (unlike CREATE VIEW) CREATE MATERIALIZED VIEW fails to apply any column aliases it's given to the stored ON SELECT rule. So to get them to be printed, we have to account for the resultDesc renaming. It might be worth changing the matview code so that it creates the ON SELECT rule with the correct aliases; but we'd still need these messy checks in get_simple_values_rte to handle the case of a subsequent column rename, so any such change would be just neatnik-ism not a bug fix. Like the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/16119-e64823f30a45a754@postgresql.org
1 parent 56c0699 commit 65da6dd

File tree

3 files changed

+38
-10
lines changed

3 files changed

+38
-10
lines changed

‎src/backend/utils/adt/ruleutils.c

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4628,19 +4628,20 @@ get_select_query_def(Query *query, deparse_context *context,
46284628
}
46294629

46304630
/*
4631-
* Detect whether query looks like SELECT ... FROM VALUES();
4632-
* if so, return the VALUES RTE. Otherwise return NULL.
4631+
* Detect whether query looks like SELECT ... FROM VALUES(),
4632+
* with no need to rename the output columns of the VALUES RTE.
4633+
* If so, return the VALUES RTE. Otherwise return NULL.
46334634
*/
46344635
static RangeTblEntry *
4635-
get_simple_values_rte(Query *query)
4636+
get_simple_values_rte(Query *query, TupleDesc resultDesc)
46364637
{
46374638
RangeTblEntry *result = NULL;
46384639
ListCell *lc;
46394640

46404641
/*
4641-
* We want to return TRUE even if the Query also contains OLD or NEW rule
4642-
* RTEs. So the idea is to scan the rtable and see if there is only one
4643-
* inFromCl RTE that is a VALUES RTE.
4642+
* We want to detect a match even if the Query also contains OLD or NEW
4643+
* rule RTEs. So the idea is to scan the rtable and see if there is only
4644+
* one inFromCl RTE that is a VALUES RTE.
46444645
*/
46454646
foreach(lc, query->rtable)
46464647
{
@@ -4663,23 +4664,36 @@ get_simple_values_rte(Query *query)
46634664
* parser/analyze.c will never generate a "bare" VALUES RTE --- they only
46644665
* appear inside auto-generated sub-queries with very restricted
46654666
* structure. However, DefineView might have modified the tlist by
4666-
* injecting new column aliases; so compare tlist resnames against the
4667-
* RTE's names to detect that.
4667+
* injecting new column aliases, or we might have some other column
4668+
* aliases forced by a resultDesc. We can only simplify if the RTE's
4669+
* column names match the names that get_target_list() would select.
46684670
*/
46694671
if (result)
46704672
{
46714673
ListCell *lcn;
4674+
int colno;
46724675

46734676
if (list_length(query->targetList) != list_length(result->eref->colnames))
46744677
return NULL; /* this probably cannot happen */
4678+
colno = 0;
46754679
forboth(lc, query->targetList, lcn, result->eref->colnames)
46764680
{
46774681
TargetEntry *tle = (TargetEntry *) lfirst(lc);
46784682
char *cname = strVal(lfirst(lcn));
4683+
char *colname;
46794684

46804685
if (tle->resjunk)
46814686
return NULL; /* this probably cannot happen */
4682-
if (tle->resname == NULL || strcmp(tle->resname, cname) != 0)
4687+
4688+
/* compute name that get_target_list would use for column */
4689+
colno++;
4690+
if (resultDesc && colno <= resultDesc->natts)
4691+
colname = NameStr(TupleDescAttr(resultDesc, colno - 1)->attname);
4692+
else
4693+
colname = tle->resname;
4694+
4695+
/* does it match the VALUES RTE? */
4696+
if (colname == NULL || strcmp(colname, cname) != 0)
46834697
return NULL; /* column name has been changed */
46844698
}
46854699
}
@@ -4707,7 +4721,7 @@ get_basic_select_query(Query *query, deparse_context *context,
47074721
* VALUES part. This reverses what transformValuesClause() did at parse
47084722
* time.
47094723
*/
4710-
values_rte = get_simple_values_rte(query);
4724+
values_rte = get_simple_values_rte(query, resultDesc);
47114725
if (values_rte)
47124726
{
47134727
get_values_def(values_rte->values_lists, context);

‎src/test/regress/expected/rules.out

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2674,6 +2674,18 @@ create view rule_v1 as values(1,2);
26742674
View definition:
26752675
VALUES (1,2);
26762676

2677+
alter table rule_v1 rename column column2 to q2;
2678+
\d+ rule_v1
2679+
View "public.rule_v1"
2680+
Column | Type | Modifiers | Storage | Description
2681+
---------+---------+-----------+---------+-------------
2682+
column1 | integer | | plain |
2683+
q2 | integer | | plain |
2684+
View definition:
2685+
SELECT "*VALUES*".column1,
2686+
"*VALUES*".column2 AS q2
2687+
FROM (VALUES (1,2)) "*VALUES*";
2688+
26772689
drop view rule_v1;
26782690
create view rule_v1(x) as values(1,2);
26792691
\d+ rule_v1

‎src/test/regress/sql/rules.sql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,6 +1013,8 @@ DROP TABLE rule_t1;
10131013
--
10141014
create view rule_v1 as values(1,2);
10151015
\d+ rule_v1
1016+
alter table rule_v1 rename column column2 to q2;
1017+
\d+ rule_v1
10161018
drop view rule_v1;
10171019
create view rule_v1(x) as values(1,2);
10181020
\d+ rule_v1

0 commit comments

Comments
 (0)