Re: Spaces before newlines in view definitions in 9.3

Joe Abbate <jma(at)freedomcircle(dot)com> writes:
> View definition:
> SELECT t1.c1,
> t1.c2
> FROM t1;

> It may not be immediately obvious but there is a space after the
> "t1.c1," and before the first newline. In 9.2 and previous releases,
> the view definition is:

> SELECT t1.c1, t1.c2
> FROM t1;

Yeah, this was changed by commit 2f582f76b1945929ff07116cd4639747ce9bb8a1,
which added newlines to the output without making any attempt to suppress
the spaces that had been printed before.

> The problem is that the string comes back, e.g., from pg_get_viewdef()
> with those extra spaces before the newlines, e.g.,

> " SELECT t1.c1, \n t1.c3 * 2 AS mc3\n FROM t1;

> and YAML has a way displaying a text string nicely so that it can be
> recovered when it's read back, but it *doesn't* work if there are
> invisible characters such as tabs or spaces before a newline because
> obviously one can't tell how many or of what kind they are.

Hm. I am not sure whether to consider that a significant complaint or
not, because in reality the ruleutils code has been pretty sloppy about
trailing whitespace ever since it grew any "pretty printing" capability
at all. Looking for trailing whitespace in the ruleutils regression test,
I find it in "UNION ALL" and "INSERT ... VALUES" constructs, which were
that way long before that patch, and there are probably more cases that
don't happen to get exercised by the regression tests. That patch
certainly made things worse, but it's not like pre-9.3 output was
YAML-safe.

Having said that, this seems relatively easy to fix by adjusting
appendContextKeyword to delete any trailing spaces that are immediately
before the newline it inserts. AFAICS that's the only place in ruleutils
that inserts newlines that might follow a space. get_target_list also
needs to do that when transposing a newline-started field value into the
main buffer; but that complexity is offset because it no longer need
consider the possibility of spaces before said newline. I've tested the
attached patch and it seems to do what's wanted (note: I didn't bother
including the regression test output changes in the patch, as they're
bulky and boring).

I think that this is a reasonable thing to fix, but I'm not sure
if we ought to back-patch it into 9.3 or not. The pretty-printing
change evidently broke your use-case but I'm thinking you weren't
pushing it very hard.

Comments?

regards, tom lane