Re: pg_dump --pretty-print-views

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Greg Stark <stark(at)mit(dot)edu>
Cc: Keith Fiske <keith(at)omniti(dot)com>, Marko Tiikkaja <pgmail(at)joh(dot)to>, Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_dump --pretty-print-views
Date: 2014-04-29 23:44:33
Message-ID: 4508.1398815073@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> While I was testing this I noticed that there's something thoroughly
> busted about the indentation of outer JOIN constructs, too --- you
> can see this in some of the regression test queries that are touched
> by this patch, where the JOINs are actually outdented to the left of
> their FROM clause in the unpatched output. (They'd be outdented in
> the new output, too, if negative indentation were possible...)
> That seems like material for a separate patch though.

I poked into that too, and found that it seems to have been busted from
the very beginning. There's a unmatched subtraction of
PRETTYINDENT_JOIN_ON from the indentLevel, which I suppose must be an
accidental leftover from some previous version of the logic. And the code
is also trying to outdent JOIN clauses further than the corresponding
FROM, which seems unintuitive to me, even when the FROM is indented far
enough to make it possible (which it generally isn't in simple views).

Attached are two separate versions of a repair patch. The first one
causes JOIN clauses to be indented the same as their parent FROM.
The second one moves them over an additional 5 spaces. I find the
second layout more sensible, but it does result in significantly
more changes in the regression test outputs, as you can see.

Comments?

regards, tom lane

Attachment Content-Type Size
join-indentation-fix-1.patch text/x-diff 27.4 KB
join-indentation-fix-2.patch text/x-diff 75.8 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Greg Stark 2014-04-29 23:49:24 Re: pg_get_viewdefs() indentation considered harmful
Previous Message Tom Lane 2014-04-29 22:27:53 Re: Fix initdb for path with whitespace and at char