From: Simon Riggs on
On Thu, 2010-07-08 at 07:16 +0100, Simon Riggs wrote:

> I'll take my previous patch through to completion now

Patch to reduce lock levels for
ALTER TABLE
CREATE TRIGGER
CREATE RULE

I've completely re-analyzed the required lock levels for sub-commands,
so lock levels can now also be these, if appropriate.
ShareUpdateExclusiveLock - allows db reads and writes
ShareRowExclusiveLock - allows db reads only

When ALTER TABLE is specified with multiple subcommands the highest lock
level required by any subcommand is applied to the whole combined
command.

The lock levels are in many ways different from both my own earlier
patch and much of the discussion on this thread, which I have taken to
be general comments rather than considered thought.

Nothing much speculative here, so will commit in a few days barring
objections.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Training and Services
From: Simon Riggs on
On Fri, 2010-07-16 at 20:41 +0200, Andres Freund wrote:

> You argue above that you cant change SET [NOT] NULL to be less
> restrictive because it might change plans - isnt that true for some of the
> above cases as well?
>
> For example UNIQUE/PRIMARY might make join removal possible - which could
> only be valid after "invalid" tuples where deleted earlier in that
> transaction. Another case which it influences are grouping plans...

This is only for adding a constraint, not removing it. Join removal
would be possible after the ALTER finishes, but won't change plans
already in progress. The idea is to minimise the impact, not maximise
the benefit of the newly added constraint; I don't think we should block
all queries just because a few might benefit.

> So I think the only case where its actually possible to lower the
> level is CONSTR_EXCLUSION and _FOREIGN.
> The latter might get impossible soon by planner improvements (Peter's
> functional dependencies patch for example).

Same, I don't see why it would block queries.


--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Training and Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Simon Riggs on
On Fri, 2010-07-16 at 21:10 +0200, Andres Freund wrote:
> On Friday 16 July 2010 20:41:44 Andres Freund wrote:
> > >> ! */
> > >> ! case AT_AddColumn: /* may
> > >> rewrite heap, in some cases and visible to SELECT */ !
> > >> case AT_DropColumn: /* change
> > >> visible to SELECT */ ! case
> > >> AT_AddColumnToView: /* CREATE VIEW */ ! case
> > >> AT_AlterColumnType: /* must rewrite heap */ !
> > >> case AT_DropConstraint: /* as DROP INDEX */
> > >> ! case AT_AddOids:
> > >> ! case AT_DropOids: /* calls
> > >> AT_DropColumn */ ! case
> > >> AT_EnableAlwaysRule: /* as DROP INDEX */ !
> > >> case AT_EnableReplicaRule: /* as DROP INDEX */
> > >> ! case AT_EnableRule: /* as DROP
> > >> INDEX */
> Another remark:
>
> Imho it would be usefull to keep that list in same order as in the enum -
> currently its hard to make sure no case is missing.

Not really; the default case is to reject, so any full test suite will
pick that up.

The cases are ordered by resulting lock type, which seemed the best way
to check we didn't accidentally assign an incorrect lock type.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Training and Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Simon Riggs on
On Fri, 2010-07-16 at 21:38 +0200, Andres Freund wrote:
> boom

Your test case would still occur in the case where the first query had
not been executed against the same table. So the test case illustrates a
failing of join removal, not of this patch.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Training and Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Simon Riggs on
On Fri, 2010-07-16 at 23:03 +0200, Andres Freund wrote:

> Sure its not that bad, but at least it needs to get documented imho.
> Likely others should chime in here ;-)

Don't understand you. This is a clear bug in join removal, test case
attached, a minor rework of your original test case.

> What could the join removal path (and similar places) *possibly* do
> against such a case? Without stopping to use SnapshotNow I dont see
> any way :-(

The bug is caused by allowing join removal to work in serializable
transactions. The fix for 9.0 is easy and clear: disallow join removal
when planning a query as the second or subsequent query in a
serializable transaction.

A wider fix might be worth doing for 9.1, not sure.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Training and Services