On Mon, Nov 11, 2013 at 4:00 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Thu, Nov 7, 2013 at 12:18 PM, Andres Freund <andres@2ndquadrant.com> wrote:
>> On 2013-11-07 10:10:34 -0500, Tom Lane wrote:
>>> Andres Freund <andres@2ndquadrant.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
