On 2013-10-26 12:25:40 -0400, Tom Lane wrote:
> Andres Freund <and...@2ndquadrant.com> writes:
> > On 2013-10-26 11:27:19 -0400, Tom Lane wrote:
> >>> +1 (but what are those silly parens in pg_seclabels definition?),
> >> 
> >> That looks weird to me too, but it's surely not the fault of this patch.
> >> Maybe we should take a look at exactly what ruleutils is doing there.
> 
> > Imo what it does looks sane - it adds parentheses whenever a child of a
> > set operation is a set operation again to make sure the order in which
> > the generated set operations are parsed/interpreted stays the same.
> 
> I'm not objecting to the parens being there, but I think the layout
> doesn't look nice.

Ah, ok. Agreed.

> Not immediately sure what would look better though.
> Obvious alternatives include one line per paren:
> 
>       (
>        (
>         (
>          SELECT ...
> 
> or getting rid of the space between parens:
> 
>       (((SELECT ...
> 
> but I'm not sure I'm thrilled with either of those.  Thoughts?

ISTM indentation generally is currently so random up that all other
considerations don't play much of a role:

DROP VIEW deparse;
CREATE VIEW deparse AS SELECT 1 UNION ALL SELECT 2 UNION ALL SELECT 3;
\d+ deparse
View definition:
        (         SELECT 1
        UNION ALL 
                 SELECT 2)
UNION ALL 
         SELECT 3;

or even more extreme:

CREATE VIEW deparse AS SELECT 1 FROM pg_class JOIN pg_attribute ON (attrelid = 
pg_class.oid) WHERE true UNION ALL SELECT 2 FROM pg_class WHERE true UNION ALL 
SELECT 3 FROM pg_class WHERE true UNION (SELECT 4 FROM pg_class WHERE true 
ORDER BY 1 LIMIT 1) ORDER BY 1 LIMIT 10;
View definition:
        (        (         SELECT 1
                           FROM pg_class
                      JOIN pg_attribute ON pg_attribute.attrelid = pg_class.oid
                     WHERE true
                UNION ALL 
                         SELECT 2
                           FROM pg_class
                          WHERE true)
        UNION ALL 
                 SELECT 3
                   FROM pg_class
                  WHERE true)
UNION 
        ( SELECT 4
           FROM pg_class
          WHERE true
          ORDER BY 4::integer
         LIMIT 1)
  ORDER BY 1
 LIMIT 10;

I don't see any consistency in that...

WRT to the parentheses around SetOps, I think they should be on their own
line. Otherwise the SELECT can wander so far right that it's hard to
correlate it to the FROM on the next line...

Greetings,

Andres Freund

-- 
 Andres Freund                     http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to