Show 'AS "?column?"' explicitly when it's important.
authorTom Lane <[email protected]>
Sat, 21 May 2022 18:45:58 +0000 (14:45 -0400)
committerTom Lane <[email protected]>
Sat, 21 May 2022 18:45:58 +0000 (14:45 -0400)
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

src/backend/utils/adt/ruleutils.c
src/test/regress/expected/matview.out

index cc59815615bde572acbe13940bee4313b439efa4..8ddfa3306456ea0e8c93684fefa6ba127bb2dcd9 100644 (file)
@@ -370,26 +370,29 @@ 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);
-static void get_insert_query_def(Query *query, deparse_context *context);
-static void get_update_query_def(Query *query, deparse_context *context);
+                                TupleDesc resultDesc, bool colNamesVisible);
+static void get_insert_query_def(Query *query, deparse_context *context,
+                                bool colNamesVisible);
+static void get_update_query_def(Query *query, deparse_context *context,
+                                bool colNamesVisible);
 static void get_update_query_targetlist_def(Query *query, List *targetList,
                                deparse_context *context,
                                RangeTblEntry *rte);
-static void get_delete_query_def(Query *query, deparse_context *context);
+static void get_delete_query_def(Query *query, deparse_context *context,
+                                bool colNamesVisible);
 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);
@@ -4788,7 +4791,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");
@@ -4806,7 +4809,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, ';');
    }
@@ -4880,7 +4883,7 @@ make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
 
    ev_relation = heap_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, ';');
 
@@ -4891,13 +4894,23 @@ make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
 /* ----------
  * get_query_def           - Parse back one query parsetree
  *
- * If resultDesc is not NULL, then it is the output tuple descriptor for
- * the view represented by a SELECT query.
+ * query: parsetree to be displayed
+ * buf: output text is appended to buf
+ * parentnamespace: list (initially empty) of outer-level deparse_namespace's
+ * resultDesc: if not NULL, the output tuple descriptor for the view
+ *     represented by a SELECT query.  We use the column names from it
+ *     to label SELECT output columns, in preference to names in the query
+ * colNamesVisible: true if the surrounding context cares about the output
+ *     column names at all (as, for example, an EXISTS() context does not);
+ *     when false, we can suppress dummy column labels such as "?column?"
+ * prettyFlags: bitmask of PRETTYFLAG_XXX options
+ * wrapColumn: maximum line length, or -1 to disable wrapping
+ * startIndent: initial indentation amount
  * ----------
  */
 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;
@@ -4934,19 +4947,19 @@ 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:
-           get_update_query_def(query, &context);
+           get_update_query_def(query, &context, colNamesVisible);
            break;
 
        case CMD_INSERT:
-           get_insert_query_def(query, &context);
+           get_insert_query_def(query, &context, colNamesVisible);
            break;
 
        case CMD_DELETE:
-           get_delete_query_def(query, &context);
+           get_delete_query_def(query, &context, colNamesVisible);
            break;
 
        case CMD_NOTHING:
@@ -5058,6 +5071,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))
@@ -5081,7 +5095,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;
@@ -5105,13 +5119,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;
    }
 
@@ -5268,7 +5283,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;
@@ -5321,7 +5336,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);
@@ -5391,11 +5406,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.
+ *
+ * resultDesc and colNamesVisible are as for get_query_def()
  * ----------
  */
 static void
 get_target_list(List *targetList, deparse_context *context,
-               TupleDesc resultDesc)
+               TupleDesc resultDesc, bool colNamesVisible)
 {
    StringInfo  buf = context->buf;
    StringInfoData targetbuf;
@@ -5446,8 +5463,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?";
        }
 
        /*
@@ -5526,7 +5548,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;
@@ -5552,6 +5574,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)
@@ -5594,7 +5617,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);
@@ -5638,7 +5661,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;
@@ -5965,7 +5988,8 @@ get_rule_windowspec(WindowClause *wc, List *targetList,
  * ----------
  */
 static void
-get_insert_query_def(Query *query, deparse_context *context)
+get_insert_query_def(Query *query, deparse_context *context,
+                    bool colNamesVisible)
 {
    StringInfo  buf = context->buf;
    RangeTblEntry *select_rte = NULL;
@@ -6074,6 +6098,7 @@ get_insert_query_def(Query *query, deparse_context *context)
    {
        /* Add the SELECT */
        get_query_def(select_rte->subquery, buf, NIL, NULL,
+                     false,
                      context->prettyFlags, context->wrapColumn,
                      context->indentLevel);
    }
@@ -6167,7 +6192,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, colNamesVisible);
    }
 }
 
@@ -6177,7 +6202,8 @@ get_insert_query_def(Query *query, deparse_context *context)
  * ----------
  */
 static void
-get_update_query_def(Query *query, deparse_context *context)
+get_update_query_def(Query *query, deparse_context *context,
+                    bool colNamesVisible)
 {
    StringInfo  buf = context->buf;
    RangeTblEntry *rte;
@@ -6222,7 +6248,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, colNamesVisible);
    }
 }
 
@@ -6383,7 +6409,8 @@ get_update_query_targetlist_def(Query *query, List *targetList,
  * ----------
  */
 static void
-get_delete_query_def(Query *query, deparse_context *context)
+get_delete_query_def(Query *query, deparse_context *context,
+                    bool colNamesVisible)
 {
    StringInfo  buf = context->buf;
    RangeTblEntry *rte;
@@ -6424,7 +6451,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, colNamesVisible);
    }
 }
 
@@ -9624,7 +9651,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);
 
@@ -9883,6 +9910,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, ')');
index f52ee813bad325a6474fc00600fe93a594b295dd..6577d8284a416cae75c072b8c2a3366b69b7cb22 100644 (file)
@@ -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,