Re: Doubt about AccessExclusiveLock in ALTER TABLE .. SET ( .. );

From: Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Simon Riggs <simon(at)2ndquadrant(dot)com>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Doubt about AccessExclusiveLock in ALTER TABLE .. SET ( .. );
Date: 2015-07-30 23:30:02
Message-ID: CAFcNs+rFpk4btHZ1eFGac49ZNdkjSFFgcMxKZ8MtqQKEOdVBEA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jul 24, 2015 at 4:05 AM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:
>
> On Fri, Jul 24, 2015 at 7:11 AM, Fabrízio de Royes Mello
> <fabriziomello(at)gmail(dot)com> wrote:
> > On Thu, Jul 2, 2015 at 2:03 PM, Simon Riggs <simon(at)2ndquadrant(dot)com>
wrote:
> >> Looks functionally complete
> >>
> >> Need a test to show that ALTER TABLE works on views, as discussed on
this
> >> thread. And confirmation that pg_dump is not broken by this.
> >>
> >> Message-ID: 20140321205828(dot)GB3969106(at)tornado(dot)leadboat(dot)com
> >>
> >
> > Added more test cases to cover ALTER TABLE on views.
> >
> > I'm thinking about the isolation tests, what about add another
'alter-table'
> > spec for isolation tests enabling and disabling 'autovacuum' options?
>
> Yes, please.
>

Added. I really don't know if my isolation tests are completely correct, is
my first time writing this kind of tests.

> > I did some tests using ALTER TABLE on views and also ALTER VIEW and I
didn't
> > identify any anomalies.
> >
> >> Needs documentation
> >>
> >
> > Added.
>
> for (i = 0; boolRelOpts[i].gen.name; i++)
> + {
> +
> Assert(DoLockModesConflict(boolRelOpts[i].gen.lockmode,
> boolRelOpts[i].gen.lockmode));
> j++;
> + }
> for (i = 0; intRelOpts[i].gen.name; i++)
> + {
> + Assert(DoLockModesConflict(intRelOpts[i].gen.lockmode,
> intRelOpts[i].gen.lockmode));
> j++;
> + }
> for (i = 0; realRelOpts[i].gen.name; i++)
> + {
> +
> Assert(DoLockModesConflict(realRelOpts[i].gen.lockmode,
> realRelOpts[i].gen.lockmode));
> j++;
> + }
> for (i = 0; stringRelOpts[i].gen.name; i++)
> + {
> +
> Assert(DoLockModesConflict(stringRelOpts[i].gen.lockmode,
> stringRelOpts[i].gen.lockmode));
> j++;
> + }
> Splitting those long lines into two will avoid some work for pgindent.
>

Fixed.

> +GetRelOptionsLockLevel(List *defList)
> +{
> + LOCKMODE lockmode = NoLock;
> Shouldn't this default to AccessExclusiveLock instead of NoLock?
>

No, because it will break the logic bellow (get the highest locklevel
according the defList), but I force return an AccessExclusiveLock if
"defList == NIL".

Thanks for reviewing.

Regards,

*** This work is funded by Zenvia Mobile Results (http://www.zenvia.com.br)

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello

Attachment Content-Type Size
alter-table-set-reduce-lock-level-for-autovac-reloptions_v3.patch text/x-diff 156.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2015-07-30 23:54:27 Re: TAP tests are badly named
Previous Message Peter Geoghegan 2015-07-30 23:26:47 Re: Using quicksort and a merge step to significantly improve on tuplesort's single run "external sort"