8000 Show 'AS "?column?"' explicitly when it's important. · postgres/postgres@f3b8d72 · GitHub
[go: up one dir, main page]
Skip to content

Commit f3b8d72

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 ee27871 commit f3b8d72

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
@@ -371,26 +371,29 @@ static void make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
371371
static void make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
372372
int prettyFlags, int wrapColumn);
373373
static void get_query_def(Query *query, StringInfo buf, List *parentnamespace,
374-
TupleDesc resultDesc,
374+
TupleDesc resultDesc, bool colNamesVisible,
375375
int prettyFlags, int wrapColumn, int startIndent);
376376
static void get_values_def(List *values_lists, deparse_context *context);
377377
static void get_with_clause(Query *query, deparse_context *context);
378378
static void get_select_query_def(Query *query, deparse_context *context,
379-
TupleDesc resultDesc);
380-
static void get_insert_query_def(Query *query, deparse_context *context);
381-
static void get_update_query_def(Query *query, deparse_context *context);
379+
TupleDesc resultDesc, bool colNamesVisible);
380+
static void get_insert_query_def(Query *query, deparse_context *context,
381+
bool colNamesVisible);
382+
static void get_update_query_def(Query *query, deparse_context *context,
383+
bool colNamesVisible);
382384
static void get_update_query_targetlist_def(Query *query, List *targetList,
383385
deparse_context *context,
384386
RangeTblEntry *rte);
385-
static void get_delete_query_def(Query *query, deparse_context *context);
387+
static void get_delete_query_def(Query *query, deparse_context *context,
388+
bool colNamesVisible);
386389
static void get_utility_query_def(Query *query, deparse_context *context);
387390
static void get_basic_select_query(Query *query, deparse_context *context,
388-
TupleDesc resultDesc);
391+
TupleDesc resultDesc, bool colNamesVisible);
389392
static void get_target_list(List *targetList, deparse_context *context,
390-
TupleDesc resultDesc);
393+
TupleDesc resultDesc, bool colNamesVisible);
391< 8000 /code>394
static void get_setop_query(Node *setOp, Query *query,
392395
deparse_context *context,
393-
TupleDesc resultDesc);
396+
TupleDesc resultDesc, bool colNamesVisible);
394397
static Node *get_rule_sortgroupclause(Index ref, List *tlist,
395398
bool force_colno,
396399
deparse_context *context);
@@ -4899,7 +4902,7 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
48994902
foreach(action, actions)
49004903
{
49014904
query = (Query *) lfirst(action);
4902-
get_query_def(query, buf, NIL, viewResultDesc,
4905+
get_query_def(query, buf, NIL, viewResultDesc, true,
49034906
prettyFlags, WRAP_COLUMN_DEFAULT, 0);
49044907
if (prettyFlags)
49054908
appendStringInfoString(buf, ";\n");
@@ -4917,7 +4920,7 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
49174920
Query *query;
49184921

49194922
query = (Query *) linitial(actions);
4920-
get_query_def(query, buf, NIL, viewResultDesc,
4923+
get_query_def(query, buf, NIL, viewResultDesc, true,
49214924
prettyFlags, WRAP_COLUMN_DEFAULT, 0);
49224925
appendStringInfoChar(buf, ';');
49234926
}
@@ -4991,7 +4994,7 @@ make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
49914994

49924995
ev_relation = heap_open(ev_class, AccessShareLock);
49934996

4994-
get_query_def(query, buf, NIL, RelationGetDescr(ev_relation),
4997+
get_query_def(query, buf, NIL, RelationGetDescr(ev_relation), true,
49954998
prettyFlags, wrapColumn, 0);
49964999
appendStringInfoChar(buf, ';');
49975000

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

50515064
case CMD_UPDATE:
5052-
get_update_query_def(query, &context);
5065+
get_update_query_def(query, &context, colNamesVisible);
50535066
break;
50545067

50555068
case CMD_INSERT:
5056-
get_insert_query_def(query, &context);
5069+
get_insert_query_def(query, &context, colNamesVisible);
50575070
break;
50585071

50595072
case CMD_DELETE:
5060-
get_delete_query_def(query, &context);
5073+
get_delete_query_def(query, &context, colNamesVisible);
50615074
break;
50625075

50635076
case CMD_NOTHING:
@@ -5169,6 +5182,7 @@ get_with_clause(Query *query, deparse_context *context)
51695182
if (PRETTY_INDENT(context))
51705183
appendContextKeyword(context, "", 0, 0, 0);
51715184
get_query_def((Query *) cte->ctequery, buf, context->namespaces, NULL,
5185+
true,
51725186
context->prettyFlags, context->wrapColumn,
51735187
context->indentLevel);
51745188
if (PRETTY_INDENT(context))
@@ -5192,7 +5206,7 @@ get_with_clause(Query *query, deparse_context *context)
51925206
*/
51935207
static void
51945208
get_select_query_def(Query *query, deparse_context *context,
5195-
TupleDesc resultDesc)
5209+
TupleDesc resultDesc, bool colNamesVisible)
51965210
{
51975211
StringInfo buf = context->buf;
51985212
List *save_windowclause;
@@ -5216,13 +5230,14 @@ get_select_query_def(Query *query, deparse_context *context,
52165230
*/
52175231
if (query->setOperations)
52185232
{
5219-
get_setop_query(query->setOperations, query, context, resultDesc);
5233+
get_setop_query(query->setOperations, query, context, resultDesc,
5234+
colNamesVisible);
52205235
/* ORDER BY clauses must be simple in this case */
52215236
force_colno = true;
52225237
}
52235238
else
52245239
{
5225-
get_basic_select_query(query, context, resultDesc);
5240+
get_basic_select_query(query, context, resultDesc, colNamesVisible);
52265241
force_colno = false;
52275242
}
52285243

@@ -5379,7 +5394,7 @@ get_simple_values_rte(Query *query, TupleDesc resultDesc)
53795394

53805395
static void
53815396
get_basic_select_query(Query *query, deparse_context *context,
5382-
TupleDesc resultDesc)
5397+
TupleDesc resultDesc, bool colNamesVisible)
53835398
{
53845399
StringInfo buf = context->buf;
53855400
RangeTblEntry *values_rte;
@@ -5432,7 +5447,7 @@ get_basic_select_query(Query *query, deparse_context *context,
54325447
}
54335448

54345449
/* Then we tell what to select (the targetlist) */
5435-
get_target_list(query->targetList, context, resultDesc);
5450+
get_target_list(query->targetList, context, resultDesc, colNamesVisible);
54365451

54375452
/* Add the FROM clause if needed */
54385453
get_from_clause(query, " FROM ", context);
@@ -5502,11 +5517,13 @@ get_basic_select_query(Query *query, deparse_context *context,
55025517
* get_target_list - Parse back a SELECT target list
55035518
*
55045519
* This is also used for RETURNING lists in INSERT/UPDATE/DELETE.
5520+
*
5521+
* resultDesc and colNamesVisible are as for get_query_def()
55055522
* ----------
55065523
*/
55075524
static void
55085525
get_target_list(List *targetList, deparse_context *context,
5509-
TupleDesc resultDesc)
5526+
TupleDesc resultDesc, bool colNamesVisible)
55105527
{
55115528
StringInfo buf = context->buf;
55125529
StringInfoData targetbuf;
@@ -5557,8 +5574,13 @@ get_target_list(List *targetList, deparse_context *context,
55575574
else
55585575
{
55595576
get_rule_expr((Node *) tle->expr, context, true);
5560-
/* We'll show the AS name unless it's this: */
5561-
attname = "?column?";
5577+
5578+
/*
5579+
* When colNamesVisible is true, we should always show the
5580+
* assigned column name explicitly. Otherwise, show it only if
5581+
* it's not FigureColname's fallback.
5582+
*/
5583+
attname = colNamesVisible ? NULL : "?column?";
55625584
}
55635585

55645586
/*
@@ -5637,7 +5659,7 @@ get_target_list(List *targetList, deparse_context *context,
56375659

56385660
static void
56395661
get_setop_query(Node *setOp, Query *query, deparse_context *context,
5640-
TupleDesc resultDesc)
5662+
TupleDesc resultDesc, bool colNamesVisible)
56415663
{
56425664
StringInfo buf = context->buf;
56435665
bool need_paren;
@@ -5663,6 +5685,7 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
56635685
if (need_paren)
56645686
appendStringInfoChar(buf, '(');
56655687
get_query_def(subquery, buf, context->namespaces, resultDesc,
5688+
colNamesVisible,
56665689
context->prettyFlags, context->wrapColumn,
56675690
context->indentLevel);
56685691
if (need_paren)
@@ -5705,7 +5728,7 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
57055728
else
57065729
subindent = 0;
57075730

5708-
get_setop_query(op->larg, query, context, resultDesc);
5731+
get_setop_query(op->larg, query, context, resultDesc, colNamesVisible);
57095732

57105733
if (need_paren)
57115734
appendContextKeyword(context, ") ", -subindent, 0, 0);
@@ -5749,7 +5772,7 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
57495772
subindent = 0;
57505773
appendContextKeyword(context, "", subindent, 0, 0);
57515774

5752-
get_setop_query(op->rarg, query, context, resultDesc);
5775+
get_setop_query(op->rarg, query, context, resultDesc, false);
57535776

57545777
if (PRETTY_INDENT(context))
57555778
context->indentLevel -= subindent;
@@ -6084,7 +6107,8 @@ get_rule_windowspec(WindowClause *wc, List *targetList,
60846107
* ----------
60856108
*/
60866109
static void
6087-
get_insert_query_def(Query *query, deparse_context *context)
6110+
get_insert_query_def(Query *query, deparse_context *context,
6111+
bool colNamesVisible)
60886112
{
60896113
StringInfo buf = context->buf;
60906114
RangeTblEntry *select_rte = NULL;
@@ -6194,6 +6218,7 @@ get_insert_query_def(Query *query, deparse_context *context)
61946218
{
61956219
/* Add the SELECT */
61966220
get_query_def(select_rte->subquery, buf, NIL, NULL,
6221+
false,
61976222
context->prettyFlags, context->wrapColumn,
61986223
context->indentLevel);
61996224
}
@@ -6287,7 +6312,7 @@ get_insert_query_def(Query *query, deparse_context *context)
62876312
{
62886313
appendContextKeyword(context, " RETURNING",
62896314
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
6290-
get_target_list(query->returningList, context, NULL);
6315+
get_target_list(query->returningList, context, NULL, colNamesVisible);
62916316
}
62926317
}
62936318

@@ -6297,7 +6322,8 @@ get_insert_query_def(Query *query, deparse_context *context)
62976322
* ----------
62986323
*/
62996324
static void
6300-
get_update_query_def(Query *query, deparse_context *context)
6325+
get_update_query_def(Query *query, deparse_context *context,
6326+
bool colNamesVisible)
63016327
{
63026328
StringInfo buf = context->buf;
63036329
RangeTblEntry *rte;
@@ -6342,7 +6368,7 @@ get_update_query_def(Query *query, deparse_context *context)
63426368
{
63436369
appendContextKeyword(context, " RETURNING",
63446370
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
6345-
get_target_list(query->returningList, context, NULL);
6371+
get_target_list(query->returningList, context, NULL, colNamesVisible);
63466372
}
63476373
}
63486374

@@ -6504,7 +6530,8 @@ get_update_query_targetlist_def(Query *query, List *targetList,
65046530
* ----------
65056531
*/
65066532
static void
6507-
get_delete_query_def(Query *query, deparse_context *context)
6533+
get_delete_query_def(Query *query, deparse_context *context,
6534+
bool colNamesVisible)
65086535
{
65096536
StringInfo buf = context->buf;
65106537
RangeTblEntry *rte;
@@ -6545,7 +6572,7 @@ get_delete_query_def(Query *query, deparse_context *context)
65456572
{
65466573
appendContextKeyword(context, " RETURNING",
65476574
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
6548-
get_target_list(query->returningList, context, NULL);
6575+
get_target_list(query->returningList, context, NULL, colNamesVisible);
65496576
}
65506577
}
65516578

@@ -9745,7 +9772,7 @@ get_sublink_expr(SubLink *sublink, deparse_context *context)
97459772
if (need_paren)
97469773
appendStringInfoChar(buf, '(');
97479774

9748-
get_query_def(query, buf, context->namespaces, NULL,
9775+
get_query_def(query, buf, context->namespaces, NULL, false,
97499776
context->prettyFlags, context->wrapColumn,
97509777
context->indentLevel);
97519778

@@ -10004,6 +10031,7 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
1000410031
/* Subquery RTE */
1000510032
appendStringInfoChar(buf, '(');
1000610033
get_query_def(rte->subquery, buf, context->namespaces, NULL,
10034+
true,
1000710035
context->prettyFlags, context->wrapColumn,
1000810036
context->indentLevel);
1000910037
appendStringInfoCha B3A7 r(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)
0