Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit fefd546

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 bb60f25 commit fefd546

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
@@ -384,26 +384,29 @@ static void make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
384384
static void make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
385385
int prettyFlags, int wrapColumn);
386386
static void get_query_def(Query *query, StringInfo buf, List *parentnamespace,
387-
TupleDesc resultDesc,
387+
TupleDesc resultDesc, bool colNamesVisible,
388388
int prettyFlags, int wrapColumn, int startIndent);
389389
static void get_values_def(List *values_lists, deparse_context *context);
390390
static void get_with_clause(Query *query, deparse_context *context);
391391
static void get_select_query_def(Query *query, deparse_context *context,
392-
TupleDesc resultDesc);
393-
static void get_insert_query_def(Query *query, deparse_context *context);
394-
static void get_update_query_def(Query *query, deparse_context *context);
392+
TupleDesc resultDesc, bool colNamesVisible);
393+
static void get_insert_query_def(Query *query, deparse_context *context,
394+
bool colNamesVisible);
395+
static void get_update_query_def(Query *query, deparse_context *context,
396+
bool colNamesVisible);
395397
static void get_update_query_targetlist_def(Query *query, List *targetList,
396398
deparse_context *context,
397399
RangeTblEntry *rte);
398-
static void get_delete_query_def(Query *query, deparse_context *context);
400+
static void get_delete_query_def(Query *query, deparse_context *context,
401+
bool colNamesVisible);
399402
static void get_utility_query_def(Query *query, deparse_context *context);
400403
static void get_basic_select_query(Query *query, deparse_context *context,
401-
TupleDesc resultDesc);
404+
TupleDesc resultDesc, bool colNamesVisible);
402405
static void get_target_list(List *targetList, deparse_context *context,
403-
TupleDesc resultDesc);
406+
TupleDesc resultDesc, bool colNamesVisible);
404407
static void get_setop_query(Node *setOp, Query *query,
405408
deparse_context *context,
406-
TupleDesc resultDesc);
409+
TupleDesc resultDesc, bool colNamesVisible);
407410
static Node *get_rule_sortgroupclause(Index ref, List *tlist,
408411
bool force_colno,
409412
deparse_context *context);
@@ -4897,7 +4900,7 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
48974900
foreach(action, actions)
48984901
{
48994902
query = (Query *) lfirst(action);
4900-
get_query_def(query, buf, NIL, viewResultDesc,
4903+
get_query_def(query, buf, NIL, viewResultDesc, true,
49014904
prettyFlags, WRAP_COLUMN_DEFAULT, 0);
49024905
if (prettyFlags)
49034906
appendStringInfoString(buf, ";\n");
@@ -4915,7 +4918,7 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
49154918
Query *query;
49164919

49174920
query = (Query *) linitial(actions);
4918-
get_query_def(query, buf, NIL, viewResultDesc,
4921+
get_query_def(query, buf, NIL, viewResultDesc, true,
49194922
prettyFlags, WRAP_COLUMN_DEFAULT, 0);
49204923
appendStringInfoChar(buf, ';');
49214924
}
@@ -4989,7 +4992,7 @@ make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
49894992

49904993
ev_relation = table_open(ev_class, AccessShareLock);
49914994

4992-
get_query_def(query, buf, NIL, RelationGetDescr(ev_relation),
4995+
get_query_def(query, buf, NIL, RelationGetDescr(ev_relation), true,
49934996
prettyFlags, wrapColumn, 0);
49944997
appendStringInfoChar(buf, ';');
49954998

@@ -5000,13 +5003,23 @@ make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
50005003
/* ----------
50015004
* get_query_def - Parse back one query parsetree
50025005
*
5003-
* If resultDesc is not NULL, then it is the output tuple descriptor for
5004-
* the view represented by a SELECT query.
5006+
* query: parsetree to be displayed
5007+
* buf: output text is appended to buf
5008+
* parentnamespace: list (initially empty) of outer-level deparse_namespace's
5009+
* resultDesc: if not NULL, the output tuple descriptor for the view
5010+
* represented by a SELECT query. We use the column names from it
5011+
* to label SELECT output columns, in preference to names in the query
5012+
* colNamesVisible: true if the surrounding context cares about the output
5013+
* column names at all (as, for example, an EXISTS() context does not);
5014+
* when false, we can suppress dummy column labels such as "?column?"
5015+
* prettyFlags: bitmask of PRETTYFLAG_XXX options
5016+
* wrapColumn: maximum line length, or -1 to disable wrapping
5017+
* startIndent: initial indentation amount
50055018
* ----------
50065019
*/
50075020
static void
50085021
get_query_def(Query *query, StringInfo buf, List *parentnamespace,
5009-
TupleDesc resultDesc,
5022+
TupleDesc resultDesc, bool colNamesVisible,
50105023
int prettyFlags, int wrapColumn, int startIndent)
50115024
{
50125025
deparse_context context;
@@ -5044,19 +5057,19 @@ get_query_def(Query *query, StringInfo buf, List *parentnamespace,
50445057
switch (query->commandType)
50455058
{
50465059
case CMD_SELECT:
5047-
get_select_query_def(query, &context, resultDesc);
5060+
get_select_query_def(query, &context, resultDesc, colNamesVisible);
50485061
break;
50495062

50505063
case CMD_UPDATE:
5051-
get_update_query_def(query, &context);
5064+
get_update_query_def(query, &context, colNamesVisible);
50525065
break;
50535066

50545067
case CMD_INSERT:
5055-
get_insert_query_def(query, &context);
5068+
get_insert_query_def(query, &context, colNamesVisible);
50565069
break;
50575070

50585071
case CMD_DELETE:
5059-
get_delete_query_def(query, &context);
5072+
get_delete_query_def(query, &context, colNamesVisible);
50605073
break;
50615074

50625075
case CMD_NOTHING:
@@ -5180,6 +5193,7 @@ get_with_clause(Query *query, deparse_context *context)
51805193
if (PRETTY_INDENT(context))
51815194
appendContextKeyword(context, "", 0, 0, 0);
51825195
get_query_def((Query *) cte->ctequery, buf, context->namespaces, NULL,
5196+
true,
51835197
context->prettyFlags, context->wrapColumn,
51845198
context->indentLevel);
51855199
if (PRETTY_INDENT(context))
@@ -5203,7 +5217,7 @@ get_with_clause(Query *query, deparse_context *context)
52035217
*/
52045218
static void
52055219
get_select_query_def(Query *query, deparse_context *context,
5206-
TupleDesc resultDesc)
5220+
TupleDesc resultDesc, bool colNamesVisible)
52075221
{
52085222
StringInfo buf = context->buf;
52095223
List *save_windowclause;
@@ -5227,13 +5241,14 @@ get_select_query_def(Query *query, deparse_context *context,
52275241
*/
52285242
if (query->setOperations)
52295243
{
5230-
get_setop_query(query->setOperations, query, context, resultDesc);
5244+
get_setop_query(query->setOperations, query, context, resultDesc,
5245+
colNamesVisible);
52315246
/* ORDER BY clauses must be simple in this case */
52325247
force_colno = true;
52335248
}
52345249
else
52355250
{
5236-
get_basic_select_query(query, context, resultDesc);
5251+
get_basic_select_query(query, context, resultDesc, colNamesVisible);
52375252
force_colno = false;
52385253
}
52395254

@@ -5403,7 +5418,7 @@ get_simple_values_rte(Query *query, TupleDesc resultDesc)
54035418

54045419
static void
54055420
get_basic_select_query(Query *query, deparse_context *context,
5406-
TupleDesc resultDesc)
5421+
TupleDesc resultDesc, bool colNamesVisible)
54075422
{
54085423
StringInfo buf = context->buf;
54095424
RangeTblEntry *values_rte;
@@ -5456,7 +5471,7 @@ get_basic_select_query(Query *query, deparse_context *context,
54565471
}
54575472

54585473
/* Then we tell what to select (the targetlist) */
5459-
get_target_list(query->targetList, context, resultDesc);
5474+
get_target_list(query->targetList, context, resultDesc, colNamesVisible);
54605475

54615476
/* Add the FROM clause if needed */
54625477
get_from_clause(query, " FROM ", context);
@@ -5526,11 +5541,13 @@ get_basic_select_query(Query *query, deparse_context *context,
55265541
* get_target_list - Parse back a SELECT target list
55275542
*
55285543
* This is also used for RETURNING lists in INSERT/UPDATE/DELETE.
5544+
*
5545+
* resultDesc and colNamesVisible are as for get_query_def()
55295546
* ----------
55305547
*/
55315548
static void
55325549
get_target_list(List *targetList, deparse_context *context,
5533-
TupleDesc resultDesc)
5550+
TupleDesc resultDesc, bool colNamesVisible)
55345551
{
55355552
StringInfo buf = context->buf;
55365553
StringInfoData targetbuf;
@@ -5581,8 +5598,13 @@ get_target_list(List *targetList, deparse_context *context,
55815598
else
55825599
{
55835600
get_rule_expr((Node *) tle->expr, context, true);
5584-
/* We'll show the AS name unless it's this: */
5585-
attname = "?column?";
5601+
5602+
/*
5603+
* When colNamesVisible is true, we should always show the
5604+
* assigned column name explicitly. Otherwise, show it only if
5605+
* it's not FigureColname's fallback.
5606+
*/
5607+
attname = colNamesVisible ? NULL : "?column?";
55865608
}
55875609

55885610
/*
@@ -5661,7 +5683,7 @@ get_target_list(List *targetList, deparse_context *context,
56615683

56625684
static void
56635685
get_setop_query(Node *setOp, Query *query, deparse_context *context,
5664-
TupleDesc resultDesc)
5686+
TupleDesc resultDesc, bool colNamesVisible)
56655687
{
56665688
StringInfo buf = context->buf;
56675689
bool need_paren;
@@ -5687,6 +5709,7 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
56875709
if (need_paren)
56885710
appendStringInfoChar(buf, '(');
56895711
get_query_def(subquery, buf, context->namespaces, resultDesc,
5712+
colNamesVisible,
56905713
context->prettyFlags, context->wrapColumn,
56915714
context->indentLevel);
56925715
if (need_paren)
@@ -5729,7 +5752,7 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
57295752
else
57305753
subindent = 0;
57315754

5732-
get_setop_query(op->larg, query, context, resultDesc);
5755+
get_setop_query(op->larg, query, context, resultDesc, colNamesVisible);
57335756

57345757
if (need_paren)
57355758
appendContextKeyword(context, ") ", -subindent, 0, 0);
@@ -5773,7 +5796,7 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
57735796
subindent = 0;
57745797
appendContextKeyword(context, "", subindent, 0, 0);
57755798

5776-
get_setop_query(op->rarg, query, context, resultDesc);
5799+
get_setop_query(op->rarg, query, context, resultDesc, false);
57775800

57785801
if (PRETTY_INDENT(context))
57795802
context->indentLevel -= subindent;
@@ -6108,7 +6131,8 @@ get_rule_windowspec(WindowClause *wc, List *targetList,
61086131
* ----------
61096132
*/
61106133
static void
6111-
get_insert_query_def(Query *query, deparse_context *context)
6134+
get_insert_query_def(Query *query, deparse_context *context,
6135+
bool colNamesVisible)
61126136
{
61136137
StringInfo buf = context->buf;
61146138
RangeTblEntry *select_rte = NULL;
@@ -6218,6 +6242,7 @@ get_insert_query_def(Query *query, deparse_context *context)
62186242
{
62196243
/* Add the SELECT */
62206244
get_query_def(select_rte->subquery, buf, NIL, NULL,
6245+
false,
62216246
context->prettyFlags, context->wrapColumn,
62226247
context->indentLevel);
62236248
}
@@ -6311,7 +6336,7 @@ get_insert_query_def(Query *query, deparse_context *context)
63116336
{
63126337
appendContextKeyword(context, " RETURNING",
63136338
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
6314-
get_target_list(query->returningList, context, NULL);
6339+
get_target_list(query->returningList, context, NULL, colNamesVisible);
63156340
}
63166341
}
63176342

@@ -6321,7 +6346,8 @@ get_insert_query_def(Query *query, deparse_context *context)
63216346
* ----------
63226347
*/
63236348
static void
6324-
get_update_query_def(Query *query, deparse_context *context)
6349+
get_update_query_def(Query *query, deparse_context *context,
6350+
bool colNamesVisible)
63256351
{
63266352
StringInfo buf = context->buf;
63276353
RangeTblEntry *rte;
@@ -6366,7 +6392,7 @@ get_update_query_def(Query *query, deparse_context *context)
63666392
{
63676393
appendContextKeyword(context, " RETURNING",
63686394
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
6369-
get_target_list(query->returningList, context, NULL);
6395+
get_target_list(query->returningList, context, NULL, colNamesVisible);
63706396
}
63716397
}
63726398

@@ -6528,7 +6554,8 @@ get_update_query_targetlist_def(Query *query, List *targetList,
65286554
* ----------
65296555
*/
65306556
static void
6531-
get_delete_query_def(Query *query, deparse_context *context)
6557+
get_delete_query_def(Query *query, deparse_context *context,
6558+
bool colNamesVisible)
65326559
{
65336560
StringInfo buf = context->buf;
65346561
RangeTblEntry *rte;
@@ -6569,7 +6596,7 @@ get_delete_query_def(Query *query, deparse_context *context)
65696596
{
65706597
appendContextKeyword(context, " RETURNING",
65716598
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
6572-
get_target_list(query->returningList, context, NULL);
6599+
get_target_list(query->returningList, context, NULL, colNamesVisible);
65736600
}
65746601
}
65756602

@@ -9896,7 +9923,7 @@ get_sublink_expr(SubLink *sublink, deparse_context *context)
98969923
if (need_paren)
98979924
appendStringInfoChar(buf, '(');
98989925

9899-
get_query_def(query, buf, context->namespaces, NULL,
9926+
get_query_def(query, buf, context->namespaces, NULL, false,
99009927
context->prettyFlags, context->wrapColumn,
99019928
context->indentLevel);
99029929

@@ -10142,6 +10169,7 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
1014210169
/* Subquery RTE */
1014310170
appendStringInfoChar(buf, '(');
1014410171
get_query_def(rte->subquery, buf, context->namespaces, NULL,
10172+
true,
1014510173
context->prettyFlags, context->wrapColumn,
1014610174
context->indentLevel);
1014710175
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)