Обсуждение: Re: [ADMIN] 'SGT DETAIL: Could not open file "pg_clog/05DC": No such file or directory' - what to do now?
Tomasz Chmielewski <mangoo@wpkg.org> writes:
> bookstor=# SELECT 1 FROM core_wot_seq FOR UPDATE;
Um ... why are you doing that on a sequence?
> ERROR: could not access status of transaction 1573786613
> DETAIL: Could not open file "pg_clog/05DC": No such file or directory.
This doesn't surprise me too much, because sequences are not expected
to contain any live XIDs, so the XID freezing mechanism ignores them.
So if you did that in the past, this would eventually happen.
I think the most appropriate solution may be to disallow SELECT FOR
UPDATE/SHARE on sequences ... so if you have a good reason why we
shouldn't do so, please explain it.
regards, tom lane
Re: [ADMIN] 'SGT DETAIL: Could not open file "pg_clog/05DC": No such file or directory' - what to do now?
От
Tomasz Chmielewski
Дата:
On 31.05.2011 05:16, Tom Lane wrote: > Tomasz Chmielewski<mangoo@wpkg.org> writes: >> bookstor=# SELECT 1 FROM core_wot_seq FOR UPDATE; > > Um ... why are you doing that on a sequence? > >> ERROR: could not access status of transaction 1573786613 >> DETAIL: Could not open file "pg_clog/05DC": No such file or directory. > > This doesn't surprise me too much, because sequences are not expected > to contain any live XIDs, so the XID freezing mechanism ignores them. > So if you did that in the past, this would eventually happen. > > I think the most appropriate solution may be to disallow SELECT FOR > UPDATE/SHARE on sequences ... so if you have a good reason why we > shouldn't do so, please explain it. That's a good question. I grepped the sources of the application using postgres, and it certainly doesn't do it. We use pgpool though, and I see: pool_process_query.c: snprintf(qbuf, sizeof(qbuf), "SELECT 1 FROM %s FOR UPDATE", seq_rel_name); So it looks to be coming from pgpool 3.x (it didn't do it in 2.x version). This is a message explaining why it was introduced to pgpool: http://comments.gmane.org/gmane.comp.db.postgresql.pgpool.devel/348 This brings two questions: 1) whatever command I send to postgres, should I expect "Could not open file "pg_clog/05DC": No such file or directory"? If so, it should be documented, and a way to recover from such a situation should be explained. 2) is pgpool behaviour correct? -- Tomasz Chmielewski http://wpkg.org
Tomasz Chmielewski <mangoo@wpkg.org> writes: > On 31.05.2011 05:16, Tom Lane wrote: >> I think the most appropriate solution may be to disallow SELECT FOR >> UPDATE/SHARE on sequences ... so if you have a good reason why we >> shouldn't do so, please explain it. > I grepped the sources of the application using postgres, and it certainly doesn't do it. > [ but pgpool does, as of a couple months ago ] > This is a message explaining why it was introduced to pgpool: > http://comments.gmane.org/gmane.comp.db.postgresql.pgpool.devel/348 Too bad that wasn't mentioned on pgsql-hackers, where someone might have pointed out the major flaws in the idea. > 2) is pgpool behaviour correct? No. Quite aside from the lack-of-XID-maintenance problem, the proposal seems just plain bizarre to me. SELECT FOR UPDATE wouldn't block nextval(), so the command doesn't actually guarantee serialization of sequence value acquisition. Taking a table lock on the sequence could do so, and wouldn't run into any implementation issues, so I fail to see why that alternative was rejected. I'm also wondering a bit how one determines *which* sequence to lock, in a case where the table has multiple serial columns ... regards, tom lane
> Tomasz Chmielewski <mangoo@wpkg.org> writes: >> On 31.05.2011 05:16, Tom Lane wrote: >>> I think the most appropriate solution may be to disallow SELECT FOR >>> UPDATE/SHARE on sequences ... so if you have a good reason why we >>> shouldn't do so, please explain it. > >> I grepped the sources of the application using postgres, and it certainly doesn't do it. >> [ but pgpool does, as of a couple months ago ] >> This is a message explaining why it was introduced to pgpool: >> http://comments.gmane.org/gmane.comp.db.postgresql.pgpool.devel/348 > > Too bad that wasn't mentioned on pgsql-hackers, where someone might have > pointed out the major flaws in the idea. > >> 2) is pgpool behaviour correct? > > No. Quite aside from the lack-of-XID-maintenance problem, the proposal > seems just plain bizarre to me. SELECT FOR UPDATE wouldn't block > nextval(), so the command doesn't actually guarantee serialization of > sequence value acquisition. Actually it was already explained before: http://archives.postgresql.org/pgsql-hackers/2011-01/msg00805.php At the time no one noticed the lack-of-XID-maintenance problem. Tomasz, thanks for the report. I will go back to old way as pgpool-II used to do, which is very inefficient unfortunately... > Taking a table lock on the sequence could > do so, and wouldn't run into any implementation issues, so I fail to see > why that alternative was rejected. Table lock on the sequence? PostgreSQL doesn't allow it... > I'm also wondering a bit how one > determines *which* sequence to lock, in a case where the table has > multiple serial columns ... No problem at least for pgpool-II. Just choose one of them and obtain lock on it is enough. Because purpose for the lock is to prevent concurrent INSERT to the table. -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese: http://www.sraoss.co.jp
I wrote:
>>>> I think the most appropriate solution may be to disallow SELECT FOR
>>>> UPDATE/SHARE on sequences ... so if you have a good reason why we
>>>> shouldn't do so, please explain it.
Attached is a proposed patch to close off this hole. I found that
somebody had already inserted code to forbid the case for foreign
tables, so I just extended that idea a bit (by copying-and-pasting
CheckValidResultRel). Questions:
* Does anyone want to bikeshed on the wording of the error messages?
* Does anyone want to argue for not forbidding SELECT FOR UPDATE on
toast tables?
regards, tom lane
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 620efda838421a94a9835a7e8db1d8fa4b50e1ea..2d491fe6c876e34bd757ab271fada1f959707447 100644
*** a/src/backend/executor/execMain.c
--- b/src/backend/executor/execMain.c
*************** ExecutorCheckPerms_hook_type ExecutorChe
*** 74,79 ****
--- 74,80 ----
/* decls for local routines only used within this module */
static void InitPlan(QueryDesc *queryDesc, int eflags);
+ static void CheckValidMarkRel(Relation rel, RowMarkType markType);
static void ExecPostprocessPlan(EState *estate);
static void ExecEndPlan(PlanState *planstate, EState *estate);
static void ExecutePlan(EState *estate, PlanState *planstate,
*************** InitPlan(QueryDesc *queryDesc, int eflag
*** 837,848 ****
break;
}
! /* if foreign table, tuples can't be locked */
! if (relation && relation->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
! ereport(ERROR,
! (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
! errmsg("SELECT FOR UPDATE/SHARE cannot be used with foreign table \"%s\"",
! RelationGetRelationName(relation))));
erm = (ExecRowMark *) palloc(sizeof(ExecRowMark));
erm->relation = relation;
--- 838,846 ----
break;
}
! /* Check that relation is a legal target for marking */
! if (relation)
! CheckValidMarkRel(relation, rc->markType);
erm = (ExecRowMark *) palloc(sizeof(ExecRowMark));
erm->relation = relation;
*************** InitPlan(QueryDesc *queryDesc, int eflag
*** 977,982 ****
--- 975,982 ----
* In most cases parser and/or planner should have noticed this already, but
* let's make sure. In the view case we do need a test here, because if the
* view wasn't rewritten by a rule, it had better have an INSTEAD trigger.
+ *
+ * Note: when changing this function, see also CheckValidMarkRel.
*/
void
CheckValidResultRel(Relation resultRel, CmdType operation)
*************** CheckValidResultRel(Relation resultRel,
*** 1048,1053 ****
--- 1048,1104 ----
}
/*
+ * Check that a proposed rowmark target relation is a legal target
+ *
+ * In most cases parser and/or planner should have noticed this already, but
+ * they don't cover all cases.
+ */
+ static void
+ CheckValidMarkRel(Relation rel, RowMarkType markType)
+ {
+ switch (rel->rd_rel->relkind)
+ {
+ case RELKIND_RELATION:
+ /* OK */
+ break;
+ case RELKIND_SEQUENCE:
+ /* Must disallow this because we don't vacuum sequences */
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot lock rows in sequence \"%s\"",
+ RelationGetRelationName(rel))));
+ break;
+ case RELKIND_TOASTVALUE:
+ /* We could allow this, but there seems no good reason to */
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot lock rows in TOAST relation \"%s\"",
+ RelationGetRelationName(rel))));
+ break;
+ case RELKIND_VIEW:
+ /* Should not get here */
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot lock rows in view \"%s\"",
+ RelationGetRelationName(rel))));
+ break;
+ case RELKIND_FOREIGN_TABLE:
+ /* Perhaps we can support this someday, but not today */
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot lock rows in foreign table \"%s\"",
+ RelationGetRelationName(rel))));
+ break;
+ default:
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot lock rows in relation \"%s\"",
+ RelationGetRelationName(rel))));
+ break;
+ }
+ }
+
+ /*
* Initialize ResultRelInfo data for one result relation
*
* Caution: before Postgres 9.1, this function included the relkind checking
On Wed, Jun 1, 2011 at 6:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I wrote: >>>>> I think the most appropriate solution may be to disallow SELECT FOR >>>>> UPDATE/SHARE on sequences ... so if you have a good reason why we >>>>> shouldn't do so, please explain it. > > Attached is a proposed patch to close off this hole. I found that > somebody had already inserted code to forbid the case for foreign > tables, so I just extended that idea a bit (by copying-and-pasting > CheckValidResultRel). Questions: > > * Does anyone want to bikeshed on the wording of the error messages? Not particularly. > * Does anyone want to argue for not forbidding SELECT FOR UPDATE on > toast tables? Maybe. How hard would it be to fix that so it doesn't blow up? What I don't like about the proposed solution is that it will cause very user-visible breakage as a result of a minor release upgrade, for anyone using pgpool, which is a lot of people; unless pgpool is upgraded to a sufficiently new version first. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Jun 1, 2011 at 6:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> * Does anyone want to argue for not forbidding SELECT FOR UPDATE on
>> toast tables?
> Maybe. How hard would it be to fix that so it doesn't blow up? What
> I don't like about the proposed solution is that it will cause very
> user-visible breakage as a result of a minor release upgrade, for
> anyone using pgpool, which is a lot of people; unless pgpool is
> upgraded to a sufficiently new version first.
I think you are answering a different question than what I asked.
I was asking about the not-strictly-necessary forbidding of SFU on
toast tables, not sequences.
If we're going to try to retroactively make the world safe for pgpool
doing what it's doing, the only way is to start including sequences in
the set of objects that are vacuumed and included in
relfrozenxid/datfrozenxid bookkeeping. Which is a lot more overhead
than I think is justified to clean up after a bad decision. I'm not
even terribly sure that it would work, since nobody has ever looked at
what would happen if nextval executed concurrently with vacuum doing
something to a sequence. The relfrozenxid logic might have some
difficulty with sequences that have zero relfrozenxid to start with,
too.
Please note also that what pgpool users have got right now is a time
bomb, which is not better than immediately-visible breakage. I would
prefer to try to get this change out ahead of widespread adoption of the
broken pgpool version.
regards, tom lane
> If we're going to try to retroactively make the world safe for pgpool > doing what it's doing, the only way is to start including sequences in > the set of objects that are vacuumed and included in > relfrozenxid/datfrozenxid bookkeeping. Which is a lot more overhead > than I think is justified to clean up after a bad decision. I'm not > even terribly sure that it would work, since nobody has ever looked at > what would happen if nextval executed concurrently with vacuum doing > something to a sequence. The relfrozenxid logic might have some > difficulty with sequences that have zero relfrozenxid to start with, > too. What pgpool really wanted to do was locking sequence tables, not locking rows in sequences. I wonder why the former is not allowed. -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese: http://www.sraoss.co.jp
> Maybe. How hard would it be to fix that so it doesn't blow up? What > I don't like about the proposed solution is that it will cause very > user-visible breakage as a result of a minor release upgrade, for > anyone using pgpool, which is a lot of people; unless pgpool is > upgraded to a sufficiently new version first. Thanks for concerning pgpool and pgpool users. BTW, there two pgpool-II versions: - pgpool-II 2.x. uses table lock. has conflict problem with autovacuum if the target table is fairly large. - pgpool-II 3.x. uses sequence row lock to avoid the autovacuum problem. However now it has XID-wrapwround problem and Tom'sfix. So both versions are having problem at this point. Yesterday advisory locking was suggested, but after thinking while, it seems using advisory locking make fragile. So I'm still looking for other ways. Probably creating a "secret" relation and acquire table locking on it is the way to go. This is essentially a dirty alternative for sequence table locking. -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese: http://www.sraoss.co.jp
Excerpts from Tatsuo Ishii's message of mié jun 01 19:08:16 -0400 2011: > What pgpool really wanted to do was locking sequence tables, not > locking rows in sequences. I wonder why the former is not allowed. Yeah -- why is LOCK SEQUENCE foo_seq not allowed? Seems a simple thing to have. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Excerpts from Tatsuo Ishii's message of mié jun 01 19:08:16 -0400 2011:
>> What pgpool really wanted to do was locking sequence tables, not
>> locking rows in sequences. I wonder why the former is not allowed.
> Yeah -- why is LOCK SEQUENCE foo_seq not allowed? Seems a simple thing
> to have.
I don't see any particular reason to continue to disallow it, but does
that actually represent a workable solution path for pgpool? Switching
over to that would fail on older servers.
regards, tom lane
>> Yeah -- why is LOCK SEQUENCE foo_seq not allowed? Seems a simple thing >> to have. > > I don't see any particular reason to continue to disallow it, but does > that actually represent a workable solution path for pgpool? Switching > over to that would fail on older servers. pgpool will provide following method for older version of PostgreSQL. > Probably creating a "secret" relation and acquire table locking > on it is the way to go. This is essentially a dirty alternative for > sequence table locking. -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese: http://www.sraoss.co.jp
I wrote:
> Please note also that what pgpool users have got right now is a time
> bomb, which is not better than immediately-visible breakage.
BTW, so far as that goes, I suggest that we tweak nextval() and setval()
to force the sequence tuple's xmax to zero. That will provide a simple
recovery path for anyone who's at risk at the moment. Of course, this
has to go hand-in-hand with the change to forbid SELECT FOR UPDATE,
else those operations would risk breaking active tuple locks.
regards, tom lane
On Wed, Jun 1, 2011 at 7:47 PM, Alvaro Herrera <alvherre@commandprompt.com> wrote: > Excerpts from Tatsuo Ishii's message of mié jun 01 19:08:16 -0400 2011: >> What pgpool really wanted to do was locking sequence tables, not >> locking rows in sequences. I wonder why the former is not allowed. > > Yeah -- why is LOCK SEQUENCE foo_seq not allowed? Seems a simple thing > to have. It cause a grammar conflict. Since SEQUENCE and NOWAIT are both unreserved keywords, it's not clear to the parser whether "LOCK SEQUENCE NOWAIT" means to lock a table called SEQUENCE without waiting, or whether it means to lock a sequence called NOWAIT. Tom and I discussed possible ways of fixing this on -hackers a few months ago. Currently the syntax for LOCK is: LOCK [ TABLE ] [ ONLY ] name [, ...] [ IN lockmode MODE ] [ NOWAIT ]; I suggested fixing this by making TABLE required, thus: LOCK TABLE [ ONLY ] name [, ...] [ IN lockmode MODE ] [ NOWAIT ]; Tom suggested fixing it by making NOWAIT require IN lockmode MODE, thus: LOCK [ TABLE ] [ ONLY ] name [,...] [ IN lockmode MODE [ NOWAIT ]]; My proposed fix is probably more likely to break people's applications, but Tom's isn't completely free from that possibility either. It's also somewhat counterintuitive IMV. The best option might be to come up with some completely new syntax that is a little better designed than the current one, maybe along the lines of the extensible-options syntax used by EXPLAIN. The trouble is that the first word of the command would probably have to be something other than LOCK if we don't want to break backward compatibility with the existing syntax in some way, and there aren't too many good synonyms for LOCK. LATCH? FASTEN? Blech. We're probably going to end up having to make a compatibility break here if we want to support this. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Wed, Jun 1, 2011 at 6:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Please note also that what pgpool users have got right now is a time > bomb, which is not better than immediately-visible breakage. I would > prefer to try to get this change out ahead of widespread adoption of the > broken pgpool version. Hmm, I gather from what Tatsuo is saying at the web site that this has only been broken since the release of 3.0 on February 23rd, so given that I think your approach makes sense. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Jun 1, 2011 at 7:47 PM, Alvaro Herrera
> <alvherre@commandprompt.com> wrote:
>> Yeah -- why is LOCK SEQUENCE foo_seq not allowed? �Seems a simple thing
>> to have.
> It cause a grammar conflict.
That's a lot of work for a purely cosmetic issue, though. What would be
trivial is to let this work:
regression=# create sequence s1;
CREATE SEQUENCE
regression=# begin;
BEGIN
regression=# lock table s1;
ERROR: "s1" is not a table
We should do that anyway, even if we put in the effort to support the
other syntax.
regards, tom lane
Robert Haas <robertmhaas@gmail.com> writes:
> Ugh. We are already stuck supporting all kinds of backward
> compatibility cruft in tablecmds.c as a result of the fact that you
> used to have to use ALTER TABLE to operate on views and sequences.
> The whole thing is confusing and a mess.
[ shrug... ] I don't find it so. We have a convention that TABLE is
an umbrella term for all applicable relation types. End of story.
Even if you disagree with that, the convention does exist, and making
LOCK the one command type that disobeys it doesn't seem like a good
plan.
regards, tom lane
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Excerpts from Tom Lane's message of jue jun 02 10:31:58 -0400 2011:
>> That's a lot of work for a purely cosmetic issue, though. What would be
>> trivial is to let this work:
>> regression=# lock table s1;
>> ERROR: "s1" is not a table
> Yeah, though it'd be nice to avoid this:
> alvherre=# create schema public_too;
> CREATE SCHEMA
> alvherre=# set search_path to 'public_too', 'public';
> SET
> alvherre=# create table public_too.s1 ();
> CREATE TABLE
> alvherre=# create sequence public.s1;
> CREATE SEQUENCE
> alvherre=# begin;
> BEGIN
> alvherre=# lock s1;
> LOCK TABLE
> At this point we have a lock on the table, but if we change LOCK to also
> look for sequences, the behavior would change.
No it wouldn't. You seem to be imagining that sequences live in a
different namespace from tables, but they don't. There can only be one
relation that "s1" will refer to for any search_path setting.
regards, tom lane
On Thu, Jun 2, 2011 at 10:31 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Wed, Jun 1, 2011 at 7:47 PM, Alvaro Herrera >> <alvherre@commandprompt.com> wrote: >>> Yeah -- why is LOCK SEQUENCE foo_seq not allowed? Seems a simple thing >>> to have. > >> It cause a grammar conflict. > > That's a lot of work for a purely cosmetic issue, though. What would be > trivial is to let this work: > > regression=# create sequence s1; > CREATE SEQUENCE > regression=# begin; > BEGIN > regression=# lock table s1; > ERROR: "s1" is not a table > > We should do that anyway, even if we put in the effort to support the > other syntax. Ugh. We are already stuck supporting all kinds of backward compatibility cruft in tablecmds.c as a result of the fact that you used to have to use ALTER TABLE to operate on views and sequences. The whole thing is confusing and a mess. -1 from me on extending that mess to more places. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Excerpts from Tom Lane's message of jue jun 02 10:31:58 -0400 2011: > Robert Haas <robertmhaas@gmail.com> writes: > > On Wed, Jun 1, 2011 at 7:47 PM, Alvaro Herrera > > <alvherre@commandprompt.com> wrote: > >> Yeah -- why is LOCK SEQUENCE foo_seq not allowed? Seems a simple thing > >> to have. > > > It cause a grammar conflict. > > That's a lot of work for a purely cosmetic issue, though. What would be > trivial is to let this work: > > regression=# create sequence s1; > CREATE SEQUENCE > regression=# begin; > BEGIN > regression=# lock table s1; > ERROR: "s1" is not a table Yeah, though it'd be nice to avoid this: alvherre=# create schema public_too; CREATE SCHEMA alvherre=# set search_path to 'public_too', 'public'; SET alvherre=# create table public_too.s1 (); CREATE TABLE alvherre=# create sequence public.s1; CREATE SEQUENCE alvherre=# begin; BEGIN alvherre=# lock s1; LOCK TABLE At this point we have a lock on the table, but if we change LOCK to also look for sequences, the behavior would change. At the very least, the command tag should be different. Hopefully few people name sequences the same as tables ... -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Excerpts from Tom Lane's message of jue jun 02 11:10:00 -0400 2011:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
> > Excerpts from Tom Lane's message of jue jun 02 10:31:58 -0400 2011:
> >> That's a lot of work for a purely cosmetic issue, though. What would be
> >> trivial is to let this work:
> >> regression=# lock table s1;
> >> ERROR: "s1" is not a table
>
> > Yeah, though it'd be nice to avoid this:
>
> > alvherre=# create schema public_too;
> > CREATE SCHEMA
> > alvherre=# set search_path to 'public_too', 'public';
> > SET
> > alvherre=# create table public_too.s1 ();
> > CREATE TABLE
> > alvherre=# create sequence public.s1;
> > CREATE SEQUENCE
> > alvherre=# begin;
> > BEGIN
> > alvherre=# lock s1;
> > LOCK TABLE
>
> > At this point we have a lock on the table, but if we change LOCK to also
> > look for sequences, the behavior would change.
>
> No it wouldn't. You seem to be imagining that sequences live in a
> different namespace from tables, but they don't. There can only be one
> relation that "s1" will refer to for any search_path setting.
Doh, I see that I messed up and reversed the schemas in the search_path
line above. If I fix that I get the expected error:
alvherre=# set search_path to 'public', 'public_too';
SET
alvherre=# lock s1;
ERROR: «s1» no es una tabla
("s1" is not a table). What I was imagining was that LOCK was using
search path to look only for tables and ignoring sequences.
--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Thu, Jun 2, 2011 at 10:47 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> Ugh. We are already stuck supporting all kinds of backward >> compatibility cruft in tablecmds.c as a result of the fact that you >> used to have to use ALTER TABLE to operate on views and sequences. >> The whole thing is confusing and a mess. > > [ shrug... ] I don't find it so. We have a convention that TABLE is > an umbrella term for all applicable relation types. End of story. > > Even if you disagree with that, the convention does exist, and making > LOCK the one command type that disobeys it doesn't seem like a good > plan. I agree that wouldn't be a good plan to make LOCK inconsistent with everything else, but LOCK is not the only case that's like this: rhaas=# drop table v1; ERROR: "v1" is not a table HINT: Use DROP VIEW to remove a view. rhaas=# comment on table v1 is 'v1 is a view'; ERROR: "v1" is not a table rhaas=# load 'dummy_seclabel'; LOAD rhaas=# security label on table v1 is 'classified'; ERROR: "v1" is not a table As far as I can see, ALTER TABLE is just about the only place where we allow this; and only for certain command types. Your commit message seems to indicate that we continue to allow that stuff only for backward-compatibility: commit a0b012a1ab85ae115f30e5e4fe09922b4885fdad Author: Tom Lane <tgl@sss.pgh.pa.us> Date: Sun Jun 15 01:25:54 2008 +0000 Rearrange ALTER TABLE syntax processing as per my recent proposal: the grammar allows ALTER TABLE/INDEX/SEQUENCE/VIEWinterchangeably for all subforms of those commands, and then we sort out what's really legal at execution time. This allows the ALTER SEQUENCE/VIEW reference pages to fully document all the ALTER forms availablefor sequences and views respectively, and eliminates a longstanding cause of confusion for users. The net effect is that the following forms are allowed that weren't before: ALTER SEQUENCE OWNER TO ALTERVIEW ALTER COLUMN SET/DROP DEFAULT ALTER VIEW OWNER TO ALTER VIEW SET SCHEMA (There's no actual functionalitygain here, but formerly you had to say ALTER TABLE instead.) Interestingly, the grammar tables actually get smaller, probably because there are fewer special cases to keep trackof. I did not disallow using ALTER TABLE for these operations. Perhaps we should, but there's a backwards-compatibilityissue if we do; in fact it would break existing pg_dump scripts. I did however tighten up ALTERSEQUENCE and ALTER VIEW to reject non-sequences and non-views in the new cases as well as a couple of cases wherethey didn't before. The patch doesn't change pg_dump to use the new syntaxes, either. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Jun 1, 2011 at 6:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Please note also that what pgpool users have got right now is a time
>> bomb, which is not better than immediately-visible breakage. �I would
>> prefer to try to get this change out ahead of widespread adoption of the
>> broken pgpool version.
> Hmm, I gather from what Tatsuo is saying at the web site that this has
> only been broken since the release of 3.0 on February 23rd, so given
> that I think your approach makes sense.
Done, and I also installed a kluge to clean up the damage retroactively
during any nextval/setval operation.
regards, tom lane