]> granicus.if.org Git - postgresql/commitdiff
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: