Fix some minor issues in view pretty-printing.

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.

1 files changed, 113 insertions, 102 deletions

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index b7aff1189b2..f58418e0ceb 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -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: