Re: alter_table regression test problem

Robert Haas <robertmhaas(at)gmail(dot)com> schrieb:
>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.

It surely is possible to go that route, but imagine what happens if the heap_open() blows away the entire hash. We'd either need to recheck if the hash exists before entering or recreate it after dropping. It seemed simpler to follow attoptcache's example.

Regards,

Andres

--
Please excuse brevity and formatting - I am writing this on my mobile phone.

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