Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit fbbd150

Browse files
committed
Fix ruleutils pretty-printing to not generate trailing whitespace.
The pretty-printing logic in ruleutils.c operates by inserting a newline and some indentation whitespace into strings that are already valid SQL. This naturally results in leaving some trailing whitespace before the newline in many cases; which can be annoying when processing the output with other tools, as complained of by Joe Abbate. We can fix that in a pretty localized fashion by deleting any trailing whitespace before we append a pretty-printing newline. In addition, we have to modify the code inserted by commit 2f582f7 so that we also delete trailing whitespace when transposing items from temporary buffers into the main result string, when a temporary item starts with a newline. This results in rather voluminous changes to the regression test results, but it's easily verified that they are only removal of trailing whitespace. Back-patch to 9.3, because the aforementioned commit resulted in many more cases of trailing whitespace than had occurred in earlier branches.
1 parent 04e6ee4 commit fbbd150

File tree

8 files changed

+990
-959
lines changed

8 files changed

+990
-959
lines changed

src/backend/utils/adt/ruleutils.c

Lines changed: 80 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,7 @@ static const char *get_simple_binary_op_name(OpExpr *expr);
366366
static bool isSimpleNode(Node *node, Node *parentNode, int prettyFlags);
367367
static void appendContextKeyword(deparse_context *context, const char *str,
368368
int indentBefore, int indentAfter, int indentPlus);
369+
static void removeStringInfoSpaces(StringInfo str);
369370
static void get_rule_expr(Node *node, deparse_context *context,
370371
bool showimplicit);
371372
static void get_oper_expr(OpExpr *expr, deparse_context *context);
@@ -4494,42 +4495,42 @@ get_target_list(List *targetList, deparse_context *context,
44944495
/* Consider line-wrapping if enabled */
44954496
if (PRETTY_INDENT(context) && context->wrapColumn >= 0)
44964497
{
4497-
int leading_nl_pos = -1;
4498-
char *trailing_nl;
4499-
int pos;
4498+
int leading_nl_pos;
45004499

4501-
/* Does the new field start with whitespace plus a new line? */
4502-
for (pos = 0; pos < targetbuf.len; pos++)
4500+
/* Does the new field start with a new line? */
4501+
if (targetbuf.len > 0 && targetbuf.data[0] == '\n')
4502+
leading_nl_pos = 0;
4503+
else
4504+
leading_nl_pos = -1;
4505+
4506+
/* If so, we shouldn't add anything */
4507+
if (leading_nl_pos >= 0)
45034508
{
4504-
if (targetbuf.data[pos] == '\n')
4505-
{
4506-
leading_nl_pos = pos;
4507-
break;
4508-
}
4509-
if (targetbuf.data[pos] != ' ')
4510-
break;
4509+
/* instead, remove any trailing spaces currently in buf */
4510+
removeStringInfoSpaces(buf);
45114511
}
4512-
4513-
/* Locate the start of the current line in the output buffer */
4514-
trailing_nl = strrchr(buf->data, '\n');
4515-
if (trailing_nl == NULL)
4516-
trailing_nl = buf->data;
45174512
else
4518-
trailing_nl++;
4513+
{
4514+
char *trailing_nl;
45194515

4520-
/*
4521-
* If the field we're adding is the first in the list, or it
4522-
* already has a leading newline, don't add anything. Otherwise,
4523-
* add a newline, plus some indentation, if either the new field
4524-
* would cause an overflow or the last field used more than one
4525-
* line.
4526-
*/
4527-
if (colno > 1 &&
4528-
leading_nl_pos == -1 &&
4529-
((strlen(trailing_nl) + strlen(targetbuf.data) > context->wrapColumn) ||
4530-
last_was_multiline))
4531-
appendContextKeyword(context, "", -PRETTYINDENT_STD,
4532-
PRETTYINDENT_STD, PRETTYINDENT_VAR);
4516+
/* Locate the start of the current line in the output buffer */
4517+
trailing_nl = strrchr(buf->data, '\n');
4518+
if (trailing_nl == NULL)
4519+
trailing_nl = buf->data;
4520+
else
4521+
trailing_nl++;
4522+
4523+
/*
4524+
* Add a newline, plus some indentation, if the new field is
4525+
* not the first and either the new field would cause an
4526+
* overflow or the last field used more than one line.
4527+
*/
4528+
if (colno > 1 &&
4529+
((strlen(trailing_nl) + targetbuf.len > context->wrapColumn) ||
4530+
last_was_multiline))
4531+
appendContextKeyword(context, "", -PRETTYINDENT_STD,
4532+
PRETTYINDENT_STD, PRETTYINDENT_VAR);
4533+
}
45334534

45344535
/* Remember this field's multiline status for next iteration */
45354536
last_was_multiline =
@@ -6251,23 +6252,42 @@ static void
62516252
appendContextKeyword(deparse_context *context, const char *str,
62526253
int indentBefore, int indentAfter, int indentPlus)
62536254
{
6255+
StringInfo buf = context->buf;
6256+
62546257
if (PRETTY_INDENT(context))
62556258
{
62566259
context->indentLevel += indentBefore;
62576260

6258-
appendStringInfoChar(context->buf, '\n');
6259-
appendStringInfoSpaces(context->buf,
6261+
/* remove any trailing spaces currently in the buffer ... */
6262+
removeStringInfoSpaces(buf);
6263+
/* ... then add a newline and some spaces */
6264+
appendStringInfoChar(buf, '\n');
6265+
appendStringInfoSpaces(buf,
62606266
Max(context->indentLevel, 0) + indentPlus);
6261-
appendStringInfoString(context->buf, str);
6267+
6268+
appendStringInfoString(buf, str);
62626269

62636270
context->indentLevel += indentAfter;
62646271
if (context->indentLevel < 0)
62656272
context->indentLevel = 0;
62666273
}
62676274
else
6268-
appendStringInfoString(context->buf, str);
6275+
appendStringInfoString(buf, str);
62696276
}
62706277

6278+
/*
6279+
* removeStringInfoSpaces - delete trailing spaces from a buffer.
6280+
*
6281+
* Possibly this should move to stringinfo.c at some point.
6282+
*/
6283+
static void
6284+
removeStringInfoSpaces(StringInfo str)
6285+
{
6286+
while (str->len > 0 && str->data[str->len - 1] == ' ')
6287+
str->data[--(str->len)] = '\0';
6288+
}
6289+
6290+
62716291
/*
62726292
* get_rule_expr_paren - deparse expr using get_rule_expr,
62736293
* embracing the string with parentheses if necessary for prettyPrint.
@@ -7929,22 +7949,33 @@ get_from_clause(Query *query, const char *prefix, deparse_context *context)
79297949
/* Consider line-wrapping if enabled */
79307950
if (PRETTY_INDENT(context) && context->wrapColumn >= 0)
79317951
{
7932-
char *trailing_nl;
7933-
7934-
/* Locate the start of the current line in the buffer */
7935-
trailing_nl = strrchr(buf->data, '\n');
7936-
if (trailing_nl == NULL)
7937-
trailing_nl = buf->data;
7952+
/* Does the new item start with a new line? */
7953+
if (itembuf.len > 0 && itembuf.data[0] == '\n')
7954+
{
7955+
/* If so, we shouldn't add anything */
7956+
/* instead, remove any trailing spaces currently in buf */
7957+
removeStringInfoSpaces(buf);
7958+
}
79387959
else
7939-
trailing_nl++;
7960+
{
7961+
char *trailing_nl;
79407962

7941-
/*
7942-
* Add a newline, plus some indentation, if the new item would
7943-
* cause an overflow.
7944-
*/
7945-
if (strlen(trailing_nl) + strlen(itembuf.data) > context->wrapColumn)
7946-
appendContextKeyword(context, "", -PRETTYINDENT_STD,
7947-
PRETTYINDENT_STD, PRETTYINDENT_VAR);
7963+
/* Locate the start of the current line in the buffer */
7964+
trailing_nl = strrchr(buf->data, '\n');
7965+
if (trailing_nl == NULL)
7966+
trailing_nl = buf->data;
7967+
else
7968+
trailing_nl++;
7969+
7970+
/*
7971+
* Add a newline, plus some indentation, if the new item
7972+
* would cause an overflow.
7973+
*/
7974+
if (strlen(trailing_nl) + itembuf.len > context->wrapColumn)
7975+
appendContextKeyword(context, "", -PRETTYINDENT_STD,
7976+
PRETTYINDENT_STD,
7977+
PRETTYINDENT_VAR);
7978+
}
79487979
}
79497980

79507981
/* Add the new item */

src/test/regress/expected/aggregates.out

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -960,10 +960,10 @@ select * from agg_view1;
960960
(1 row)
961961

962962
select pg_get_viewdef('agg_view1'::regclass);
963-
pg_get_viewdef
964-
----------------------------------------------------------------------------------------------------------------------
965-
SELECT aggfns(DISTINCT v.a, v.b, v.c) AS aggfns +
966-
FROM ( VALUES (1,3,'foo'::text), (0,NULL::integer,NULL::text), (2,2,'bar'::text), (3,1,'baz'::text)) v(a, b, c), +
963+
pg_get_viewdef
964+
---------------------------------------------------------------------------------------------------------------------
965+
SELECT aggfns(DISTINCT v.a, v.b, v.c) AS aggfns +
966+
FROM ( VALUES (1,3,'foo'::text), (0,NULL::integer,NULL::text), (2,2,'bar'::text), (3,1,'baz'::text)) v(a, b, c),+
967967
generate_series(1, 3) i(i);
968968
(1 row)
969969

@@ -978,10 +978,10 @@ select * from agg_view1;
978978
(1 row)
979979

980980
select pg_get_viewdef('agg_view1'::regclass);
981-
pg_get_viewdef
982-
----------------------------------------------------------------------------------------------------------------------
983-
SELECT aggfns(DISTINCT v.a, v.b, v.c ORDER BY v.b) AS aggfns +
984-
FROM ( VALUES (1,3,'foo'::text), (0,NULL::integer,NULL::text), (2,2,'bar'::text), (3,1,'baz'::text)) v(a, b, c), +
981+
pg_get_viewdef
982+
---------------------------------------------------------------------------------------------------------------------
983+
SELECT aggfns(DISTINCT v.a, v.b, v.c ORDER BY v.b) AS aggfns +
984+
FROM ( VALUES (1,3,'foo'::text), (0,NULL::integer,NULL::text), (2,2,'bar'::text), (3,1,'baz'::text)) v(a, b, c),+
985985
generate_series(1, 3) i(i);
986986
(1 row)
987987

@@ -1044,10 +1044,10 @@ select * from agg_view1;
10441044
(1 row)
10451045

10461046
select pg_get_viewdef('agg_view1'::regclass);
1047-
pg_get_viewdef
1048-
----------------------------------------------------------------------------------------------------------------------
1049-
SELECT aggfns(DISTINCT v.a, v.b, v.c ORDER BY v.a, v.c USING ~<~ NULLS LAST, v.b) AS aggfns +
1050-
FROM ( VALUES (1,3,'foo'::text), (0,NULL::integer,NULL::text), (2,2,'bar'::text), (3,1,'baz'::text)) v(a, b, c), +
1047+
pg_get_viewdef
1048+
---------------------------------------------------------------------------------------------------------------------
1049+
SELECT aggfns(DISTINCT v.a, v.b, v.c ORDER BY v.a, v.c USING ~<~ NULLS LAST, v.b) AS aggfns +
1050+
FROM ( VALUES (1,3,'foo'::text), (0,NULL::integer,NULL::text), (2,2,'bar'::text), (3,1,'baz'::text)) v(a, b, c),+
10511051
generate_series(1, 2) i(i);
10521052
(1 row)
10531053

0 commit comments

Comments
 (0)