Re: BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name. - Mailing list pgsql-bugs
From | Tom Lane |
---|---|
Subject | Re: BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name. |
Date | |
Msg-id | 428377.1653066358@sss.pgh.pa.us Whole thread Raw |
In response to | Re: BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name. (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name.
|
List | pgsql-bugs |
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,
pgsql-bugs by date: