Thread: BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name.
BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name.
From
PG Bug reporting form
Date:
The following bug has been logged on the website:
Bug reference: 17486
Logged by: Nicolas Lutic
Email address: n.lutic@loxodata.com
PostgreSQL version: 14.3
Operating system: Debian 11
Description:
Hi Team,
I found something weird. Restoring a view fails if this view contains an
attribute without alias name.
Please find below the details to reproduce the problem:
psql -h localhost -c 'CREATE DATABASE demo;'
psql -h localhost -d demo
demo=# CREATE VIEW v_static_select as
WITH static_select as (
select
1 as foo,
'text'
)
select * from static_select;
demo=# \d+ v_static_select
View "public.v_static_select"
Column | Type | Collation | Nullable | Default | Storage |
Description
----------+---------+-----------+----------+---------+----------+-------------
foo | integer | | | | plain |
?column? | text | | | | extended |
View definition:
WITH static_select AS (
SELECT 1 AS foo,
'text'::text
)
SELECT static_select.foo,
static_select."?column?"
FROM static_select;
demo=# select * from v_static_select;
foo | ?column?
-----+----------
1 | text
(1 row)
pg_dump -h localhost -p5432 -U postgres -d demo -Fc -f /tmp/demo.dump
psql -h localhost -p5432 -U postgres -d demo -c 'DROP VIEW
v_static_select ;'
pg_restore -s -h localhost -p5432 -U postgres -d demo /tmp/demo.dump
pg_restore: error: could not execute query: ERROR: column
static_select.?column? does not exist
LINE 7: static_select."?column?"
^
Command was: CREATE VIEW public.v_static_select AS
WITH static_select AS (
SELECT 1 AS foo,
'text'::text
)
SELECT static_select.foo,
static_select."?column?"
FROM static_select;
Regards, Nicolas Lutic
Re: BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name.
From
Daniel Gustafsson
Date:
> On 19 May 2022, at 14:03, PG Bug reporting form <noreply@postgresql.org> wrote: > I found something weird. Restoring a view fails if this view contains an > attribute without alias name. This is not unique to 14, it can be reproduced further down as well. > pg_restore: error: could not execute query: ERROR: column > static_select.?column? does not exist > LINE 7: static_select."?column?" > ^ > Command was: CREATE VIEW public.v_static_select AS > WITH static_select AS ( > SELECT 1 AS foo, > 'text'::text > ) > SELECT static_select.foo, > static_select."?column?" > FROM static_select; Since there is no column name given, FigureColname() will do its best to figure something out and the typecast to TEXT added will lead it to chose the column type as the column name. This will lead the qualified "?column?" target col in the view query to not resolve. When creating the view the ::text explicit cast is added after column names are resolved so this only breaks on restoring the view definition for the expanded SELECT *. -- Daniel Gustafsson https://vmware.com/
Re: BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name.
From
Tom Lane
Date:
Daniel Gustafsson <daniel@yesql.se> writes:
>> On 19 May 2022, at 14:03, PG Bug reporting form <noreply@postgresql.org> wrote:
>> I found something weird. Restoring a view fails if this view contains an
>> attribute without alias name.
> This is not unique to 14, it can be reproduced further down as well.
Yeah, it's an ancient behavior; interesting that no one has complained
before. This code in ruleutils is assuming that FigureColname will
make the same choice at reload as it did before; which is shaky both
because we aren't necessarily printing the identical raw text, and
because there's no guarantee we won't change those rules in future.
Perhaps we should just tweak ruleutils so that the alias is always
printed for non-Var columns, even when it's "?column?". That's kind of
ugly, but if you wanted non-ugly you should have selected a better column
name to start with.
regards, tom lane
Re: BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name.
From
Daniel Gustafsson
Date:
> On 20 May 2022, at 16:34, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Perhaps we should just tweak ruleutils so that the alias is always > printed for non-Var columns, even when it's "?column?". That's kind of > ugly, but if you wanted non-ugly you should have selected a better column > name to start with. That might be the path of least confusion, and as you rightly say, if you don't like the ugliness then there is a very easy way to fix it. -- Daniel Gustafsson https://vmware.com/
Re: BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name.
From
Tom Lane
Date:
Daniel Gustafsson <daniel@yesql.se> writes:
>> On 20 May 2022, at 16:34, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Perhaps we should just tweak ruleutils so that the alias is always
>> printed for non-Var columns, even when it's "?column?". That's kind of
>> ugly, but if you wanted non-ugly you should have selected a better column
>> name to start with.
> That might be the path of least confusion, and as you rightly say, if you don't
> like the ugliness then there is a very easy way to fix it.
Hmm ... it's a very easy code change, but it results in a lot of
changes in the regression tests (and I've only tried the core tests
so far). Given the lack of prior complaints, I wonder if it's going
to be worth this much behavioral churn.
It'd be better if we could do this only when the name is actually
referenced somewhere, but I don't think that's an easy thing to
determine.
regards, tom lane
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 49c4201dde..41275d39b3 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -6042,8 +6042,8 @@ get_target_list(List *targetList, deparse_context *context,
else
{
get_rule_expr((Node *) tle->expr, context, true);
- /* We'll show the AS name unless it's this: */
- attname = "?column?";
+ /* Always show the assigned column name explicitly. */
+ attname = NULL;
}
/*
diff -U3 /home/postgres/pgsql/src/test/regress/expected/create_view.out
/home/postgres/pgsql/src/test/regress/results/create_view.out
--- /home/postgres/pgsql/src/test/regress/expected/create_view.out 2022-05-05 11:23:56.699131282 -0400
+++ /home/postgres/pgsql/src/test/regress/results/create_view.out 2022-05-20 11:14:00.316538617 -0400
@@ -444,7 +444,7 @@
tt1.f2,
tt1.f3
FROM tt1
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM tx1
WHERE tt1.f1 = tx1.x1));
@@ -460,7 +460,7 @@
a1.f2,
a1.f3
FROM tt1 a1
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM tx1
WHERE a1.f1 = tx1.x1));
@@ -476,7 +476,7 @@
tt1.f2,
tt1.f3
FROM tt1
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM tx1 a2
WHERE tt1.f1 = a2.x1));
@@ -492,7 +492,7 @@
tt1.f2,
tt1.f3
FROM temp_view_test.tt1
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM tt1 tt1_1
WHERE tt1.y1 = tt1_1.f1));
@@ -509,7 +509,7 @@
tt1.f2,
tt1.f3
FROM tt1
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM a1
WHERE tt1.f1 = a1.x1));
@@ -525,7 +525,7 @@
a1.f2,
a1.f3
FROM tt1 a1
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM a1 a1_1
WHERE a1.f1 = a1_1.x1));
@@ -541,7 +541,7 @@
tt1.f2,
tt1.f3
FROM tt1
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM a1 a2
WHERE tt1.f1 = a2.x1));
@@ -557,7 +557,7 @@
tt1.f2,
tt1.f3
FROM temp_view_test.tt1
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM tt1 tt1_1
WHERE tt1.y1 = tt1_1.f1));
@@ -574,7 +574,7 @@
a2.f2,
a2.f3
FROM a2
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM a1
WHERE a2.f1 = a1.x1));
@@ -590,7 +590,7 @@
a1.f2,
a1.f3
FROM a2 a1
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM a1 a1_1
WHERE a1.f1 = a1_1.x1));
@@ -606,7 +606,7 @@
a2.f2,
a2.f3
FROM a2
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM a1 a2_1
WHERE a2.f1 = a2_1.x1));
@@ -622,7 +622,7 @@
tt1.f2,
tt1.f3
FROM temp_view_test.tt1
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM a2
WHERE tt1.y1 = a2.f1));
@@ -639,7 +639,7 @@
a2.f2,
a2.f3
FROM a2
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM tt1
WHERE a2.f1 = tt1.x1));
@@ -655,7 +655,7 @@
a1.f2,
a1.f3
FROM a2 a1
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM tt1
WHERE a1.f1 = tt1.x1));
@@ -671,7 +671,7 @@
a2.f2,
a2.f3
FROM a2
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM tt1 a2_1
WHERE a2.f1 = a2_1.x1));
@@ -687,7 +687,7 @@
tt1.f2,
tt1.f3
FROM temp_view_test.tt1
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM a2
WHERE tt1.y1 = a2.f1));
@@ -705,7 +705,7 @@
tx1.f2,
tx1.f3
FROM temp_view_test.tx1
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM tt1
WHERE tx1.f1 = tt1.x1));
@@ -721,7 +721,7 @@
a1.f2,
a1.f3
FROM temp_view_test.tx1 a1
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM tt1
WHERE a1.f1 = tt1.x1));
@@ -737,7 +737,7 @@
tx1.f2,
tx1.f3
FROM temp_view_test.tx1
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM tt1 a2
WHERE tx1.f1 = a2.x1));
@@ -753,7 +753,7 @@
tt1.f2,
tt1.f3
FROM temp_view_test.tt1
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM temp_view_test.tx1
WHERE tt1.y1 = tx1.f1));
@@ -772,7 +772,7 @@
tx1.f2,
tx1.f3
FROM temp_view_test.tx1
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM tt1
WHERE tx1.f1 = tt1.x1));
@@ -788,7 +788,7 @@
a1.f2,
a1.f3
FROM temp_view_test.tx1 a1
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM tt1
WHERE a1.f1 = tt1.x1));
@@ -804,7 +804,7 @@
tx1.f2,
tx1.f3
FROM temp_view_test.tx1
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM tt1 a2
WHERE tx1.f1 = a2.x1));
@@ -820,7 +820,7 @@
tx1.f2,
tx1.f3
FROM tx1
- WHERE (EXISTS ( SELECT 1
+ WHERE (EXISTS ( SELECT 1 AS "?column?"
FROM temp_view_test.tx1 tx1_1
WHERE tx1.y1 = tx1_1.f1));
diff -U3 /home/postgres/pgsql/src/test/regress/expected/create_function_sql.out
/home/postgres/pgsql/src/test/regress/results/create_function_sql.out
--- /home/postgres/pgsql/src/test/regress/expected/create_function_sql.out 2022-03-15 14:54:21.139676523 -0400
+++ /home/postgres/pgsql/src/test/regress/results/create_function_sql.out 2022-05-20 11:14:00.599539001 -0400
@@ -384,14 +384,14 @@
(1 row)
SELECT pg_get_functiondef('functest_S_10'::regproc);
- pg_get_functiondef
--------------------------------------------------------------------------
- CREATE OR REPLACE FUNCTION temp_func_test.functest_s_10(a text, b date)+
- RETURNS boolean +
- LANGUAGE sql +
- BEGIN ATOMIC +
- SELECT ((a = 'abcd'::text) AND (b > '01-01-2001'::date)); +
- END +
+ pg_get_functiondef
+--------------------------------------------------------------------------
+ CREATE OR REPLACE FUNCTION temp_func_test.functest_s_10(a text, b date) +
+ RETURNS boolean +
+ LANGUAGE sql +
+ BEGIN ATOMIC +
+ SELECT ((a = 'abcd'::text) AND (b > '01-01-2001'::date)) AS "?column?";+
+ END +
(1 row)
@@ -402,8 +402,8 @@
RETURNS boolean +
LANGUAGE sql +
BEGIN ATOMIC +
- SELECT 1; +
- SELECT false; +
+ SELECT 1 AS "?column?"; +
+ SELECT false AS "?column?"; +
END +
(1 row)
@@ -425,14 +425,14 @@
(1 row)
SELECT pg_get_functiondef('functest_S_16'::regproc);
- pg_get_functiondef
--------------------------------------------------------------------------------
- CREATE OR REPLACE FUNCTION temp_func_test.functest_s_16(a integer, b integer)+
- RETURNS void +
- LANGUAGE sql +
- BEGIN ATOMIC +
- INSERT INTO functest1 (i) SELECT (functest_s_16.a + functest_s_16.b); +
- END +
+ pg_get_functiondef
+---------------------------------------------------------------------------------------
+ CREATE OR REPLACE FUNCTION temp_func_test.functest_s_16(a integer, b integer) +
+ RETURNS void +
+ LANGUAGE sql +
+ BEGIN ATOMIC +
+ INSERT INTO functest1 (i) SELECT (functest_s_16.a + functest_s_16.b) AS "?column?";+
+ END +
(1 row)
diff -U3 /home/postgres/pgsql/src/test/regress/expected/matview.out
/home/postgres/pgsql/src/test/regress/results/matview.out
--- /home/postgres/pgsql/src/test/regress/expected/matview.out 2021-12-20 12:30:54.358454587 -0500
+++ /home/postgres/pgsql/src/test/regress/results/matview.out 2022-05-20 11:14:02.077541009 -0400
@@ -347,11 +347,11 @@
?column? | integer | | | | plain |
View definition:
SELECT mvtest_vt1.moo,
- 2 * mvtest_vt1.moo
+ 2 * mvtest_vt1.moo AS "?column?"
FROM mvtest_vt1
UNION ALL
SELECT mvtest_vt1.moo,
- 3 * mvtest_vt1.moo
+ 3 * mvtest_vt1.moo AS "?column?"
FROM mvtest_vt1;
CREATE MATERIALIZED VIEW mv_test2 AS SELECT moo, 2*moo FROM mvtest_vt2 UNION ALL SELECT moo, 3*moo FROM mvtest_vt2;
@@ -363,11 +363,11 @@
?column? | integer | | | | plain | |
View definition:
SELECT mvtest_vt2.moo,
- 2 * mvtest_vt2.moo
+ 2 * mvtest_vt2.moo AS "?column?"
FROM mvtest_vt2
UNION ALL
SELECT mvtest_vt2.moo,
- 3 * mvtest_vt2.moo
+ 3 * mvtest_vt2.moo AS "?column?"
FROM mvtest_vt2;
CREATE MATERIALIZED VIEW mv_test3 AS SELECT * FROM mv_test2 WHERE moo = 12345;
diff -U3 /home/postgres/pgsql/src/test/regress/expected/rules.out
/home/postgres/pgsql/src/test/regress/results/rules.out
--- /home/postgres/pgsql/src/test/regress/expected/rules.out 2022-05-19 17:33:41.008350365 -0400
+++ /home/postgres/pgsql/src/test/regress/results/rules.out 2022-05-20 11:14:03.175542500 -0400
@@ -2458,7 +2458,7 @@
array_agg(pg_mcv_list_items.frequency) AS most_common_freqs,
array_agg(pg_mcv_list_items.base_frequency) AS most_common_base_freqs
FROM pg_mcv_list_items(sd.stxdmcv) pg_mcv_list_items(index, "values", nulls, frequency, base_frequency)) m
ON((sd.stxdmcv IS NOT NULL)))
- WHERE ((NOT (EXISTS ( SELECT 1
+ WHERE ((NOT (EXISTS ( SELECT 1 AS "?column?"
FROM (unnest(s.stxkeys) k(k)
JOIN pg_attribute a ON (((a.attrelid = s.stxrelid) AND (a.attnum = k.k))))
WHERE (NOT has_column_privilege(c.oid, a.attnum, 'select'::text))))) AND ((c.relrowsecurity = false) OR (NOT
row_security_active(c.oid))));
diff -U3 /home/postgres/pgsql/src/test/regress/expected/with.out /home/postgres/pgsql/src/test/regress/results/with.out
--- /home/postgres/pgsql/src/test/regress/expected/with.out 2022-05-19 17:33:41.008350365 -0400
+++ /home/postgres/pgsql/src/test/regress/results/with.out 2022-05-20 11:14:05.002544982 -0400
@@ -442,7 +442,7 @@
WITH RECURSIVE t(n) AS (
VALUES (1)
UNION ALL
- SELECT t_1.n + 1
+ SELECT t_1.n + 1 AS "?column?"
FROM t t_1
WHERE t_1.n < 100
)
Re: BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name.
From
Tom Lane
Date:
I wrote:
> Hmm ... it's a very easy code change, but it results in a lot of
> changes in the regression tests (and I've only tried the core tests
> so far). Given the lack of prior complaints, I wonder if it's going
> to be worth this much behavioral churn.
> It'd be better if we could do this only when the name is actually
> referenced somewhere, but I don't think that's an easy thing to
> determine.
I thought of a compromise position that's not hard to implement:
change the behavior only if the SELECT output column name is *possibly*
visible elsewhere, which it is not in (for example) an EXISTS subquery.
This is easy to keep track of while descending the parse tree.
The attached quick-hack draft results in only one place changing in
the regression tests, and that's a place where a view's visible
column name is already "?column?", so whoever wrote that test case
didn't give a fig for prettiness anyway. This seems like it might be
an acceptable amount of behavioral churn.
Thoughts?
regards, tom lane
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 49c4201dde..d8b54e1361 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -395,12 +395,12 @@ static void make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
static void make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
int prettyFlags, int wrapColumn);
static void get_query_def(Query *query, StringInfo buf, List *parentnamespace,
- TupleDesc resultDesc,
+ TupleDesc resultDesc, bool colNamesVisible,
int prettyFlags, int wrapColumn, int startIndent);
static void get_values_def(List *values_lists, deparse_context *context);
static void get_with_clause(Query *query, deparse_context *context);
static void get_select_query_def(Query *query, deparse_context *context,
- TupleDesc resultDesc);
+ TupleDesc resultDesc, bool colNamesVisible);
static void get_insert_query_def(Query *query, deparse_context *context);
static void get_update_query_def(Query *query, deparse_context *context);
static void get_update_query_targetlist_def(Query *query, List *targetList,
@@ -409,12 +409,12 @@ static void get_update_query_targetlist_def(Query *query, List *targetList,
static void get_delete_query_def(Query *query, deparse_context *context);
static void get_utility_query_def(Query *query, deparse_context *context);
static void get_basic_select_query(Query *query, deparse_context *context,
- TupleDesc resultDesc);
+ TupleDesc resultDesc, bool colNamesVisible);
static void get_target_list(List *targetList, deparse_context *context,
- TupleDesc resultDesc);
+ TupleDesc resultDesc, bool colNamesVisible);
static void get_setop_query(Node *setOp, Query *query,
deparse_context *context,
- TupleDesc resultDesc);
+ TupleDesc resultDesc, bool colNamesVisible);
static Node *get_rule_sortgroupclause(Index ref, List *tlist,
bool force_colno,
deparse_context *context);
@@ -1544,7 +1544,8 @@ pg_get_querydef(Query *query, bool pretty)
initStringInfo(&buf);
- get_query_def(query, &buf, NIL, NULL, prettyFlags, WRAP_COLUMN_DEFAULT, 0);
+ get_query_def(query, &buf, NIL, NULL, true,
+ prettyFlags, WRAP_COLUMN_DEFAULT, 0);
return buf.data;
}
@@ -3548,7 +3549,7 @@ print_function_sqlbody(StringInfo buf, HeapTuple proctup)
/* It seems advisable to get at least AccessShareLock on rels */
AcquireRewriteLocks(query, false, false);
- get_query_def(query, buf, list_make1(&dpns), NULL,
+ get_query_def(query, buf, list_make1(&dpns), NULL, false,
PRETTYFLAG_INDENT, WRAP_COLUMN_DEFAULT, 1);
appendStringInfoChar(buf, ';');
appendStringInfoChar(buf, '\n');
@@ -3562,7 +3563,7 @@ print_function_sqlbody(StringInfo buf, HeapTuple proctup)
/* It seems advisable to get at least AccessShareLock on rels */
AcquireRewriteLocks(query, false, false);
- get_query_def(query, buf, list_make1(&dpns), NULL,
+ get_query_def(query, buf, list_make1(&dpns), NULL, false,
0, WRAP_COLUMN_DEFAULT, 0);
}
}
@@ -5299,7 +5300,7 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
foreach(action, actions)
{
query = (Query *) lfirst(action);
- get_query_def(query, buf, NIL, viewResultDesc,
+ get_query_def(query, buf, NIL, viewResultDesc, true,
prettyFlags, WRAP_COLUMN_DEFAULT, 0);
if (prettyFlags)
appendStringInfoString(buf, ";\n");
@@ -5313,7 +5314,7 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
Query *query;
query = (Query *) linitial(actions);
- get_query_def(query, buf, NIL, viewResultDesc,
+ get_query_def(query, buf, NIL, viewResultDesc, true,
prettyFlags, WRAP_COLUMN_DEFAULT, 0);
appendStringInfoChar(buf, ';');
}
@@ -5387,7 +5388,7 @@ make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
ev_relation = table_open(ev_class, AccessShareLock);
- get_query_def(query, buf, NIL, RelationGetDescr(ev_relation),
+ get_query_def(query, buf, NIL, RelationGetDescr(ev_relation), true,
prettyFlags, wrapColumn, 0);
appendStringInfoChar(buf, ';');
@@ -5404,7 +5405,7 @@ make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
*/
static void
get_query_def(Query *query, StringInfo buf, List *parentnamespace,
- TupleDesc resultDesc,
+ TupleDesc resultDesc, bool colNamesVisible,
int prettyFlags, int wrapColumn, int startIndent)
{
deparse_context context;
@@ -5442,7 +5443,7 @@ get_query_def(Query *query, StringInfo buf, List *parentnamespace,
switch (query->commandType)
{
case CMD_SELECT:
- get_select_query_def(query, &context, resultDesc);
+ get_select_query_def(query, &context, resultDesc, colNamesVisible);
break;
case CMD_UPDATE:
@@ -5578,6 +5579,7 @@ get_with_clause(Query *query, deparse_context *context)
if (PRETTY_INDENT(context))
appendContextKeyword(context, "", 0, 0, 0);
get_query_def((Query *) cte->ctequery, buf, context->namespaces, NULL,
+ true,
context->prettyFlags, context->wrapColumn,
context->indentLevel);
if (PRETTY_INDENT(context))
@@ -5659,7 +5661,7 @@ get_with_clause(Query *query, deparse_context *context)
*/
static void
get_select_query_def(Query *query, deparse_context *context,
- TupleDesc resultDesc)
+ TupleDesc resultDesc, bool colNamesVisible)
{
StringInfo buf = context->buf;
List *save_windowclause;
@@ -5683,13 +5685,14 @@ get_select_query_def(Query *query, deparse_context *context,
*/
if (query->setOperations)
{
- get_setop_query(query->setOperations, query, context, resultDesc);
+ get_setop_query(query->setOperations, query, context, resultDesc,
+ colNamesVisible);
/* ORDER BY clauses must be simple in this case */
force_colno = true;
}
else
{
- get_basic_select_query(query, context, resultDesc);
+ get_basic_select_query(query, context, resultDesc, colNamesVisible);
force_colno = false;
}
@@ -5859,7 +5862,7 @@ get_simple_values_rte(Query *query, TupleDesc resultDesc)
static void
get_basic_select_query(Query *query, deparse_context *context,
- TupleDesc resultDesc)
+ TupleDesc resultDesc, bool colNamesVisible)
{
StringInfo buf = context->buf;
RangeTblEntry *values_rte;
@@ -5915,7 +5918,7 @@ get_basic_select_query(Query *query, deparse_context *context,
}
/* Then we tell what to select (the targetlist) */
- get_target_list(query->targetList, context, resultDesc);
+ get_target_list(query->targetList, context, resultDesc, colNamesVisible);
/* Add the FROM clause if needed */
get_from_clause(query, " FROM ", context);
@@ -5987,11 +5990,13 @@ get_basic_select_query(Query *query, deparse_context *context,
* get_target_list - Parse back a SELECT target list
*
* This is also used for RETURNING lists in INSERT/UPDATE/DELETE.
+ *
+ * XXX document arguments better
* ----------
*/
static void
get_target_list(List *targetList, deparse_context *context,
- TupleDesc resultDesc)
+ TupleDesc resultDesc, bool colNamesVisible)
{
StringInfo buf = context->buf;
StringInfoData targetbuf;
@@ -6042,8 +6047,13 @@ get_target_list(List *targetList, deparse_context *context,
else
{
get_rule_expr((Node *) tle->expr, context, true);
- /* We'll show the AS name unless it's this: */
- attname = "?column?";
+
+ /*
+ * When colNamesVisible is true, we should always show the
+ * assigned column name explicitly. Otherwise, show it only if
+ * it's not FigureColname's fallback.
+ */
+ attname = colNamesVisible ? NULL : "?column?";
}
/*
@@ -6122,7 +6132,7 @@ get_target_list(List *targetList, deparse_context *context,
static void
get_setop_query(Node *setOp, Query *query, deparse_context *context,
- TupleDesc resultDesc)
+ TupleDesc resultDesc, bool colNamesVisible)
{
StringInfo buf = context->buf;
bool need_paren;
@@ -6148,6 +6158,7 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
if (need_paren)
appendStringInfoChar(buf, '(');
get_query_def(subquery, buf, context->namespaces, resultDesc,
+ colNamesVisible,
context->prettyFlags, context->wrapColumn,
context->indentLevel);
if (need_paren)
@@ -6190,7 +6201,7 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
else
subindent = 0;
- get_setop_query(op->larg, query, context, resultDesc);
+ get_setop_query(op->larg, query, context, resultDesc, colNamesVisible);
if (need_paren)
appendContextKeyword(context, ") ", -subindent, 0, 0);
@@ -6234,7 +6245,7 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
subindent = 0;
appendContextKeyword(context, "", subindent, 0, 0);
- get_setop_query(op->rarg, query, context, resultDesc);
+ get_setop_query(op->rarg, query, context, resultDesc, false);
if (PRETTY_INDENT(context))
context->indentLevel -= subindent;
@@ -6680,6 +6691,7 @@ get_insert_query_def(Query *query, deparse_context *context)
{
/* Add the SELECT */
get_query_def(select_rte->subquery, buf, context->namespaces, NULL,
+ false,
context->prettyFlags, context->wrapColumn,
context->indentLevel);
}
@@ -6773,7 +6785,7 @@ get_insert_query_def(Query *query, deparse_context *context)
{
appendContextKeyword(context, " RETURNING",
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
- get_target_list(query->returningList, context, NULL);
+ get_target_list(query->returningList, context, NULL, true);
}
}
@@ -6828,7 +6840,7 @@ get_update_query_def(Query *query, deparse_context *context)
{
appendContextKeyword(context, " RETURNING",
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
- get_target_list(query->returningList, context, NULL);
+ get_target_list(query->returningList, context, NULL, true);
}
}
@@ -7031,7 +7043,7 @@ get_delete_query_def(Query *query, deparse_context *context)
{
appendContextKeyword(context, " RETURNING",
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
- get_target_list(query->returningList, context, NULL);
+ get_target_list(query->returningList, context, NULL, true);
}
}
@@ -11039,7 +11051,7 @@ get_sublink_expr(SubLink *sublink, deparse_context *context)
if (need_paren)
appendStringInfoChar(buf, '(');
- get_query_def(query, buf, context->namespaces, NULL,
+ get_query_def(query, buf, context->namespaces, NULL, false,
context->prettyFlags, context->wrapColumn,
context->indentLevel);
@@ -11548,6 +11560,7 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
/* Subquery RTE */
appendStringInfoChar(buf, '(');
get_query_def(rte->subquery, buf, context->namespaces, NULL,
+ true,
context->prettyFlags, context->wrapColumn,
context->indentLevel);
appendStringInfoChar(buf, ')');
diff --git a/src/test/regress/expected/matview.out b/src/test/regress/expected/matview.out
index 313c72a268..c109d97635 100644
--- a/src/test/regress/expected/matview.out
+++ b/src/test/regress/expected/matview.out
@@ -347,7 +347,7 @@ CREATE VIEW mvtest_vt2 AS SELECT moo, 2*moo FROM mvtest_vt1 UNION ALL SELECT moo
?column? | integer | | | | plain |
View definition:
SELECT mvtest_vt1.moo,
- 2 * mvtest_vt1.moo
+ 2 * mvtest_vt1.moo AS "?column?"
FROM mvtest_vt1
UNION ALL
SELECT mvtest_vt1.moo,
@@ -363,7 +363,7 @@ CREATE MATERIALIZED VIEW mv_test2 AS SELECT moo, 2*moo FROM mvtest_vt2 UNION ALL
?column? | integer | | | | plain | |
View definition:
SELECT mvtest_vt2.moo,
- 2 * mvtest_vt2.moo
+ 2 * mvtest_vt2.moo AS "?column?"
FROM mvtest_vt2
UNION ALL
SELECT mvtest_vt2.moo,
Re: BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name.
From
Daniel Gustafsson
Date:
> On 20 May 2022, at 19:05, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > I wrote: >> Hmm ... it's a very easy code change, but it results in a lot of >> changes in the regression tests (and I've only tried the core tests >> so far). Given the lack of prior complaints, I wonder if it's going >> to be worth this much behavioral churn. > >> It'd be better if we could do this only when the name is actually >> referenced somewhere, but I don't think that's an easy thing to >> determine. > > I thought of a compromise position that's not hard to implement: > change the behavior only if the SELECT output column name is *possibly* > visible elsewhere, which it is not in (for example) an EXISTS subquery. Nice one! I think that's even better than the previous version actually. Skimming the patch it seems like a reasonable approach. -- Daniel Gustafsson https://vmware.com/
Re: BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name.
From
Tom Lane
Date:
Daniel Gustafsson <daniel@yesql.se> writes:
> On 20 May 2022, at 19:05, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I thought of a compromise position that's not hard to implement:
>> change the behavior only if the SELECT output column name is *possibly*
>> visible elsewhere, which it is not in (for example) an EXISTS subquery.
> Nice one! I think that's even better than the previous version actually.
> Skimming the patch it seems like a reasonable approach.
Pushed, thanks for looking.
regards, tom lane