Re: RULE regression test fragility?

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Mike Blackwell <mike(dot)blackwell(at)rrd(dot)com>, PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: RULE regression test fragility?
Date: 2013-10-26 17:07:09
Message-ID: 20131026170709.GB5279@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2013-10-26 12:25:40 -0400, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2013-10-26 19:06:45 Re: proposal: lob conversion functionality
Previous Message Tom Lane 2013-10-26 16:25:40 Re: RULE regression test fragility?