Spaces before newlines in view definitions in 9.3
Example test code:
$ psql pyrseas_testdb
psql (9.3.0)
Type "help" for help.
pyrseas_testdb=# create table t1 (c1 int, c2 text);
CREATE TABLE
pyrseas_testdb=# create view v1 as select * from t1;
CREATE VIEW
pyrseas_testdb=# \d+ v1
View "public.v1"
Column | Type | Modifiers | Storage | Description
--------+---------+-----------+----------+-------------
c1 | integer | | plain |
c2 | text | | extended |
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;
If there are more columns, there's an extra space for each except the
last one, e.g., (with _ denoting a trailing space):
SELECT t2.c1,_
t2.c2,_
t2.c3,_
t2.c4
FROM t2;
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.
Note: This applies to both views and materialized views.
I believe the reformatting of view text (breaking each column on a
separate line) was done to improve readability but it has the side
effect of making the text unreadable if processed via a YAML utility
such as Pyrseas dbtoyaml (since YAML then quotes the entire string and
even breaks it down further with extra backslashes).
Regards,
Joe
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Joe Abbate <jma@freedomcircle.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
Attachments:
ruleutils-no-trailing-spaces.patchtext/x-diff; charset=us-ascii; name=ruleutils-no-trailing-spaces.patchDownload+73-76
Hello Tom,
On 08/11/13 17:24, Tom Lane wrote:
Joe Abbate <jma@freedomcircle.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.
Agreed it's not a significant complaint, just an extra annoyance --made
more visible because for Pyrseas 0.7 we added nicer view text formatting
rather than outputting what pg_get_viewdef gave us, no matter how ugly
and long the resulting quoted string looked like.
Our tests generally compare SQL texts in a "forgiving" manner,
particularly when it comes to whitespace. However, the problem was
introduced in 9.3 and broke some of our view tests (and analogous
matview tests derived from them). It's not a big deal if you don't
backpatch to 9.3, but it would help, since some of our tests now read:
if ("postgres version" < 90300):
fmt = "(%s%s %s%s )"
else:
fmt = "( %s\n %s\n %s\n %s\n)"
That would probably require yet another branch for 9.4 and later.
Best regards,
Joe
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Joe Abbate <jma@freedomcircle.com> writes:
On 08/11/13 17:24, Tom Lane wrote:
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.
Agreed it's not a significant complaint, just an extra annoyance --made
more visible because for Pyrseas 0.7 we added nicer view text formatting
rather than outputting what pg_get_viewdef gave us, no matter how ugly
and long the resulting quoted string looked like.
Not hearing any objections, I went ahead and pushed this fix into
9.3 as well as HEAD.
regards, tom lane
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs