Skip to content

Commit bb2c046

Browse files
tglsfdctglsfdc
committed
Show 'AS "?column?"' explicitly when it's important.
ruleutils.c was coded to suppress the AS label for a SELECT output expression if the column name is "?column?", which is the parser's fallback if it can't think of something better. This is fine, and avoids ugly clutter, so long as (1) nothing further up in the parse tree relies on that column name or (2) the same fallback would be assigned when the rule or view definition is reloaded. Unfortunately (2) is far from certain, both because ruleutils.c might print the expression in a different form from how it was originally written and because FigureColname's rules might change in future releases. So we shouldn't rely on that. Detecting exactly whether there is any outer-level use of a SELECT column name would be rather expensive. This patch takes the simpler approach of just passing down a flag indicating whether there *could* be any outer use; for example, the output column names of a SubLink are not referenceable, and we also do not care about the names exposed by the right-hand side of a setop. This is sufficient to suppress unwanted clutter in all but one case in the regression tests. That seems like reasonable evidence that it won't be too much in users' faces, while still fixing the cases we need to fix. Per bug #17486 from Nicolas Lutic. This issue is ancient, so back-patch to all supported branches. Discussion: https://postgr.es/m/17486-1ad6fd786728b8af@postgresql.org
1 parent 5f6baee commit bb2c046

File tree

2 files changed

+66
-38
lines changed

2 files changed

+66
-38
lines changed

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

Lines changed: 64 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -372,26 +372,29 @@ static void make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
372372
static void make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
373373
int prettyFlags, int wrapColumn);
374374
static void get_query_def(Query *query, StringInfo buf, List *parentnamespace,
375-
TupleDesc resultDesc,
375+
TupleDesc resultDesc, bool colNamesVisible,
376376
int prettyFlags, int wrapColumn, int startIndent);
377377
static void get_values_def(List *values_lists, deparse_context *context);
378378
static void get_with_clause(Query *query, deparse_context *context);
379379
static void get_select_query_def(Query *query, deparse_context *context,
380-
TupleDesc resultDesc);
381-
static void get_insert_query_def(Query *query, deparse_context *context);
382-
static void get_update_query_def(Query *query, deparse_context *context);
380+
TupleDesc resultDesc, bool colNamesVisible);
381+
static void get_insert_query_def(Query *query, deparse_context *context,
382+
bool colNamesVisible);
383+
static void get_update_query_def(Query *query, deparse_context *context,
384+
bool colNamesVisible);
383385
static void get_update_query_targetlist_def(Query *query, List *targetList,
384386
deparse_context *context,
385387
RangeTblEntry *rte);
386-
static void get_delete_query_def(Query *query, deparse_context *context);
388+
static void get_delete_query_def(Query *query, deparse_context *context,
389+
bool colNamesVisible);
387390
static void get_utility_query_def(Query *query, deparse_context *context);
388391
static void get_basic_select_query(Query *query, deparse_context *context,
389-
TupleDesc resultDesc);
392+
TupleDesc resultDesc, bool colNamesVisible);
390393
static void get_target_list(List *targetList, deparse_context *context,
391-
TupleDesc resultDesc);
394+
TupleDesc resultDesc, bool colNamesVisible);
392395
static void get_setop_query(Node *setOp, Query *query,
393396
deparse_context *context,
394-
TupleDesc resultDesc);
397+
TupleDesc resultDesc, bool colNamesVisible);
395398
static Node *get_rule_sortgroupclause(Index ref, List *tlist,
396399
bool force_colno,
397400
deparse_context *context);
@@ -4960,7 +4963,7 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
49604963
foreach(action, actions)
49614964
{
49624965
query = (Query *) lfirst(action);
4963-
get_query_def(query, buf, NIL, viewResultDesc,
4966+
get_query_def(query, buf, NIL, viewResultDesc, true,
49644967
prettyFlags, WRAP_COLUMN_DEFAULT, 0);
49654968
if (prettyFlags)
49664969
appendStringInfoString(buf, ";\n");
@@ -4978,7 +4981,7 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
49784981
Query *query;
49794982

49804983
query = (Query *) linitial(actions);
4981-
get_query_def(query, buf, NIL, viewResultDesc,
4984+
get_query_def(query, buf, NIL, viewResultDesc, true,
49824985
prettyFlags, WRAP_COLUMN_DEFAULT, 0);
49834986
appendStringInfoChar(buf, ';');
49844987
}
@@ -5052,7 +5055,7 @@ make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
50525055

50535056
ev_relation = table_open(ev_class, AccessShareLock);
50545057

5055-
get_query_def(query, buf, NIL, RelationGetDescr(ev_relation),
5058+
get_query_def(query, buf, NIL, RelationGetDescr(ev_relation), true,
50565059
prettyFlags, wrapColumn, 0);
50575060
appendStringInfoChar(buf, ';');
50585061

@@ -5063,13 +5066,23 @@ make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
50635066
/* ----------
50645067
* get_query_def - Parse back one query parsetree
50655068
*
5066-
* If resultDesc is not NULL, then it is the output tuple descriptor for
5067-
* the view represented by a SELECT query.
5069+
* query: parsetree to be displayed
5070+
* buf: output text is appended to buf
5071+
* parentnamespace: list (initially empty) of outer-level deparse_namespace's
5072+
* resultDesc: if not NULL, the output tuple descriptor for the view
5073+
* represented by a SELECT query. We use the column names from it
5074+
* to label SELECT output columns, in preference to names in the query
5075+
* colNamesVisible: true if the surrounding context cares about the output
5076+
* column names at all (as, for example, an EXISTS() context does not);
5077+
* when false, we can suppress dummy column labels such as "?column?"
5078+
* prettyFlags: bitmask of PRETTYFLAG_XXX options
5079+
* wrapColumn: maximum line length, or -1 to disable wrapping
5080+
* startIndent: initial indentation amount
50685081
* ----------
50695082
*/
50705083
static void
50715084
get_query_def(Query *query, StringInfo buf, List *parentnamespace,
5072-
TupleDesc resultDesc,
5085+
TupleDesc resultDesc, bool colNamesVisible,
50735086
int prettyFlags, int wrapColumn, int startIndent)
50745087
{
50755088
deparse_context context;
@@ -5106,19 +5119,19 @@ get_query_def(Query *query, StringInfo buf, List *parentnamespace,
51065119
switch (query->commandType)
51075120
{
51085121
case CMD_SELECT:
5109-
get_select_query_def(query, &context, resultDesc);
5122+
get_select_query_def(query, &context, resultDesc, colNamesVisible);
51105123
break;
51115124

51125125
case CMD_UPDATE:
5113-
get_update_query_def(query, &context);
5126+
get_update_query_def(query, &context, colNamesVisible);
51145127
break;
51155128

51165129
case CMD_INSERT:
5117-
get_insert_query_def(query, &context);
5130+
get_insert_query_def(query, &context, colNamesVisible);
51185131
break;
51195132

51205133
case CMD_DELETE:
5121-
get_delete_query_def(query, &context);
5134+
get_delete_query_def(query, &context, colNamesVisible);
51225135
break;
51235136

51245137
case CMD_NOTHING:
@@ -5242,6 +5255,7 @@ get_with_clause(Query *query, deparse_context *context)
52425255
if (PRETTY_INDENT(context))
52435256
appendContextKeyword(context, "", 0, 0, 0);
52445257
get_query_def((Query *) cte->ctequery, buf, context->namespaces, NULL,
5258+
true,
52455259
context->prettyFlags, context->wrapColumn,
52465260
context->indentLevel);
52475261
if (PRETTY_INDENT(context))
@@ -5265,7 +5279,7 @@ get_with_clause(Query *query, deparse_context *context)
52655279
*/
52665280
static void
52675281
get_select_query_def(Query *query, deparse_context *context,
5268-
TupleDesc resultDesc)
5282+
TupleDesc resultDesc, bool colNamesVisible)
52695283
{
52705284
StringInfo buf = context->buf;
52715285
List *save_windowclause;
@@ -5289,13 +5303,14 @@ get_select_query_def(Query *query, deparse_context *context,
52895303
*/
52905304
if (query->setOperations)
52915305
{
5292-
get_setop_query(query->setOperations, query, context, resultDesc);
5306+
get_setop_query(query->setOperations, query, context, resultDesc,
5307+
colNamesVisible);
52935308
/* ORDER BY clauses must be simple in this case */
52945309
force_colno = true;
52955310
}
52965311
else
52975312
{
5298-
get_basic_select_query(query, context, resultDesc);
5313+
get_basic_select_query(query, context, resultDesc, colNamesVisible);
52995314
force_colno = false;
53005315
}
53015316

@@ -5452,7 +5467,7 @@ get_simple_values_rte(Query *query, TupleDesc resultDesc)
54525467

54535468
static void
54545469
get_basic_select_query(Query *query, deparse_context *context,
5455-
TupleDesc resultDesc)
5470+
TupleDesc resultDesc, bool colNamesVisible)
54565471
{
54575472
StringInfo buf = context->buf;
54585473
RangeTblEntry *values_rte;
@@ -5505,7 +5520,7 @@ get_basic_select_query(Query *query, deparse_context *context,
55055520
}
55065521

55075522
/* Then we tell what to select (the targetlist) */
5508-
get_target_list(query->targetList, context, resultDesc);
5523+
get_target_list(query->targetList, context, resultDesc, colNamesVisible);
55095524

55105525
/* Add the FROM clause if needed */
55115526
get_from_clause(query, " FROM ", context);
@@ -5575,11 +5590,13 @@ get_basic_select_query(Query *query, deparse_context *context,
55755590
* get_target_list - Parse back a SELECT target list
55765591
*
55775592
* This is also used for RETURNING lists in INSERT/UPDATE/DELETE.
5593+
*
5594+
* resultDesc and colNamesVisible are as for get_query_def()
55785595
* ----------
55795596
*/
55805597
static void
55815598
get_target_list(List *targetList, deparse_context *context,
5582-
TupleDesc resultDesc)
5599+
TupleDesc resultDesc, bool colNamesVisible)
55835600
{
55845601
StringInfo buf = context->buf;
55855602
StringInfoData targetbuf;
@@ -5630,8 +5647,13 @@ get_target_list(List *targetList, deparse_context *context,
56305647
else
56315648
{
56325649
get_rule_expr((Node *) tle->expr, context, true);
5633-
/* We'll show the AS name unless it's this: */
5634-
attname = "?column?";
5650+
5651+
/*
5652+
* When colNamesVisible is true, we should always show the
5653+
* assigned column name explicitly. Otherwise, show it only if
5654+
* it's not FigureColname's fallback.
5655+
*/
5656+
attname = colNamesVisible ? NULL : "?column?";
56355657
}
56365658

56375659
/*
@@ -5710,7 +5732,7 @@ get_target_list(List *targetList, deparse_context *context,
57105732

57115733
static void
57125734
get_setop_query(Node *setOp, Query *query, deparse_context *context,
5713-
TupleDesc resultDesc)
5735+
TupleDesc resultDesc, bool colNamesVisible)
57145736
{
57155737
StringInfo buf = context->buf;
57165738
bool need_paren;
@@ -5736,6 +5758,7 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
57365758
if (need_paren)
57375759
appendStringInfoChar(buf, '(');
57385760
get_query_def(subquery, buf, context->namespaces, resultDesc,
5761+
colNamesVisible,
57395762
context->prettyFlags, context->wrapColumn,
57405763
context->indentLevel);
57415764
if (need_paren)
@@ -5778,7 +5801,7 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
57785801
else
57795802
subindent = 0;
57805803

5781-
get_setop_query(op->larg, query, context, resultDesc);
5804+
get_setop_query(op->larg, query, context, resultDesc, colNamesVisible);
57825805

57835806
if (need_paren)
57845807
appendContextKeyword(context, ") ", -subindent, 0, 0);
@@ -5822,7 +5845,7 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
58225845
subindent = 0;
58235846
appendContextKeyword(context, "", subindent, 0, 0);
58245847

5825-
get_setop_query(op->rarg, query, context, resultDesc);
5848+
get_setop_query(op->rarg, query, context, resultDesc, false);
58265849

58275850
if (PRETTY_INDENT(context))
58285851
context->indentLevel -= subindent;
@@ -6157,7 +6180,8 @@ get_rule_windowspec(WindowClause *wc, List *targetList,
61576180
* ----------
61586181
*/
61596182
static void
6160-
get_insert_query_def(Query *query, deparse_context *context)
6183+
get_insert_query_def(Query *query, deparse_context *context,
6184+
bool colNamesVisible)
61616185
{
61626186
StringInfo buf = context->buf;
61636187
RangeTblEntry *select_rte = NULL;
@@ -6267,6 +6291,7 @@ get_insert_query_def(Query *query, deparse_context *context)
62676291
{
62686292
/* Add the SELECT */
62696293
get_query_def(select_rte->subquery, buf, NIL, NULL,
6294+
false,
62706295
context->prettyFlags, context->wrapColumn,
62716296
context->indentLevel);
62726297
}
@@ -6360,7 +6385,7 @@ get_insert_query_def(Query *query, deparse_context *context)
63606385
{
63616386
appendContextKeyword(context, " RETURNING",
63626387
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
6363-
get_target_list(query->returningList, context, NULL);
6388+
get_target_list(query->returningList, context, NULL, colNamesVisible);
63646389
}
63656390
}
63666391

@@ -6370,7 +6395,8 @@ get_insert_query_def(Query *query, deparse_context *context)
63706395
* ----------
63716396
*/
63726397
static void
6373-
get_update_query_def(Query *query, deparse_context *context)
6398+
get_update_query_def(Query *query, deparse_context *context,
6399+
bool colNamesVisible)
63746400
{
63756401
StringInfo buf = context->buf;
63766402
RangeTblEntry *rte;
@@ -6415,7 +6441,7 @@ get_update_query_def(Query *query, deparse_context *context)
64156441
{
64166442
appendContextKeyword(context, " RETURNING",
64176443
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
6418-
get_target_list(query->returningList, context, NULL);
6444+
get_target_list(query->returningList, context, NULL, colNamesVisible);
64196445
}
64206446
}
64216447

@@ -6578,7 +6604,8 @@ get_update_query_targetlist_def(Query *query, List *targetList,
65786604
* ----------
65796605
*/
65806606
static void
6581-
get_delete_query_def(Query *query, deparse_context *context)
6607+
get_delete_query_def(Query *query, deparse_context *context,
6608+
bool colNamesVisible)
65826609
{
65836610
StringInfo buf = context->buf;
65846611
RangeTblEntry *rte;
@@ -6619,7 +6646,7 @@ get_delete_query_def(Query *query, deparse_context *context)
66196646
{
66206647
appendContextKeyword(context, " RETURNING",
66216648
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
6622-
get_target_list(query->returningList, context, NULL);
6649+
get_target_list(query->returningList, context, NULL, colNamesVisible);
66236650
}
66246651
}
66256652

@@ -9822,7 +9849,7 @@ get_sublink_expr(SubLink *sublink, deparse_context *context)
98229849
if (need_paren)
98239850
appendStringInfoChar(buf, '(');
98249851

9825-
get_query_def(query, buf, context->namespaces, NULL,
9852+
get_query_def(query, buf, context->namespaces, NULL, false,
98269853
context->prettyFlags, context->wrapColumn,
98279854
context->indentLevel);
98289855

@@ -10068,6 +10095,7 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
1006810095
/* Subquery RTE */
1006910096
appendStringInfoChar(buf, '(');
1007010097
get_query_def(rte->subquery, buf, context->namespaces, NULL,
10098+
true,
1007110099
context->prettyFlags, context->wrapColumn,
1007210100
context->indentLevel);
1007310101
appendStringInfoChar(buf, ')');

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ CREATE VIEW mvtest_vt2 AS SELECT moo, 2*moo FROM mvtest_vt1 UNION ALL SELECT moo
347347
?column? | integer | | | | plain |
348348
View definition:
349349
SELECT mvtest_vt1.moo,
350-
2 * mvtest_vt1.moo
350+
2 * mvtest_vt1.moo AS "?column?"
351351
FROM mvtest_vt1
352352
UNION ALL
353353
SELECT mvtest_vt1.moo,
@@ -363,7 +363,7 @@ CREATE MATERIALIZED VIEW mv_test2 AS SELECT moo, 2*moo FROM mvtest_vt2 UNION ALL
363363
?column? | integer | | | | plain | |
364364
View definition:
365365
SELECT mvtest_vt2.moo,
366-
2 * mvtest_vt2.moo
366+
2 * mvtest_vt2.moo AS "?column?"
367367
FROM mvtest_vt2
368368
UNION ALL
369369
SELECT mvtest_vt2.moo,

0 commit comments

Comments
 (0)