LOCK for non-tables

On Fri, Jan 07, 2011 at 08:16:33AM -0500, Robert Haas wrote:

One of the things that I ripped out of the SQL/MED syntax patch before
committing it was the hack that made LOCK TABLE also work on FOREIGN
TABLEs. Since we're treating a foreign table as a different kind of
object than a TABLE in some places, we shouldn't confuse the two
things elsewhere, at least in my opinion. I also noticed that pg_dump
has the following comment:

* NOTE: it'd be kinda nice to lock other relations
too, not only
* plain tables, but the backend doesn't presently allow that.

This is pretty trivial from a backend point of view, especially with
the new objectaddress.c machinery. In a comparatively small amount of
code we could support locks not only on all types of relations but
also on any other backend objects we think it's worth supporting. The
only difficulty is the grammar: we allow either "LOCK relname" or
"LOCK TABLE relname", so adding e.g. "LOCK SEQUENCE relname" creates a
shift/reduce conflict because SEQUENCE is unreserved. We can easily
make "LOCK FOREIGN TABLE relname" work because FOREIGN is already
full-reserved, but that seems to be the only case that can be done
relatively painlessly.

So, the options as I see them are:

1. Do nothing.
2. Support LOCK FOREIGN TABLE relname and forget about the rest.
Feels fairly arbitrary, but avoids any hard decisions.
3. Partially reserve keywords like VIEW and SEQUENCE, and support LOCK
[ TABLE | VIEW | SEQUENCE | FOREIGN TABLE ] relname. Doesn't really
scale to other object types unless you keep reserving more keywords,
but maybe we don't care.
4. Make the keyword TABLE required, and support LOCK { TABLE | VIEW |
SEQUENCE | FOREIGN TABLE | maybe other object types } relname.

I'm not sure I understand this. Does it mean I'd have to say

LOCK TABLE my_view;

? If so, I don't think that's a great idea. We used to have to do
TABLE operations on SEQUENCEs because they just happened to be
implemented as special tables, which wired implementation details into
the API. This is Generally Not A Good Thing™, and we removed that
some time back.

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

On Fri, Jan 7, 2011 at 10:52 AM, David Fetter <david@fetter.org> wrote:

I'm not sure I understand this.  Does it mean I'd have to say

   LOCK TABLE my_view;

No.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Jan 07, 2011 at 10:58:41AM -0500, Robert Haas wrote:

On Fri, Jan 7, 2011 at 10:52 AM, David Fetter <david@fetter.org> wrote:

I'm not sure I understand this. �Does it mean I'd have to say

� �LOCK TABLE my_view;

No.

+1 for #4, then :)

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

On Fri, 2011-01-07 at 08:16 -0500, Robert Haas wrote:

One of the things that I ripped out of the SQL/MED syntax patch before
committing it was the hack that made LOCK TABLE also work on FOREIGN
TABLEs. Since we're treating a foreign table as a different kind of
object than a TABLE in some places, we shouldn't confuse the two
things elsewhere, at least in my opinion. I also noticed that pg_dump
has the following comment:

* NOTE: it'd be kinda nice to lock other relations
too, not only
* plain tables, but the backend doesn't presently
allow that.

This is pretty trivial from a backend point of view, especially with
the new objectaddress.c machinery.

I'm not clear why we'd want to do that. We shouldn't just be adding
things because we can do them easily, we should be adding things with a
clear use case or a standardization requirement.

If anyone suggested tuning some aspect of the code, yet offered no
evidence that it was ever important, it would get shot down. Why is this
any different?

Allowing LOCK on views would significantly undermine admin structures
where the only access to a table is via a view. This would allow people
to lock objects they didn't previously have access to and seems likely
to introduce more contention into applications as a result.

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

On Fri, Jan 7, 2011 at 12:17 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

On Fri, 2011-01-07 at 08:16 -0500, Robert Haas wrote:

One of the things that I ripped out of the SQL/MED syntax patch before
committing it was the hack that made LOCK TABLE also work on FOREIGN
TABLEs.  Since we're treating a foreign table as a different kind of
object than a TABLE in some places, we shouldn't confuse the two
things elsewhere, at least in my opinion.  I also noticed that pg_dump
has the following comment:

              * NOTE: it'd be kinda nice to lock other relations
too, not only
                 * plain tables, but the backend doesn't presently
allow that.

This is pretty trivial from a backend point of view, especially with
the new objectaddress.c machinery.

I'm not clear why we'd want to do that. We shouldn't just be adding
things because we can do them easily, we should be adding things with a
clear use case or a standardization requirement.

Good point.

The reason why the pg_dump comment suggests that this feature would be
useful is that it would allow locking relations, and possibly other
objects, against concurrent drops. pg_dump can offer a
mostly-consistent view of the contents of every table in the system by
taking a snapshot at the beginning of operation and using that same
snapshot to completion. But that won't necessarily give it a
consistent view of the system catalogs, because those generally follow
SnapshotNow rules. To be fully consistent, it needs to take
AccessShareLocks on any schema objects it looks at. Currently, it can
do that for tables, but not other object types.

So suppose you pg_dump a view and and a function that uses the view.
In the middle of the dump, someone alters the view and the function in
a single transaction and commits it. You might dump the function
before the transaction commits and the view afterward, or visca versa,
and the result will be an inconsistent view of the database schema.
Allowing pg_dump to take AccessShareLocks on the objects in question
would prevent this sort of anomaly, which certainly seems to have some
value.

But if it doesn't have *enough* value, then we can go with the first
option I listed: do nothing.

If anyone suggested tuning some aspect of the code, yet offered no
evidence that it was ever important, it would get shot down. Why is this
any different?

It's not.

Allowing LOCK on views would significantly undermine admin structures
where the only access to a table is via a view. This would allow people
to lock objects they didn't previously have access to and seems likely
to introduce more contention into applications as a result.

As long as we include appropriate privilege checks, this doesn't seem
like a major issue to me.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Jan7, 2011, at 22:21 , Robert Haas wrote:

So suppose you pg_dump a view and and a function that uses the view.
In the middle of the dump, someone alters the view and the function in
a single transaction and commits it. You might dump the function
before the transaction commits and the view afterward, or visca versa,
and the result will be an inconsistent view of the database schema.
Allowing pg_dump to take AccessShareLocks on the objects in question
would prevent this sort of anomaly, which certainly seems to have some
value.

That'd actually work fine I believe. AFAICT, system catalogs are *not*
accessed with SnapshotNow semantics if accessed via SQL, they're treated
like any other table in that case. The following confirms this

T1> BEGIN TRANSACTION ISOLATION SERIALIZABLE;
T1> SELECT TRUE; -- T1's snapshot is now set
T2> CREATE TABLE test(id int);
T1> SELECT * FROM test; -- Succeeds
T1> SELECT * FROM pg_class WHERE relname = 'test'; -- Returns 0 rows

Thus, all objects which are dumped purely by SQL-level inspection of the
system catalogs are safe I think. This is true for most objects I guess,
with the important exception being dumping a table's contents (but not
dumping its structure!). The lock makes sure that the structure we see
when inspecting the catalogs is also what "SELECT * FROM table" will return.
I dunno if there are any other objects like that, though - but if there
are, they could probably use a lock too.

Another class of failure cases can be constructed from output functions
which access the catalog. For example,

CREATE TABLE my_types (a_type regtype);
CREATE TYPE my_type AS (id int);
INSERT INTO my_types VALUES ('my_type');

T1> BEGIN TRANSACTION ISOLATION SERIALIZABLE;
T1> SELECT TRUE; -- T1's snapshot is now set
T1> SELECT * FROM my_types;
a_type
---------
my_type

T2> BEGIN;
T2> DELETE FROM my_types WHERE a_type = 'my_type';
T2> DROP TYPE my_type;
T2> COMMIT;
T1> SELECT * FROM my_types;
a_type
--------
291919

best regards.
Florian Pflug

On Fri, Jan 7, 2011 at 5:17 PM, Florian Pflug <fgp@phlo.org> wrote:

On Jan7, 2011, at 22:21 , Robert Haas wrote:

So suppose you pg_dump a view and and a function that uses the view.
In the middle of the dump, someone alters the view and the function in
a single transaction and commits it.  You might dump the function
before the transaction commits and the view afterward, or visca versa,
and the result will be an inconsistent view of the database schema.
Allowing pg_dump to take AccessShareLocks on the objects in question
would prevent this sort of anomaly, which certainly seems to have some
value.

That'd actually work fine I believe. AFAICT, system catalogs are *not*
accessed with SnapshotNow semantics if accessed via SQL, they're treated
like any other table in that case.

Oh, hm. Interesting point.

Thus, all objects which are dumped purely by SQL-level inspection of the
system catalogs are safe I think. This is true for most objects I guess,
with the important exception being dumping a table's contents (but not
dumping its structure!). The lock makes sure that the structure we see
when inspecting the catalogs is also what "SELECT * FROM table" will return.
I dunno if there are any other objects like that, though - but if there
are, they could probably use a lock too.

Hmm. It would seem that to be vulnerable you'd need an object where
you need to dump both the structure and the contents, and I can't
think of any. Or it could apply to an object where you called some
pg_foo-ish function that did some SnapshotNow magic behind the scenes;
not sure if there are any of those, and this might not be the right
fix anyway.

Another class of failure cases can be constructed from output functions
which access the catalog. For example,

CREATE TABLE my_types (a_type regtype);
CREATE TYPE my_type AS (id int);
INSERT INTO my_types VALUES ('my_type');

T1> BEGIN TRANSACTION ISOLATION SERIALIZABLE;
T1> SELECT TRUE; -- T1's snapshot is now set
T1> SELECT * FROM my_types;
 a_type
---------
 my_type

T2> BEGIN;
T2> DELETE FROM my_types WHERE a_type = 'my_type';
T2> DROP TYPE my_type;
T2> COMMIT;
T1> SELECT * FROM my_types;
 a_type
--------
 291919

Well, that happens even if there's no concurrency involved.

rhaas=# create table my_types (a_type regtype);
CREATE TABLE
rhaas=# create type my_type as (id int);
CREATE TYPE
rhaas=# insert into my_types values ('my_type');
INSERT 0 1
rhaas=# select * from my_types;
a_type
---------
my_type
(1 row)

rhaas=# drop type my_type;
DROP TYPE
rhaas=# select * from my_types;
a_type
--------
16474
(1 row)

If the conclusion of this discussion is that pg_dump doesn't really
need to lock anything other than tables, we should update the comments
to say that, rather than what they say now.

It's also worth thinking about whether there's any use case for
locking other types of objects for any reason *other than* pg_dump. I
can't immediately think of anything terribly compelling, but I might
be missing something.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Jan7, 2011, at 23:56 , Robert Haas wrote:

On Fri, Jan 7, 2011 at 5:17 PM, Florian Pflug <fgp@phlo.org> wrote:

Thus, all objects which are dumped purely by SQL-level inspection of the
system catalogs are safe I think. This is true for most objects I guess,
with the important exception being dumping a table's contents (but not
dumping its structure!). The lock makes sure that the structure we see
when inspecting the catalogs is also what "SELECT * FROM table" will return.
I dunno if there are any other objects like that, though - but if there
are, they could probably use a lock too.

Hmm. It would seem that to be vulnerable you'd need an object where
you need to dump both the structure and the contents, and I can't
think of any. Or it could apply to an object where you called some
pg_foo-ish function that did some SnapshotNow magic behind the scenes;
not sure if there are any of those, and this might not be the right
fix anyway.

I forgot about sequences earlier. If we dump while someone deletes all
rows and resets the sequence the dump might contain rows and still
reset the sequence. This could cause duplicate key errors on restore.
I haven't checked if this is really possible though - I guess it would
depend on the exact order of these events...

Another class of failure cases can be constructed from output functions
which access the catalog. For example,

<snipped>

T2> BEGIN;
T2> DELETE FROM my_types WHERE a_type = 'my_type';
T2> DROP TYPE my_type;
T2> COMMIT;
T1> SELECT * FROM my_types;
a_type
--------
291919

Well, that happens even if there's no concurrency involved.

Not really, because the DROP TYPE and DELETE FROM my_types is
done within a transaction, so one might expect nobody else to
see the intermediate state. But yeah, I agree, this corner-case isn't
something we have to worry about too much.

If the conclusion of this discussion is that pg_dump doesn't really
need to lock anything other than tables, we should update the comments
to say that, rather than what they say now.

+1

It's also worth thinking about whether there's any use case for
locking other types of objects for any reason *other than* pg_dump. I
can't immediately think of anything terribly compelling, but I might
be missing something.

I wonder how such locks would work. Would such locks prevent accessing
these objects? Or just modifications? For example, if I locked a function,
could someone else execute it while I held the lock?

best regards,
Florian Pflug

On Fri, Jan 7, 2011 at 6:28 PM, Florian Pflug <fgp@phlo.org> wrote:

I forgot about sequences earlier. If we dump while someone deletes all
rows and resets the sequence the dump might contain rows and still
reset the sequence. This could cause duplicate key errors on restore.
I haven't checked if this is really possible though - I guess it would
depend on the exact order of these events...

To fix this, you'd need a lock that allowed getting values from the
sequence but prevented resetting it, and...

I wonder how such locks would work. Would such locks prevent accessing
these objects? Or just modifications? For example, if I locked a function,
could someone else execute it while I held the lock?

...in fact we do very little locking of objects other than tables.
DROP takes an AccessExclusiveLock on whatever it's dropping, and
COMMENT and SECURITY LABEL take ShareUpdateExclusiveLocks to avoid
orphaning pg_{sh,}description or pg_seclabel entries in the face of a
concurrent drop, but most operations on non-table objects don't AFAIK
take any lock at all. We probably don't want to make too many changes
in this area, because there are already workloads where the
heavyweight lock manager can become a bottleneck, and one can easily
imagine that locking operators or namespaces could make that problem
much worse.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Jan 7, 2011 at 6:28 PM, Florian Pflug <fgp@phlo.org> wrote:

I forgot about sequences earlier. If we dump while someone deletes all
rows and resets the sequence the dump might contain rows and still
reset the sequence. This could cause duplicate key errors on restore.
I haven't checked if this is really possible though - I guess it would
depend on the exact order of these events...

To fix this, you'd need a lock that allowed getting values from the
sequence but prevented resetting it, and...

I wonder how such locks would work. Would such locks prevent accessing
these objects? Or just modifications? For example, if I locked a function,
could someone else execute it while I held the lock?

...in fact we do very little locking of objects other than tables.
DROP takes an AccessExclusiveLock on whatever it's dropping, and
COMMENT and SECURITY LABEL take ShareUpdateExclusiveLocks to avoid
orphaning pg_{sh,}description or pg_seclabel entries in the face of a
concurrent drop, but most operations on non-table objects don't AFAIK
take any lock at all. We probably don't want to make too many changes
in this area, because there are already workloads where the
heavyweight lock manager can become a bottleneck, and one can easily
imagine that locking operators or namespaces could make that problem
much worse.

For query based replication tools like pgpool-II (I don't know any
other tools, for example Postgres XC falls in this category or
not...), we need to be able to lock sequences. Fortunately it is allowed to:

SELECT 1 FROM foo_sequece FOR UPDATE;

but LOCK foo_sequence looks more appropreate syntax for me.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp

At present, XC does not seem to need locks to maintain cluster-wide
integrity because it is maintained centrally in GTM. If application
needs to do this, for example, to synchronize between different
clusters, XC needs this capability obviously.
----------
Koichi Suzuki

2011/1/11 Tatsuo Ishii <ishii@postgresql.org>:

On Tue, Jan 11, 2011 at 4:46 AM, Tatsuo Ishii <ishii@postgresql.org> wrote:

On Fri, Jan 7, 2011 at 6:28 PM, Florian Pflug <fgp@phlo.org> wrote:

I forgot about sequences earlier. If we dump while someone deletes all
rows and resets the sequence the dump might contain rows and still
reset the sequence. This could cause duplicate key errors on restore.
I haven't checked if this is really possible though - I guess it would
depend on the exact order of these events...

To fix this, you'd need a lock that allowed getting values from the
sequence but prevented resetting it, and...

I wonder how such locks would work. Would such locks prevent accessing
these objects? Or just modifications? For example, if I locked a function,
could someone else execute it while I held the lock?

...in fact we do very little locking of objects other than tables.
DROP takes an AccessExclusiveLock on whatever it's dropping, and
COMMENT and SECURITY LABEL take ShareUpdateExclusiveLocks to avoid
orphaning pg_{sh,}description or pg_seclabel entries in the face of a
concurrent drop, but most operations on non-table objects don't AFAIK
take any lock at all.  We probably don't want to make too many changes
in this area, because there are already workloads where the
heavyweight lock manager can become a bottleneck, and one can easily
imagine that locking operators or namespaces could make that problem
much worse.

For query based replication tools like pgpool-II (I don't know any
other tools, for example Postgres XC falls in this category or
not...), we need to be able to lock sequences. Fortunately it is allowed to:

SELECT 1 FROM foo_sequece FOR UPDATE;

but LOCK foo_sequence looks more appropreate syntax for me.

Those aren't doing the same thing. The first is locking the one and
only tuple that is contained within the sequence, while the second is
locking the sequence object itself.

At this point, I'm inclined to think that the pg_dump comment is just
wrong, and we ought to fix it to say that we don't really want to be
able to lock other relations after all, and call it good.

As a side node, locking a sequence for replication seems like it could
have pretty dire effects on performance in certain workloads. Why do
you need to do that, anyway?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Tue, Jan 11, 2011 at 4:46 AM, Tatsuo Ishii <ishii@postgresql.org> wrote:

On Fri, Jan 7, 2011 at 6:28 PM, Florian Pflug <fgp@phlo.org> wrote:

I forgot about sequences earlier. If we dump while someone deletes all
rows and resets the sequence the dump might contain rows and still
reset the sequence. This could cause duplicate key errors on restore.
I haven't checked if this is really possible though - I guess it would
depend on the exact order of these events...

To fix this, you'd need a lock that allowed getting values from the
sequence but prevented resetting it, and...

I wonder how such locks would work. Would such locks prevent accessing
these objects? Or just modifications? For example, if I locked a function,
could someone else execute it while I held the lock?

...in fact we do very little locking of objects other than tables.
DROP takes an AccessExclusiveLock on whatever it's dropping, and
COMMENT and SECURITY LABEL take ShareUpdateExclusiveLocks to avoid
orphaning pg_{sh,}description or pg_seclabel entries in the face of a
concurrent drop, but most operations on non-table objects don't AFAIK
take any lock at all.  We probably don't want to make too many changes
in this area, because there are already workloads where the
heavyweight lock manager can become a bottleneck, and one can easily
imagine that locking operators or namespaces could make that problem
much worse.

For query based replication tools like pgpool-II (I don't know any
other tools, for example Postgres XC falls in this category or
not...), we need to be able to lock sequences. Fortunately it is allowed to:

SELECT 1 FROM foo_sequece FOR UPDATE;

but LOCK foo_sequence looks more appropreate syntax for me.

Those aren't doing the same thing. The first is locking the one and
only tuple that is contained within the sequence, while the second is
locking the sequence object itself.

But a sequence relation contains only 1 tuple and there's no
difference among them, no?

At this point, I'm inclined to think that the pg_dump comment is just
wrong, and we ought to fix it to say that we don't really want to be
able to lock other relations after all, and call it good.

As a side node, locking a sequence for replication seems like it could
have pretty dire effects on performance in certain workloads. Why do
you need to do that, anyway?

Pgpool not only needs to replicate sequences but replicates tuples
updated by DMLs which are using sequence value(I am talking about
SERIAL data types). For this purpose, pgpool issue nextval() to master
DB server first, then use the value for subsequent INSERT/UPDATE. This
will guarantee that inserted/updated values using sequences are
identical among master and slave DB servers. Problem is, if this
process happens in concurrent sessions, inserted/updated tuples might
not have identical value among DB servers. So I need "sequence lock"
here. This is the price statement based replication tools have to pay
for:-<
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp

On Tue, Jan 11, 2011 at 6:31 AM, Tatsuo Ishii <ishii@postgresql.org> wrote:

For query based replication tools like pgpool-II (I don't know any
other tools, for example Postgres XC falls in this category or
not...), we need to be able to lock sequences. Fortunately it is allowed to:

SELECT 1 FROM foo_sequece FOR UPDATE;

but LOCK foo_sequence looks more appropreate syntax for me.

Those aren't doing the same thing.  The first is locking the one and
only tuple that is contained within the sequence, while the second is
locking the sequence object itself.

But a sequence relation contains only 1 tuple and there's no
difference among them, no?

No, not really. It's still a different object.

As a side node, locking a sequence for replication seems like it could
have pretty dire effects on performance in certain workloads.  Why do
you need to do that, anyway?

Pgpool not only needs to replicate sequences but replicates tuples
updated by DMLs which are using sequence value(I am talking about
SERIAL data types). For this purpose, pgpool issue nextval() to master
DB server first, then use the value for subsequent INSERT/UPDATE. This
will guarantee that inserted/updated values using sequences are
identical among master and slave DB servers. Problem is, if this
process happens in concurrent sessions, inserted/updated tuples might
not have identical value among DB servers. So I need "sequence lock"
here. This is the price statement based replication tools have to pay
for:-<

Ouch.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Jan 11, 2011 at 4:46 AM, Tatsuo Ishii <ishii@postgresql.org> wrote:

For query based replication tools like pgpool-II (I don't know any
other tools, for example Postgres XC falls in this category or
not...), we need to be able to lock sequences. Fortunately it is allowed to:

SELECT 1 FROM foo_sequece FOR UPDATE;

but LOCK foo_sequence looks more appropreate syntax for me.

Those aren't doing the same thing. The first is locking the one and
only tuple that is contained within the sequence, while the second is
locking the sequence object itself.

At this point, I'm inclined to think that the pg_dump comment is just
wrong, and we ought to fix it to say that we don't really want to be
able to lock other relations after all, and call it good.

The reason that pg_dump tries to acquire locks at all is to ensure that
it dumps a consistent view of the database. The general excuse for not
locking non-table objects is that (at least in most cases) they are
defined by single catalog entries and so there's no way to see a
non-self-consistent view of them. Tables, being defined by a collection
of rows in different catalogs, are *very* risky to dump without any
lock. This doesn't get noticeably better for non-table relation types.

An example of the sort of risk I'm thinking about is dumping a view
without any lock while someone else does a CREATE OR REPLACE VIEW on
it. You could very easily see a set of attributes (in pg_attribute)
that don't agree with the view rules you pulled from pg_rewrite. The
risk is minimal now since we don't allow C.O.R.V. to change the column
set, but as soon as somebody creates a patch that allows that, pg_dump
will have a problem.

Note that using a serializable transaction (with or without "true"
serializability) doesn't fix this issue, since pg_dump depends so
heavily on backend-side support functions that work in SnapshotNow mode.
It really needs locks to ensure that the support functions see a view
consistent with its own catalog reads.

In the SEQUENCE example above, SELECT ... FOR UPDATE is certainly not
adequate to protect the sequence against DDL-level changes. Fortunately
sequences don't have too many DDL commands, but still an ALTER RENAME
might be enough to confuse pg_dump.

(By the way, does that SELECT ... FOR UPDATE actually accomplish
anything at all? nextval() doesn't go through heap_update, and neither
does ALTER SEQUENCE, so I'd be a bit surprised if it really manages to
block changes to the sequence.)

regards, tom lane

In the SEQUENCE example above, SELECT ... FOR UPDATE is certainly not
adequate to protect the sequence against DDL-level changes. Fortunately
sequences don't have too many DDL commands, but still an ALTER RENAME
might be enough to confuse pg_dump.

(By the way, does that SELECT ... FOR UPDATE actually accomplish
anything at all? nextval() doesn't go through heap_update, and neither
does ALTER SEQUENCE, so I'd be a bit surprised if it really manages to
block changes to the sequence.)

Of course "SELECT ... FOR UPDATE" does not block nextval(). It just
blocks concurrent "SELECT ... FOR UPDATE" in other session. This is
enough for pgpool's use case.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp

On Tue, Jan 11, 2011 at 10:35 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Jan 11, 2011 at 4:46 AM, Tatsuo Ishii <ishii@postgresql.org> wrote:

For query based replication tools like pgpool-II (I don't know any
other tools, for example Postgres XC falls in this category or
not...), we need to be able to lock sequences. Fortunately it is allowed to:

SELECT 1 FROM foo_sequece FOR UPDATE;

but LOCK foo_sequence looks more appropreate syntax for me.

Those aren't doing the same thing.  The first is locking the one and
only tuple that is contained within the sequence, while the second is
locking the sequence object itself.

At this point, I'm inclined to think that the pg_dump comment is just
wrong, and we ought to fix it to say that we don't really want to be
able to lock other relations after all, and call it good.

The reason that pg_dump tries to acquire locks at all is to ensure that
it dumps a consistent view of the database.  The general excuse for not
locking non-table objects is that (at least in most cases) they are
defined by single catalog entries and so there's no way to see a
non-self-consistent view of them.  Tables, being defined by a collection
of rows in different catalogs, are *very* risky to dump without any
lock.  This doesn't get noticeably better for non-table relation types.

An example of the sort of risk I'm thinking about is dumping a view
without any lock while someone else does a CREATE OR REPLACE VIEW on
it.  You could very easily see a set of attributes (in pg_attribute)
that don't agree with the view rules you pulled from pg_rewrite.  The
risk is minimal now since we don't allow C.O.R.V. to change the column
set, but as soon as somebody creates a patch that allows that, pg_dump
will have a problem.

Actually, we do allow C.O.R.V. to do just that - I believe since 8.4.

rhaas=# create view v(a) as select 1;
CREATE VIEW
rhaas=# create or replace view v(a,b) as select 1, 2;
CREATE VIEW

Note that using a serializable transaction (with or without "true"
serializability) doesn't fix this issue, since pg_dump depends so
heavily on backend-side support functions that work in SnapshotNow mode.
It really needs locks to ensure that the support functions see a view
consistent with its own catalog reads.

In that case, can I have some comments on approaches mentioned in my OP?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Tue, Jan 11, 2011 at 8:31 PM, Robert Haas <robertmhaas@gmail.com> wrote:

In that case, can I have some comments on approaches mentioned in my OP?

Tom - I am willing to implement this if you think it's valuable, but
I'd like your input on the syntax.

http://archives.postgresql.org/pgsql-hackers/2011-01/msg00472.php

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:

Tom - I am willing to implement this if you think it's valuable, but
I'd like your input on the syntax.
http://archives.postgresql.org/pgsql-hackers/2011-01/msg00472.php

It looks to me like the reason why there's a shift/reduce conflict is
not so much that TABLE is optional as that we allow the syntax

LOCK tablename NOWAIT

If that weren't possible, then a table name would have to be followed by
EOL or IN (which is full-reserved), while an optional object type name
could not be followed by either, so there would be no shift/reduce
conflict. So we broke it when we added NOWAIT, not when TABLE was made
optional.

So it looks to me like there are at least two fixes other than the ones
you enumerated:

1. Make NOWAIT a reserved word. Not good, but perhaps better than
reserving all the different object type names.

2. Disallow the above abbreviated syntax; allow NOWAIT only after an
explicit IN ... MODE phrase. This would probably break a couple of
applications, but I bet a lot fewer than changing the longer-established
parts of the command syntax would break.

I think #2 might be the best choice here.

regards, tom lane