Re: alter_table regression test problem

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Kevin Grittner <kgrittn(at)ymail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: alter_table regression test problem
Date: 2013-11-11 21:09:55
Message-ID: CA+Tgmob_KA1JA02g2sujeBAzcd9nBE2B3uKKur4KotqKL_QmiQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Nov 11, 2013 at 4:00 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Thu, Nov 7, 2013 at 12:18 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> On 2013-11-07 10:10:34 -0500, Tom Lane wrote:
>>> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
>>> > On 2013-11-07 06:49:58 -0800, Kevin Grittner wrote:
>>> >> It's up to the committer whether to indent
>>> >> after review and make both substantive and whitespace changes
>>> >> together, but I have definitely seen those done separately, or even
>>> >> left for the next global pgindent run to fix.
>>>
>>> > Hm. I don't remember many such cases and I've just looked across the git
>>> > history and i didn't really find anything after
>>> > a04161f2eab55f72b3c3dba9baed0ec09e7f633f, but that was back when
>>> > individuals couldn't run pgindent because of the typedefs file.
>>>
>>> FWIW, I tend to pgindent before committing, now that it's easy to do so.
>>> But in cases like this, I'd much rather review the patch *before* that
>>> happens. Basically the point of the review is to follow and confirm
>>> the patch author's thought process, and I'll bet you put the braces in
>>> before reindenting too.
>>
>> Well, I did both at the same time, I have an emacs command for that
>> ;). But independent from that technicality, reindenting is part of
>> changing the flow of logic for me - I've spent a couple of years
>> primarily writing python (where indentation is significant), maybe it's
>> that.
>>
>> So, here's the patch (slightly editorialized) with reverted indenting of
>> that block.
>
> Gah. Well, of course, I have the opposite preference from Tom on how
> this should be indented. Sigh.

...and I hit send too soon.

I'm pretty sure that the current coding, which blows away the whole
relation, is used in other places, and I really don't see why it
should be fundamentally flawed, or why we should change it to clear
the cache entries out one by one instead of en masse.
RelidByRelfilenode definitely needs to use HASH_FIND rather than
HASH_ENTER, so that part I agree with.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2013-11-11 21:19:17 Re: Re: [BUGS] BUG #7873: pg_restore --clean tries to drop tables that don't exist
Previous Message Robert Haas 2013-11-11 21:00:59 Re: alter_table regression test problem