From: | Andres Freund <andres(at)2ndquadrant(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(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:34:58 |
Message-ID: | 0e91fbbc-fe56-42a3-8978-734c48898b3a@email.android.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2013-11-11 21:39:42 | Re: [v9.4] row level security |
Previous 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 |