Fix some minor issues in view pretty-printing.
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 24 Dec 2012 22:52:19 +0000 (17:52 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 24 Dec 2012 22:52:19 +0000 (17:52 -0500)
Code review for commit 2f582f76b1945929ff07116cd4639747ce9bb8a1: don't use
a static variable for what ought to be a deparse_context field, fix
non-multibyte-safe test for spaces, avoid useless and potentially O(N^2)
(though admittedly with a very small constant) calculations of wrap
positions when we aren't going to wrap.

doc/src/sgml/func.sgml
src/backend/utils/adt/ruleutils.c

index e184d99626e6a86e36bebc45de2dd20ffd94652a..35c7f75eab2626c3d7a644c39e42101d557b995c 100644 (file)
@@ -13858,7 +13858,7 @@ SELECT pg_type_is_visible('myschema.widget'::regtype);
       <row>
        <entry><literal><function>pg_get_viewdef(<parameter>view_name</parameter>, <parameter>pretty_bool</>)</function></literal></entry>
        <entry><type>text</type></entry>
-       <entry>get underlying <command>SELECT</command> command for view,
+       <entry>get underlying <command>SELECT</command> command for view;
               lines with fields are wrapped to 80 columns if <parameter>pretty_bool</parameter> is true (<emphasis>deprecated</emphasis>)</entry>
       </row>
       <row>
@@ -13869,14 +13869,15 @@ SELECT pg_type_is_visible('myschema.widget'::regtype);
       <row>
        <entry><literal><function>pg_get_viewdef(<parameter>view_oid</parameter>, <parameter>pretty_bool</>)</function></literal></entry>
        <entry><type>text</type></entry>
-       <entry>get underlying <command>SELECT</command> command for view,
+       <entry>get underlying <command>SELECT</command> command for view;
               lines with fields are wrapped to 80 columns if <parameter>pretty_bool</parameter> is true</entry>
       </row>
       <row>
-       <entry><literal><function>pg_get_viewdef(<parameter>view_oid</parameter>, <parameter>wrap_int</>)</function></literal></entry>
+       <entry><literal><function>pg_get_viewdef(<parameter>view_oid</parameter>, <parameter>wrap_column_int</>)</function></literal></entry>
        <entry><type>text</type></entry>
-       <entry>get underlying <command>SELECT</command> command for view,
-              wrapping lines with fields as specified, pretty printing is implied</entry>
+       <entry>get underlying <command>SELECT</command> command for view;
+              lines with fields are wrapped to specified number of columns,
+              pretty printing is implied</entry>
       </row>
       <row>
        <entry><literal><function>pg_options_to_table(<parameter>reloptions</parameter>)</function></literal></entry>
index b7aff1189b26909a7bc6b0db6c40685ba419f6f4..f58418e0ceb257a59862096f52bfb03f8b327655 100644 (file)
@@ -74,7 +74,8 @@
 #define PRETTYFLAG_PAREN       1
 #define PRETTYFLAG_INDENT      2
 
-#define PRETTY_WRAP_DEFAULT        79
+/* Default line length for pretty-print wrapping */
+#define WRAP_COLUMN_DEFAULT        79
 
 /* macro to test if pretty action needed */
 #define PRETTY_PAREN(context)  ((context)->prettyFlags & PRETTYFLAG_PAREN)
@@ -94,6 +95,7 @@ typedef struct
    List       *windowClause;   /* Current query level's WINDOW clause */
    List       *windowTList;    /* targetlist for resolving WINDOW clause */
    int         prettyFlags;    /* enabling of pretty-print functions */
+   int         wrapColumn;     /* max line length, or -1 for no limit */
    int         indentLevel;    /* current indent level for prettyprint */
    bool        varprefix;      /* TRUE to print prefixes on Vars */
 } deparse_context;
@@ -142,7 +144,6 @@ static SPIPlanPtr plan_getrulebyoid = NULL;
 static const char *query_getrulebyoid = "SELECT * FROM pg_catalog.pg_rewrite WHERE oid = $1";
 static SPIPlanPtr plan_getviewrule = NULL;
 static const char *query_getviewrule = "SELECT * FROM pg_catalog.pg_rewrite WHERE ev_class = $1 AND rulename = $2";
-static int pretty_wrap = PRETTY_WRAP_DEFAULT;
 
 /* GUC parameters */
 bool       quote_all_identifiers = false;
@@ -159,7 +160,8 @@ bool        quote_all_identifiers = false;
 static char *deparse_expression_pretty(Node *expr, List *dpcontext,
                          bool forceprefix, bool showimplicit,
                          int prettyFlags, int startIndent);
-static char *pg_get_viewdef_worker(Oid viewoid, int prettyFlags);
+static char *pg_get_viewdef_worker(Oid viewoid,
+                     int prettyFlags, int wrapColumn);
 static char *pg_get_triggerdef_worker(Oid trigid, bool pretty);
 static void decompile_column_index_array(Datum column_index_array, Oid relId,
                             StringInfo buf);
@@ -192,9 +194,10 @@ static void pop_ancestor_plan(deparse_namespace *dpns,
 static void make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
             int prettyFlags);
 static void make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
-            int prettyFlags);
+            int prettyFlags, int wrapColumn);
 static void get_query_def(Query *query, StringInfo buf, List *parentnamespace,
-             TupleDesc resultDesc, int prettyFlags, int startIndent);
+             TupleDesc resultDesc,
+             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,
@@ -374,7 +377,7 @@ pg_get_viewdef(PG_FUNCTION_ARGS)
    /* By OID */
    Oid         viewoid = PG_GETARG_OID(0);
 
-   PG_RETURN_TEXT_P(string_to_text(pg_get_viewdef_worker(viewoid, 0)));
+   PG_RETURN_TEXT_P(string_to_text(pg_get_viewdef_worker(viewoid, 0, -1)));
 }
 
 
@@ -387,7 +390,7 @@ pg_get_viewdef_ext(PG_FUNCTION_ARGS)
    int         prettyFlags;
 
    prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : 0;
-   PG_RETURN_TEXT_P(string_to_text(pg_get_viewdef_worker(viewoid, prettyFlags)));
+   PG_RETURN_TEXT_P(string_to_text(pg_get_viewdef_worker(viewoid, prettyFlags, WRAP_COLUMN_DEFAULT)));
 }
 
 Datum
@@ -401,9 +404,7 @@ pg_get_viewdef_wrap(PG_FUNCTION_ARGS)
 
    /* calling this implies we want pretty printing */
    prettyFlags = PRETTYFLAG_PAREN | PRETTYFLAG_INDENT;
-   pretty_wrap = wrap;
-   result = pg_get_viewdef_worker(viewoid, prettyFlags);
-   pretty_wrap = PRETTY_WRAP_DEFAULT;
+   result = pg_get_viewdef_worker(viewoid, prettyFlags, wrap);
    PG_RETURN_TEXT_P(string_to_text(result));
 }
 
@@ -419,7 +420,7 @@ pg_get_viewdef_name(PG_FUNCTION_ARGS)
    viewrel = makeRangeVarFromNameList(textToQualifiedNameList(viewname));
    viewoid = RangeVarGetRelid(viewrel, NoLock, false);
 
-   PG_RETURN_TEXT_P(string_to_text(pg_get_viewdef_worker(viewoid, 0)));
+   PG_RETURN_TEXT_P(string_to_text(pg_get_viewdef_worker(viewoid, 0, -1)));
 }
 
 
@@ -439,14 +440,14 @@ pg_get_viewdef_name_ext(PG_FUNCTION_ARGS)
    viewrel = makeRangeVarFromNameList(textToQualifiedNameList(viewname));
    viewoid = RangeVarGetRelid(viewrel, NoLock, false);
 
-   PG_RETURN_TEXT_P(string_to_text(pg_get_viewdef_worker(viewoid, prettyFlags)));
+   PG_RETURN_TEXT_P(string_to_text(pg_get_viewdef_worker(viewoid, prettyFlags, WRAP_COLUMN_DEFAULT)));
 }
 
 /*
  * Common code for by-OID and by-name variants of pg_get_viewdef
  */
 static char *
-pg_get_viewdef_worker(Oid viewoid, int prettyFlags)
+pg_get_viewdef_worker(Oid viewoid, int prettyFlags, int wrapColumn)
 {
    Datum       args[2];
    char        nulls[2];
@@ -504,7 +505,7 @@ pg_get_viewdef_worker(Oid viewoid, int prettyFlags)
         */
        ruletup = SPI_tuptable->vals[0];
        rulettc = SPI_tuptable->tupdesc;
-       make_viewdef(&buf, ruletup, rulettc, prettyFlags);
+       make_viewdef(&buf, ruletup, rulettc, prettyFlags, wrapColumn);
    }
 
    /*
@@ -711,6 +712,7 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty)
        context.windowTList = NIL;
        context.varprefix = true;
        context.prettyFlags = pretty ? PRETTYFLAG_PAREN : 0;
+       context.wrapColumn = WRAP_COLUMN_DEFAULT;
        context.indentLevel = PRETTYINDENT_STD;
 
        get_rule_expr(qual, &context, false);
@@ -2157,6 +2159,7 @@ deparse_expression_pretty(Node *expr, List *dpcontext,
    context.windowTList = NIL;
    context.varprefix = forceprefix;
    context.prettyFlags = prettyFlags;
+   context.wrapColumn = WRAP_COLUMN_DEFAULT;
    context.indentLevel = startIndent;
 
    get_rule_expr(expr, &context, showimplicit);
@@ -2678,6 +2681,7 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
        context.windowTList = NIL;
        context.varprefix = (list_length(query->rtable) != 1);
        context.prettyFlags = prettyFlags;
+       context.wrapColumn = WRAP_COLUMN_DEFAULT;
        context.indentLevel = PRETTYINDENT_STD;
 
        memset(&dpns, 0, sizeof(dpns));
@@ -2704,7 +2708,8 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
        foreach(action, actions)
        {
            query = (Query *) lfirst(action);
-           get_query_def(query, buf, NIL, NULL, prettyFlags, 0);
+           get_query_def(query, buf, NIL, NULL,
+                         prettyFlags, WRAP_COLUMN_DEFAULT, 0);
            if (prettyFlags)
                appendStringInfo(buf, ";\n");
            else
@@ -2721,7 +2726,8 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
        Query      *query;
 
        query = (Query *) linitial(actions);
-       get_query_def(query, buf, NIL, NULL, prettyFlags, 0);
+       get_query_def(query, buf, NIL, NULL,
+                     prettyFlags, WRAP_COLUMN_DEFAULT, 0);
        appendStringInfo(buf, ";");
    }
 }
@@ -2734,7 +2740,7 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
  */
 static void
 make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
-            int prettyFlags)
+            int prettyFlags, int wrapColumn)
 {
    Query      *query;
    char        ev_type;
@@ -2789,7 +2795,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),
-                 prettyFlags, 0);
+                 prettyFlags, wrapColumn, 0);
    appendStringInfo(buf, ";");
 
    heap_close(ev_relation, AccessShareLock);
@@ -2805,7 +2811,8 @@ make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
  */
 static void
 get_query_def(Query *query, StringInfo buf, List *parentnamespace,
-             TupleDesc resultDesc, int prettyFlags, int startIndent)
+             TupleDesc resultDesc,
+             int prettyFlags, int wrapColumn, int startIndent)
 {
    deparse_context context;
    deparse_namespace dpns;
@@ -2825,6 +2832,7 @@ get_query_def(Query *query, StringInfo buf, List *parentnamespace,
    context.varprefix = (parentnamespace != NIL ||
                         list_length(query->rtable) != 1);
    context.prettyFlags = prettyFlags;
+   context.wrapColumn = wrapColumn;
    context.indentLevel = startIndent;
 
    memset(&dpns, 0, sizeof(dpns));
@@ -2961,7 +2969,8 @@ 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,
-                     context->prettyFlags, context->indentLevel);
+                     context->prettyFlags, context->wrapColumn,
+                     context->indentLevel);
        if (PRETTY_INDENT(context))
            appendContextKeyword(context, "", 0, 0, 0);
        appendStringInfoChar(buf, ')');
@@ -3218,10 +3227,14 @@ get_target_list(List *targetList, deparse_context *context,
                TupleDesc resultDesc)
 {
    StringInfo  buf = context->buf;
+   StringInfoData targetbuf;
+   bool        last_was_multiline = false;
    char       *sep;
    int         colno;
    ListCell   *l;
-   bool        last_was_multiline = false;
+
+   /* we use targetbuf to hold each TLE's text temporarily */
+   initStringInfo(&targetbuf);
 
    sep = " ";
    colno = 0;
@@ -3230,10 +3243,6 @@ get_target_list(List *targetList, deparse_context *context,
        TargetEntry *tle = (TargetEntry *) lfirst(l);
        char       *colname;
        char       *attname;
-       StringInfoData targetbuf;
-       int         leading_nl_pos = -1;
-       char       *trailing_nl;
-       int         pos;
 
        if (tle->resjunk)
            continue;           /* ignore junk entries */
@@ -3243,11 +3252,10 @@ get_target_list(List *targetList, deparse_context *context,
        colno++;
 
        /*
-        * Put the new field spec into targetbuf so we can decide after we've
+        * Put the new field text into targetbuf so we can decide after we've
         * got it whether or not it needs to go on a new line.
         */
-
-       initStringInfo(&targetbuf);
+       resetStringInfo(&targetbuf);
        context->buf = &targetbuf;
 
        /*
@@ -3288,63 +3296,60 @@ get_target_list(List *targetList, deparse_context *context,
                appendStringInfo(&targetbuf, " AS %s", quote_identifier(colname));
        }
 
-       /* Restore context buffer */
-
+       /* Restore context's output buffer */
        context->buf = buf;
 
-       /* Does the new field start with whitespace plus a new line? */
-
-       for (pos = 0; pos < targetbuf.len; pos++)
+       /* Consider line-wrapping if enabled */
+       if (PRETTY_INDENT(context) && context->wrapColumn >= 0)
        {
-           if (targetbuf.data[pos] == '\n')
+           int         leading_nl_pos = -1;
+           char       *trailing_nl;
+           int         pos;
+
+           /* Does the new field start with whitespace plus a new line? */
+           for (pos = 0; pos < targetbuf.len; pos++)
            {
-               leading_nl_pos = pos;
-               break;
+               if (targetbuf.data[pos] == '\n')
+               {
+                   leading_nl_pos = pos;
+                   break;
+               }
+               if (targetbuf.data[pos] != ' ')
+                   break;
            }
-           if (targetbuf.data[pos] > ' ')
-               break;
-       }
 
-       /* Locate the start of the current  line in the buffer */
-
-       trailing_nl = (strrchr(buf->data, '\n'));
-       if (trailing_nl == NULL)
-           trailing_nl = buf->data;
-       else
-           trailing_nl++;
+           /* Locate the start of the current line in the output buffer */
+           trailing_nl = strrchr(buf->data, '\n');
+           if (trailing_nl == NULL)
+               trailing_nl = buf->data;
+           else
+               trailing_nl++;
 
-       /*
-        * If the field we're adding is the first in the list, or it already
-        * has a leading newline, or wrap mode is disabled (pretty_wrap < 0),
-        * don't add anything. Otherwise, add a newline, plus some
-        * indentation, if either the new field would cause an overflow or the
-        * last field used more than one line.
-        */
+           /*
+            * If the field we're adding is the first in the list, or it
+            * already has a leading newline, don't add anything. Otherwise,
+            * add a newline, plus some indentation, if either the new field
+            * would cause an overflow or the last field used more than one
+            * line.
+            */
+           if (colno > 1 &&
+               leading_nl_pos == -1 &&
+               ((strlen(trailing_nl) + strlen(targetbuf.data) > context->wrapColumn) ||
+                last_was_multiline))
+               appendContextKeyword(context, "", -PRETTYINDENT_STD,
+                                    PRETTYINDENT_STD, PRETTYINDENT_VAR);
 
-       if (colno > 1 &&
-           leading_nl_pos == -1 &&
-           pretty_wrap >= 0 &&
-           ((strlen(trailing_nl) + strlen(targetbuf.data) > pretty_wrap) ||
-            last_was_multiline))
-       {
-           appendContextKeyword(context, "", -PRETTYINDENT_STD,
-                                PRETTYINDENT_STD, PRETTYINDENT_VAR);
+           /* Remember this field's multiline status for next iteration */
+           last_was_multiline =
+               (strchr(targetbuf.data + leading_nl_pos + 1, '\n') != NULL);
        }
 
        /* Add the new field */
-
        appendStringInfoString(buf, targetbuf.data);
-
-
-       /* Keep track of this field's status for next iteration */
-
-       last_was_multiline =
-           (strchr(targetbuf.data + leading_nl_pos + 1, '\n') != NULL);
-
-       /* cleanup */
-
-       pfree(targetbuf.data);
    }
+
+   /* clean up */
+   pfree(targetbuf.data);
 }
 
 static void
@@ -3371,7 +3376,8 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
        if (need_paren)
            appendStringInfoChar(buf, '(');
        get_query_def(subquery, buf, context->namespaces, resultDesc,
-                     context->prettyFlags, context->indentLevel);
+                     context->prettyFlags, context->wrapColumn,
+                     context->indentLevel);
        if (need_paren)
            appendStringInfoChar(buf, ')');
    }
@@ -3787,7 +3793,8 @@ get_insert_query_def(Query *query, deparse_context *context)
    {
        /* Add the SELECT */
        get_query_def(select_rte->subquery, buf, NIL, NULL,
-                     context->prettyFlags, context->indentLevel);
+                     context->prettyFlags, context->wrapColumn,
+                     context->indentLevel);
    }
    else if (values_rte)
    {
@@ -6639,7 +6646,8 @@ get_sublink_expr(SubLink *sublink, deparse_context *context)
        appendStringInfoChar(buf, '(');
 
    get_query_def(query, buf, context->namespaces, NULL,
-                 context->prettyFlags, context->indentLevel);
+                 context->prettyFlags, context->wrapColumn,
+                 context->indentLevel);
 
    if (need_paren)
        appendStringInfo(buf, "))");
@@ -6693,47 +6701,49 @@ get_from_clause(Query *query, const char *prefix, deparse_context *context)
        }
        else
        {
-           StringInfoData targetbuf;
-           char       *trailing_nl;
+           StringInfoData itembuf;
 
            appendStringInfoString(buf, ", ");
 
-           initStringInfo(&targetbuf);
-           context->buf = &targetbuf;
+           /*
+            * Put the new FROM item's text into itembuf so we can decide
+            * after we've got it whether or not it needs to go on a new line.
+            */
+           initStringInfo(&itembuf);
+           context->buf = &itembuf;
 
            get_from_clause_item(jtnode, query, context);
 
+           /* Restore context's output buffer */
            context->buf = buf;
 
-           /* Locate the start of the current  line in the buffer */
-
-           trailing_nl = (strrchr(buf->data, '\n'));
-           if (trailing_nl == NULL)
-               trailing_nl = buf->data;
-           else
-               trailing_nl++;
+           /* Consider line-wrapping if enabled */
+           if (PRETTY_INDENT(context) && context->wrapColumn >= 0)
+           {
+               char       *trailing_nl;
 
-           /*
-            * Add a newline, plus some  indentation, if pretty_wrap is on and
-            * the new from-clause item would cause an overflow.
-            */
+               /* Locate the start of the current line in the buffer */
+               trailing_nl = strrchr(buf->data, '\n');
+               if (trailing_nl == NULL)
+                   trailing_nl = buf->data;
+               else
+                   trailing_nl++;
 
-           if (pretty_wrap >= 0 &&
-               (strlen(trailing_nl) + strlen(targetbuf.data) > pretty_wrap))
-           {
-               appendContextKeyword(context, "", -PRETTYINDENT_STD,
-                                    PRETTYINDENT_STD, PRETTYINDENT_VAR);
+               /*
+                * Add a newline, plus some indentation, if the new item would
+                * cause an overflow.
+                */
+               if (strlen(trailing_nl) + strlen(itembuf.data) > context->wrapColumn)
+                   appendContextKeyword(context, "", -PRETTYINDENT_STD,
+                                        PRETTYINDENT_STD, PRETTYINDENT_VAR);
            }
 
            /* Add the new item */
+           appendStringInfoString(buf, itembuf.data);
 
-           appendStringInfoString(buf, targetbuf.data);
-
-           /* cleanup */
-
-           pfree(targetbuf.data);
+           /* clean up */
+           pfree(itembuf.data);
        }
-
    }
 }
 
@@ -6766,7 +6776,8 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
                /* Subquery RTE */
                appendStringInfoChar(buf, '(');
                get_query_def(rte->subquery, buf, context->namespaces, NULL,
-                             context->prettyFlags, context->indentLevel);
+                             context->prettyFlags, context->wrapColumn,
+                             context->indentLevel);
                appendStringInfoChar(buf, ')');
                break;
            case RTE_FUNCTION: