Thread: Re: [PATCHES] NO WAIT ...
Tom Lane wrote: > Hans-Jürgen Schönig <postgres@cybertec.at> writes: > > The problem with adding NO WAIT to specific commands is that is > > inheritly unflexible. I think this is why the community has agreed on > > implementing it based on GUC. > > I recall no such agreement ... when was this exactly? In any case > Bruce's recent complaints about regex_flavor have altered my opinions > about GUC variables a bit. They are bigger safety risks than they look, > especially ones that change semantics and are intended to be modified on > the fly. > > > Do you think it would help to reduce the GUCs flexibility by reducing > > the lock levels a user is allowed to define? > > I will vote against the patch no matter what, but I agree that it would > be less dangerous if it were confined to only apply to a limited set of > lock types. The question is whether we should have a GUC variable to control no waiting on locks or add NO WAIT to specific SQL commands. Does anyone want to vote _against_ the GUC idea for nowait locking. (We already have two voting for such a variable.) If there is no one except Tom, we can continue. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian <pgman@candle.pha.pa.us> writes: > The question is whether we should have a GUC variable to control no > waiting on locks or add NO WAIT to specific SQL commands. That's only a minor part of the issue. The major problem I have with the patch is that it affects *all* locks, including system-internal lock attempts that the user is probably not even aware of much less able to control. It's like giving someone a poorly-aligned shotgun when what they need is a rifle --- they'll end up putting holes in a lot of other things besides what they intended. I think that what we actually want is something that is narrowly tailored to affect only row-level locks taken by SELECT FOR UPDATE, and maybe one or two other places that (a) people can make specific use-cases for, and (b) we can be certain are only invoked by user commands and never indirectly from behind-the-scenes system operations. The reason for proposing syntax rather than a GUC variable is the same one of control. If you set a GUC variable then it will be hard to prevent it from breaking operations other than the one you thought you intended. (Example: you think you are only causing your SELECT FOR UPDATE to error out, but what about ones done behind the scenes for foreign key checks?) GUC variables are good for stuff that tends to apply application-wide, which is why I thought regex_flavor wasn't too dangerous, but they're terrible for functions that you want to apply to only certain specific operations. And I can't imagine an app where that wouldn't be true for NO WAIT. regards, tom lane
--On Wednesday, February 18, 2004 13:56:14 -0500 Bruce Momjian <pgman@candle.pha.pa.us> wrote: > Tom Lane wrote: > The question is whether we should have a GUC variable to control no > waiting on locks or add NO WAIT to specific SQL commands. > > Does anyone want to vote _against_ the GUC idea for nowait locking. (We > already have two voting for such a variable.) > > If there is no one except Tom, we can continue. I'm with Tom. Playing with *ALL* locks is just asking for TROUBLE. (I don't know if I count). -- Larry Rosenman http://www.lerctr.org/~ler Phone: +1 972-414-9812 E-Mail: ler@lerctr.org US Mail: 1905 Steamboat Springs Drive, Garland, TX 75044-6749
> The question is whether we should have a GUC variable to control no > waiting on locks or add NO WAIT to specific SQL commands. > > Does anyone want to vote _against_ the GUC idea for nowait locking. (We > already have two voting for such a variable.) I vote against. We got bit by both the regex and the autocommit GUC vars and this is setting up to cause a similar headache with old code on new platforms.
I agree with Tom here. I have used the Oracle NOWAIT feature in the past and think it is a great feature IMHO. But when you need to use it, you want it to apply very specifically to a single statement. Using a sledge hammer when you need a tweezers isn't the right way to go. thanks, --Barry Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > >>The question is whether we should have a GUC variable to control no >>waiting on locks or add NO WAIT to specific SQL commands. > > > That's only a minor part of the issue. The major problem I have with > the patch is that it affects *all* locks, including system-internal > lock attempts that the user is probably not even aware of much less > able to control. It's like giving someone a poorly-aligned shotgun > when what they need is a rifle --- they'll end up putting holes in > a lot of other things besides what they intended. > > I think that what we actually want is something that is narrowly > tailored to affect only row-level locks taken by SELECT FOR UPDATE, > and maybe one or two other places that (a) people can make specific > use-cases for, and (b) we can be certain are only invoked by user > commands and never indirectly from behind-the-scenes system operations. > > The reason for proposing syntax rather than a GUC variable is the same > one of control. If you set a GUC variable then it will be hard to > prevent it from breaking operations other than the one you thought you > intended. (Example: you think you are only causing your SELECT FOR > UPDATE to error out, but what about ones done behind the scenes for > foreign key checks?) GUC variables are good for stuff that tends to > apply application-wide, which is why I thought regex_flavor wasn't too > dangerous, but they're terrible for functions that you want to apply to > only certain specific operations. And I can't imagine an app where that > wouldn't be true for NO WAIT. > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 5: Have you checked our extensive FAQ? > > http://www.postgresql.org/docs/faqs/FAQ.html
Bruce Momjian wrote: >The question is whether we should have a GUC variable to control no >waiting on locks or add NO WAIT to specific SQL commands. > >Does anyone want to vote _against_ the GUC idea for nowait locking. (We >already have two voting for such a variable.) > >If there is no one except Tom, we can continue. > > > This strikes me as a very broad-brush approach. I suspect that even if we did not cause major breakage we would end up wanting to go back and implement the finer grained per command mechanism. So I'm not in favor - always open to persuasion, of course ;-) cheers andrew
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > The question is whether we should have a GUC variable to control no > > waiting on locks or add NO WAIT to specific SQL commands. > > That's only a minor part of the issue. The major problem I have with > the patch is that it affects *all* locks, including system-internal > lock attempts that the user is probably not even aware of much less > able to control. It's like giving someone a poorly-aligned shotgun > when what they need is a rifle --- they'll end up putting holes in > a lot of other things besides what they intended. > > I think that what we actually want is something that is narrowly > tailored to affect only row-level locks taken by SELECT FOR UPDATE, > and maybe one or two other places that (a) people can make specific > use-cases for, and (b) we can be certain are only invoked by user > commands and never indirectly from behind-the-scenes system operations. > > The reason for proposing syntax rather than a GUC variable is the same > one of control. If you set a GUC variable then it will be hard to > prevent it from breaking operations other than the one you thought you > intended. (Example: you think you are only causing your SELECT FOR > UPDATE to error out, but what about ones done behind the scenes for > foreign key checks?) GUC variables are good for stuff that tends to > apply application-wide, which is why I thought regex_flavor wasn't too > dangerous, but they're terrible for functions that you want to apply to > only certain specific operations. And I can't imagine an app where that > wouldn't be true for NO WAIT. Well, they have statement_timeout to prevent a command from taking too long, so that obviously isn't the usage case for NO WAIT. The problem I see for statement_timeout emulating NO WAIT is that on a lightly loaded machine, the usual query time is different from a loaded machine, so guessing a number seems difficult. Saying "Oh, the query is taking longer than X seconds, I must be waiting on a lock" is prone to failure. And if you get a faster machine or have an app that runs on machines of different speeds, it doesn't work either. One idea would be to allow the NOWAIT take a duration like statement_timeout so you could say I only want to wait a maximum of X ms before failing. However, if the usage case for NOWAIT is for an applicaiton to return a string saying "Record Locked", a GUC variable will not work and we have to be fine-grained about it as you suggest. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Larry Rosenman wrote: -- Start of PGP signed section. > > > --On Wednesday, February 18, 2004 13:56:14 -0500 Bruce Momjian > <pgman@candle.pha.pa.us> wrote: > > > Tom Lane wrote: > > > The question is whether we should have a GUC variable to control no > > waiting on locks or add NO WAIT to specific SQL commands. > > > > Does anyone want to vote _against_ the GUC idea for nowait locking. (We > > already have two voting for such a variable.) > > > > If there is no one except Tom, we can continue. > I'm with Tom. Playing with *ALL* locks is just asking for TROUBLE. OK, seems folks want the NO WAIT to apply only to the requested lock, and want syntax to show that. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
TODO updated: < * Add GUC variable to prevent waiting on locks> * Add NO WAIT option to various SQL commands --------------------------------------------------------------------------- Barry Lind wrote: > I agree with Tom here. I have used the Oracle NOWAIT feature in the > past and think it is a great feature IMHO. But when you need to use it, > you want it to apply very specifically to a single statement. Using a > sledge hammer when you need a tweezers isn't the right way to go. > > thanks, > --Barry > > Tom Lane wrote: > > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > > >>The question is whether we should have a GUC variable to control no > >>waiting on locks or add NO WAIT to specific SQL commands. > > > > > > That's only a minor part of the issue. The major problem I have with > > the patch is that it affects *all* locks, including system-internal > > lock attempts that the user is probably not even aware of much less > > able to control. It's like giving someone a poorly-aligned shotgun > > when what they need is a rifle --- they'll end up putting holes in > > a lot of other things besides what they intended. > > > > I think that what we actually want is something that is narrowly > > tailored to affect only row-level locks taken by SELECT FOR UPDATE, > > and maybe one or two other places that (a) people can make specific > > use-cases for, and (b) we can be certain are only invoked by user > > commands and never indirectly from behind-the-scenes system operations. > > > > The reason for proposing syntax rather than a GUC variable is the same > > one of control. If you set a GUC variable then it will be hard to > > prevent it from breaking operations other than the one you thought you > > intended. (Example: you think you are only causing your SELECT FOR > > UPDATE to error out, but what about ones done behind the scenes for > > foreign key checks?) GUC variables are good for stuff that tends to > > apply application-wide, which is why I thought regex_flavor wasn't too > > dangerous, but they're terrible for functions that you want to apply to > > only certain specific operations. And I can't imagine an app where that > > wouldn't be true for NO WAIT. > > > > regards, tom lane > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 5: Have you checked our extensive FAQ? > > > > http://www.postgresql.org/docs/faqs/FAQ.html > > > ---------------------------(end of broadcast)--------------------------- > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: >> The question is whether we should have a GUC variable to control no >> waiting on locks or add NO WAIT to specific SQL commands. > > That's only a minor part of the issue. The major problem I have with > the patch is that it affects *all* locks, including system-internal > lock attempts that the user is probably not even aware of much less > able to control. It's like giving someone a poorly-aligned shotgun > when what they need is a rifle --- they'll end up putting holes in > a lot of other things besides what they intended. I absolutely agree to that. > > I think that what we actually want is something that is narrowly > tailored to affect only row-level locks taken by SELECT FOR UPDATE, > and maybe one or two other places that (a) people can make specific > use-cases for, and (b) we can be certain are only invoked by user > commands and never indirectly from behind-the-scenes system operations. I would gereralize this to user table row level and explicit lock table. There is no need to force a separate SELECT FOR UPDATE in front of every UPDATE or DELETE attempt in order to achieve the desired nolock behaviour. > > The reason for proposing syntax rather than a GUC variable is the same > one of control. If you set a GUC variable then it will be hard to > prevent it from breaking operations other than the one you thought you > intended. (Example: you think you are only causing your SELECT FOR > UPDATE to error out, but what about ones done behind the scenes for > foreign key checks?) GUC variables are good for stuff that tends to > apply application-wide, which is why I thought regex_flavor wasn't too > dangerous, but they're terrible for functions that you want to apply to > only certain specific operations. And I can't imagine an app where that > wouldn't be true for NO WAIT. This all needs a lot more detail than it has so far. If one tries to UPDATE a row with NOLOCK, and a trigger fired during this would block on the attempt to create a temp table, I think the resulting transaction abort would rather be reported as a bug to us, then accepted as a nice feature. If there is a call for vote, I vote against. Jan -- #======================================================================# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #================================================== JanWieck@Yahoo.com #
> I agree with Tom here. I have used the Oracle NOWAIT feature in the > past and think it is a great feature IMHO. But when you need to use it, > you want it to apply very specifically to a single statement. Using a > sledge hammer when you need a tweezers isn't the right way to go. Once I have written patches for 7.3 to implement this feature for LOCK statement. For example: test=# LOCK TABLE sales NO WAIT; ERROR: Cannot aquire relation lock If there's enough interest, I will modify and submit it for 7.5. -- Tatsuo Ishii
Tatsuo Ishii wrote: >>I agree with Tom here. I have used the Oracle NOWAIT feature in the >>past and think it is a great feature IMHO. But when you need to use it, >>you want it to apply very specifically to a single statement. Using a >>sledge hammer when you need a tweezers isn't the right way to go. > > > Once I have written patches for 7.3 to implement this feature for LOCK > statement. For example: > > test=# LOCK TABLE sales NO WAIT; > ERROR: Cannot aquire relation lock > > If there's enough interest, I will modify and submit it for 7.5. > -- > Tatsuo Ishii > > ---------------------------(end of broadcast)--------------------------- > TIP 8: explain analyze is your friend That would be great. Many people are asking for that. Maybe I have time to implement that for SELECT FOR UPDATE. Hans -- Cybertec Geschwinde u Schoenig Schoengrabern 134, A-2020 Hollabrunn, Austria Tel: +43/2952/30706 or +43/664/233 90 75 www.cybertec.at, www.postgresql.at, kernel.cybertec.at
> Tatsuo Ishii wrote: > >>I agree with Tom here. I have used the Oracle NOWAIT feature in the > >>past and think it is a great feature IMHO. But when you need to use it, > >>you want it to apply very specifically to a single statement. Using a > >>sledge hammer when you need a tweezers isn't the right way to go. > > > > > > Once I have written patches for 7.3 to implement this feature for LOCK > > statement. For example: > > > > test=# LOCK TABLE sales NO WAIT; > > ERROR: Cannot aquire relation lock > > > > If there's enough interest, I will modify and submit it for 7.5. > > -- > > Tatsuo Ishii > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 8: explain analyze is your friend > > > That would be great. > Many people are asking for that. > Maybe I have time to implement that for SELECT FOR UPDATE. Here it is(against 7.3.3). -- Tatsuo Ishii *** ./src/backend/access/heap/heapam.c.orig 2003-07-04 09:12:10.000000000 +0900 --- ./src/backend/access/heap/heapam.c 2003-07-12 13:52:43.000000000 +0900 *************** *** 479,484 **** --- 479,517 ---- return r; } + Relation + conditional_relation_open(Oid relationId, LOCKMODE lockmode, bool no_wait) + { + Relation r; + + Assert(lockmode >= NoLock && lockmode < MAX_LOCKMODES); + + /* + * increment access statistics + */ + IncrHeapAccessStat(local_open); + IncrHeapAccessStat(global_open); + + /* The relcache does all the real work... */ + r = RelationIdGetRelation(relationId); + + if (!RelationIsValid(r)) + elog(ERROR, "Relation %u does not exist", relationId); + + if (lockmode != NoLock) + { + if (no_wait) + { + if (!ConditionalLockRelation(r, lockmode)) + elog(ERROR, "Cannot aquire relation lock"); + } + else + LockRelation(r, lockmode); + } + + return r; + } + /* ---------------- * relation_openrv - open any relation specified by a RangeVar * *** ./src/backend/commands/lockcmds.c.orig 2003-07-04 09:21:30.000000000 +0900 --- ./src/backend/commands/lockcmds.c 2003-07-12 13:53:30.000000000 +0900 *************** *** 58,64 **** if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, get_rel_name(reloid)); ! rel = relation_open(reloid, lockstmt->mode); /* Currently, we only allow plain tables to be locked */ if (rel->rd_rel->relkind != RELKIND_RELATION) --- 58,64 ---- if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, get_rel_name(reloid)); ! rel = conditional_relation_open(reloid, lockstmt->mode, lockstmt->no_wait); /* Currently, we only allowplain tables to be locked */ if (rel->rd_rel->relkind != RELKIND_RELATION) *** ./src/backend/parser/gram.y.orig 2003-07-12 12:45:30.000000000 +0900 --- ./src/backend/parser/gram.y 2003-07-12 13:37:27.000000000 +0900 *************** *** 161,166 **** --- 161,167 ---- %type <ival> opt_lock lock_type cast_context %type <boolean> opt_force opt_or_replace + %type <boolean> opt_no_wait %type <list> user_list *************** *** 392,397 **** --- 393,400 ---- VACUUM VALID VALIDATOR VALUES VARCHAR VARYING VERBOSE VERSION VIEW VOLATILE + WAIT + WHEN WHERE WITH WITHOUT WORK WRITE YEAR_P *************** *** 4050,4061 **** } ; ! LockStmt: LOCK_P opt_table qualified_name_list opt_lock { LockStmt *n = makeNode(LockStmt); n->relations = $3; n->mode = $4; $$ = (Node*)n; } ; --- 4053,4065 ---- } ; ! LockStmt: LOCK_P opt_table qualified_name_list opt_lock opt_no_wait { LockStmt *n= makeNode(LockStmt); n->relations = $3; n->mode = $4; + n->no_wait = $5; $$ = (Node *)n; } ; *************** *** 4074,4079 **** --- 4078,4087 ---- | ACCESS EXCLUSIVE { $$ = AccessExclusiveLock; } ; + opt_no_wait: NO WAIT { $$ = TRUE; } + | /*EMPTY*/ { $$ = FALSE; } + ; + /***************************************************************************** * *** ./src/backend/parser/keywords.c.orig 2003-07-12 13:39:23.000000000 +0900 --- ./src/backend/parser/keywords.c 2003-07-12 13:39:47.000000000 +0900 *************** *** 318,323 **** --- 318,324 ---- {"version", VERSION}, {"view", VIEW}, {"volatile", VOLATILE}, + {"wait", WAIT}, {"when", WHEN}, {"where", WHERE}, {"with", WITH}, *** ./src/include/nodes/parsenodes.h.orig 2003-07-12 12:48:48.000000000 +0900 --- ./src/include/nodes/parsenodes.h 2003-07-12 13:44:04.000000000 +0900 *************** *** 1593,1598 **** --- 1593,1599 ---- NodeTag type; List *relations; /* relations to lock */ int mode; /* lock mode */ + bool no_wait; /* no wait mode */ } LockStmt; /* ---------------------- *** src/include/access/heapam.h.orig 2003-07-19 14:08:22.000000000 +0900 --- src/include/access/heapam.h 2003-07-19 14:08:48.000000000 +0900 *************** *** 136,141 **** --- 136,142 ---- /* heapam.c */ extern Relation relation_open(Oid relationId, LOCKMODE lockmode); + extern Relation conditional_relation_open(Oid relationId, LOCKMODE lockmode, bool no_wait); extern Relation relation_openrv(constRangeVar *relation, LOCKMODE lockmode); extern Relation relation_openr(const char *sysRelationName,LOCKMODE lockmode); extern void relation_close(Relation relation, LOCKMODE lockmode);
LOCK TABLE table NO WAIT is OK for 7.5? If ok, I will make patches against current with some docs changes. -- Tatsuo Ishii > > Tatsuo Ishii wrote: > > >>I agree with Tom here. I have used the Oracle NOWAIT feature in the > > >>past and think it is a great feature IMHO. But when you need to use it, > > >>you want it to apply very specifically to a single statement. Using a > > >>sledge hammer when you need a tweezers isn't the right way to go. > > > > > > > > > Once I have written patches for 7.3 to implement this feature for LOCK > > > statement. For example: > > > > > > test=# LOCK TABLE sales NO WAIT; > > > ERROR: Cannot aquire relation lock > > > > > > If there's enough interest, I will modify and submit it for 7.5. > > > -- > > > Tatsuo Ishii > > > > > > ---------------------------(end of broadcast)--------------------------- > > > TIP 8: explain analyze is your friend > > > > > > That would be great. > > Many people are asking for that. > > Maybe I have time to implement that for SELECT FOR UPDATE. > > Here it is(against 7.3.3). > -- > Tatsuo Ishii
Yes, I think it looks good. --------------------------------------------------------------------------- Tatsuo Ishii wrote: > LOCK TABLE table NO WAIT is OK for 7.5? If ok, I will make patches > against current with some docs changes. > -- > Tatsuo Ishii > > > > Tatsuo Ishii wrote: > > > >>I agree with Tom here. I have used the Oracle NOWAIT feature in the > > > >>past and think it is a great feature IMHO. But when you need to use it, > > > >>you want it to apply very specifically to a single statement. Using a > > > >>sledge hammer when you need a tweezers isn't the right way to go. > > > > > > > > > > > > Once I have written patches for 7.3 to implement this feature for LOCK > > > > statement. For example: > > > > > > > > test=# LOCK TABLE sales NO WAIT; > > > > ERROR: Cannot aquire relation lock > > > > > > > > If there's enough interest, I will modify and submit it for 7.5. > > > > -- > > > > Tatsuo Ishii > > > > > > > > ---------------------------(end of broadcast)--------------------------- > > > > TIP 8: explain analyze is your friend > > > > > > > > > That would be great. > > > Many people are asking for that. > > > Maybe I have time to implement that for SELECT FOR UPDATE. > > > > Here it is(against 7.3.3). > > -- > > Tatsuo Ishii > > ---------------------------(end of broadcast)--------------------------- > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Tatsuo Ishii <t-ishii@sra.co.jp> writes: > LOCK TABLE table NO WAIT is OK for 7.5? If ok, I will make patches > against current with some docs changes. Dept of minor gripes: can we do this without turning "NO" into a keyword? Even as a nonreserved word, I think that would be annoying. "no" is a common abbreviation for "number" so I think it's likely to get used as a column name. If Oracle spells it "NOWAIT" then I'd be much happier with that... regards, tom lane
If "NOWAIT" is the choice, I could live with it. If there's no objection, I will go with "NOWAIT", not "NO WAIT". -- Tatsuo Ishii > Tatsuo Ishii <t-ishii@sra.co.jp> writes: > > LOCK TABLE table NO WAIT is OK for 7.5? If ok, I will make patches > > against current with some docs changes. > > Dept of minor gripes: can we do this without turning "NO" into a > keyword? Even as a nonreserved word, I think that would be annoying. > "no" is a common abbreviation for "number" so I think it's likely to > get used as a column name. > > If Oracle spells it "NOWAIT" then I'd be much happier with that... > > regards, tom lane >
> If "NOWAIT" is the choice, I could live with it. If there's no > objection, I will go with "NOWAIT", not "NO WAIT". How about "WITHOUT WAIT", which is like many of our other commands? Chris
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes: >> If "NOWAIT" is the choice, I could live with it. If there's no >> objection, I will go with "NOWAIT", not "NO WAIT". > How about "WITHOUT WAIT", which is like many of our other commands? The first question in my mind is "exactly how does Oracle spell this?" Let's not go inventing compatible-with-no-one syntax if we don't have to. regards, tom lane
Oracle uses "NOWAIT" so we should go for that one.Regards, Hans Tatsuo Ishii wrote: > If "NOWAIT" is the choice, I could live with it. If there's no > objection, I will go with "NOWAIT", not "NO WAIT". > -- > Tatsuo Ishii > > >>Tatsuo Ishii <t-ishii@sra.co.jp> writes: >> >>>LOCK TABLE table NO WAIT is OK for 7.5? If ok, I will make patches >>>against current with some docs changes. >> >>Dept of minor gripes: can we do this without turning "NO" into a >>keyword? Even as a nonreserved word, I think that would be annoying. >>"no" is a common abbreviation for "number" so I think it's likely to >>get used as a column name. >> >>If Oracle spells it "NOWAIT" then I'd be much happier with that... >> >> regards, tom lane >> > > > ---------------------------(end of broadcast)--------------------------- > TIP 6: Have you searched our list archives? > > http://archives.postgresql.org -- Cybertec Geschwinde u Schoenig Schoengrabern 134, A-2020 Hollabrunn, Austria Tel: +43/2952/30706 or +43/664/233 90 75 www.cybertec.at, www.postgresql.at, kernel.cybertec.at
It seems "NOWAIT" is the winner... -- Tatsuo Ishii > Oracle uses "NOWAIT" so we should go for that one. > > Regards, > > Hans > > > > Tatsuo Ishii wrote: > > If "NOWAIT" is the choice, I could live with it. If there's no > > objection, I will go with "NOWAIT", not "NO WAIT". > > -- > > Tatsuo Ishii > > > > > >>Tatsuo Ishii <t-ishii@sra.co.jp> writes: > >> > >>>LOCK TABLE table NO WAIT is OK for 7.5? If ok, I will make patches > >>>against current with some docs changes. > >> > >>Dept of minor gripes: can we do this without turning "NO" into a > >>keyword? Even as a nonreserved word, I think that would be annoying. > >>"no" is a common abbreviation for "number" so I think it's likely to > >>get used as a column name. > >> > >>If Oracle spells it "NOWAIT" then I'd be much happier with that... > >> > >> regards, tom lane > >> > > > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 6: Have you searched our list archives? > > > > http://archives.postgresql.org > > > -- > Cybertec Geschwinde u Schoenig > Schoengrabern 134, A-2020 Hollabrunn, Austria > Tel: +43/2952/30706 or +43/664/233 90 75 > www.cybertec.at, www.postgresql.at, kernel.cybertec.at > > > ---------------------------(end of broadcast)--------------------------- > TIP 5: Have you checked our extensive FAQ? > > http://www.postgresql.org/docs/faqs/FAQ.html >
Here is the patch I promised (against current). Regression tests all passed. One thing I have not checked is the doc(lock.sgml). For some reason I failed to install docbook V4.2 (I have working docbook V3.1 though), and I couldn't test the correctness of the file. Also, it would be nice if some one checks my English grammer:-) -- Tatsuo Ishii > It seems "NOWAIT" is the winner... > -- > Tatsuo Ishii > > > Oracle uses "NOWAIT" so we should go for that one. > > > > Regards, > > > > Hans > > > > > > > > Tatsuo Ishii wrote: > > > If "NOWAIT" is the choice, I could live with it. If there's no > > > objection, I will go with "NOWAIT", not "NO WAIT". > > > -- > > > Tatsuo Ishii > > > > > > > > >>Tatsuo Ishii <t-ishii@sra.co.jp> writes: > > >> > > >>>LOCK TABLE table NO WAIT is OK for 7.5? If ok, I will make patches > > >>>against current with some docs changes. > > >> > > >>Dept of minor gripes: can we do this without turning "NO" into a > > >>keyword? Even as a nonreserved word, I think that would be annoying. > > >>"no" is a common abbreviation for "number" so I think it's likely to > > >>get used as a column name. > > >> > > >>If Oracle spells it "NOWAIT" then I'd be much happier with that... > > >> > > >> regards, tom lane > > >> > > > > > > > > > ---------------------------(end of broadcast)--------------------------- > > > TIP 6: Have you searched our list archives? > > > > > > http://archives.postgresql.org > > > > > > -- > > Cybertec Geschwinde u Schoenig > > Schoengrabern 134, A-2020 Hollabrunn, Austria > > Tel: +43/2952/30706 or +43/664/233 90 75 > > www.cybertec.at, www.postgresql.at, kernel.cybertec.at > > > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 5: Have you checked our extensive FAQ? > > > > http://www.postgresql.org/docs/faqs/FAQ.html > > > > ---------------------------(end of broadcast)--------------------------- > TIP 2: you can get off all lists at once with the unregister command > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) > Index: doc/src/sgml/ref/lock.sgml =================================================================== RCS file: /cvsroot/pgsql-server/doc/src/sgml/ref/lock.sgml,v retrieving revision 1.40 diff -c -r1.40 lock.sgml *** doc/src/sgml/ref/lock.sgml 14 Dec 2003 00:05:29 -0000 1.40 --- doc/src/sgml/ref/lock.sgml 9 Mar 2004 12:42:31 -0000 *************** *** 20,26 **** <refsynopsisdiv> <synopsis> ! LOCK [ TABLE ] <replaceable class="PARAMETER">name</replaceable> [, ...] [ IN <replaceable class="PARAMETER">lockmode</replaceable>MODE ] where <replaceable class="PARAMETER">lockmode</replaceable> is one of: --- 20,26 ---- <refsynopsisdiv> <synopsis> ! LOCK [ TABLE ] <replaceable class="PARAMETER">name</replaceable> [, ...] [ IN <replaceable class="PARAMETER">lockmode</replaceable>MODE ] [ NOWAIT ] where <replaceable class="PARAMETER">lockmode</replaceable> isone of: *************** *** 34,41 **** <para> <command>LOCK TABLE</command> obtains a table-level lock, waiting if ! necessary for any conflicting locks to be released. Once obtained, ! the lock is held for the remainder of the current transaction. (There is no <command>UNLOCK TABLE</command> command;locks are always released at transaction end.) </para> --- 34,43 ---- <para> <command>LOCK TABLE</command> obtains a table-level lock, waiting if ! necessary for any conflicting locks to be released. ! If <literal>NOWAIT</literal> is given, <command>LOCK TABLE</command> ! does not wait for acquiring lock, and throws an error instead. ! Once obtained, the lock is held for the remainder of the current transaction. (There is no <command>UNLOCK TABLE</command>command; locks are always released at transaction end.) </para> Index: src/backend/access/heap/heapam.c =================================================================== RCS file: /cvsroot/pgsql-server/src/backend/access/heap/heapam.c,v retrieving revision 1.162 diff -c -r1.162 heapam.c *** src/backend/access/heap/heapam.c 16 Jan 2004 20:51:30 -0000 1.162 --- src/backend/access/heap/heapam.c 9 Mar 2004 12:42:33 -0000 *************** *** 464,469 **** --- 464,496 ---- return r; } + Relation + conditional_relation_open(Oid relationId, LOCKMODE lockmode, bool nowait) + { + Relation r; + + Assert(lockmode >= NoLock && lockmode < MAX_LOCKMODES); + + /* The relcache does all the real work... */ + r = RelationIdGetRelation(relationId); + + if (!RelationIsValid(r)) + elog(ERROR, "could not open relation with OID %u", relationId); + + if (lockmode != NoLock) + { + if (nowait) + { + if (!ConditionalLockRelation(r, lockmode)) + elog(ERROR, "could not aquire relation lock"); + } + else + LockRelation(r, lockmode); + } + + return r; + } + /* ---------------- * relation_openrv - open any relation specified by a RangeVar * Index: src/backend/commands/lockcmds.c =================================================================== RCS file: /cvsroot/pgsql-server/src/backend/commands/lockcmds.c,v retrieving revision 1.8 diff -c -r1.8 lockcmds.c *** src/backend/commands/lockcmds.c 29 Nov 2003 19:51:47 -0000 1.8 --- src/backend/commands/lockcmds.c 9 Mar 2004 12:42:34 -0000 *************** *** 59,65 **** aclcheck_error(aclresult, ACL_KIND_CLASS, get_rel_name(reloid)); ! rel = relation_open(reloid, lockstmt->mode); /* Currently, we only allow plain tables to be locked */ if (rel->rd_rel->relkind != RELKIND_RELATION) --- 59,65 ---- aclcheck_error(aclresult, ACL_KIND_CLASS, get_rel_name(reloid)); ! rel = conditional_relation_open(reloid, lockstmt->mode, lockstmt->nowait); /* Currently, we only allowplain tables to be locked */ if (rel->rd_rel->relkind != RELKIND_RELATION) Index: src/backend/parser/gram.y =================================================================== RCS file: /cvsroot/pgsql-server/src/backend/parser/gram.y,v retrieving revision 2.447 diff -c -r2.447 gram.y *** src/backend/parser/gram.y 9 Mar 2004 05:05:41 -0000 2.447 --- src/backend/parser/gram.y 9 Mar 2004 12:42:37 -0000 *************** *** 169,174 **** --- 169,175 ---- %type <ival> opt_lock lock_type cast_context %type <boolean> opt_force opt_or_replace transaction_access_mode opt_grant_grant_option opt_revoke_grant_option + opt_nowait %type <boolean> like_including_defaults *************** *** 375,381 **** MATCH MAXVALUE MINUTE_P MINVALUE MODE MONTH_P MOVE NAMES NATIONAL NATURAL NCHAR NEW NEXT NO NOCREATEDB ! NOCREATEUSER NONE NOT NOTHING NOTIFY NOTNULL NULL_P NULLIF NUMERIC OBJECT_P OF OFF OFFSET OIDS OLD ON ONLYOPERATOR OPTION OR --- 376,382 ---- MATCH MAXVALUE MINUTE_P MINVALUE MODE MONTH_P MOVE NAMES NATIONAL NATURAL NCHAR NEW NEXT NO NOCREATEDB ! NOCREATEUSER NONE NOT NOTHING NOTIFY NOTNULL NOWAIT NULL_P NULLIF NUMERIC OBJECT_P OF OFF OFFSET OIDS OLDON ONLY OPERATOR OPTION OR *************** *** 4347,4358 **** } ; ! LockStmt: LOCK_P opt_table qualified_name_list opt_lock { LockStmt *n = makeNode(LockStmt); n->relations = $3; n->mode = $4; $$ = (Node*)n; } ; --- 4348,4360 ---- } ; ! LockStmt: LOCK_P opt_table qualified_name_list opt_lock opt_nowait { LockStmt *n= makeNode(LockStmt); n->relations = $3; n->mode = $4; + n->nowait = $5; $$ = (Node *)n; } ; *************** *** 4369,4374 **** --- 4371,4380 ---- | SHARE ROW EXCLUSIVE { $$ = ShareRowExclusiveLock; } | EXCLUSIVE { $$ = ExclusiveLock; } | ACCESS EXCLUSIVE { $$ = AccessExclusiveLock; } + ; + + opt_nowait: NOWAIT { $$ = TRUE; } + | /*EMPTY*/ { $$ = FALSE; } ; Index: src/backend/parser/keywords.c =================================================================== RCS file: /cvsroot/pgsql-server/src/backend/parser/keywords.c,v retrieving revision 1.146 diff -c -r1.146 keywords.c *** src/backend/parser/keywords.c 9 Mar 2004 05:05:41 -0000 1.146 --- src/backend/parser/keywords.c 9 Mar 2004 12:42:37 -0000 *************** *** 213,218 **** --- 213,219 ---- {"nothing", NOTHING}, {"notify", NOTIFY}, {"notnull", NOTNULL}, + {"nowait", NOWAIT}, {"null", NULL_P}, {"nullif", NULLIF}, {"numeric", NUMERIC}, Index: src/include/access/heapam.h =================================================================== RCS file: /cvsroot/pgsql-server/src/include/access/heapam.h,v retrieving revision 1.86 diff -c -r1.86 heapam.h *** src/include/access/heapam.h 29 Nov 2003 22:40:55 -0000 1.86 --- src/include/access/heapam.h 9 Mar 2004 12:42:38 -0000 *************** *** 129,134 **** --- 129,135 ---- /* heapam.c */ extern Relation relation_open(Oid relationId, LOCKMODE lockmode); + extern Relation conditional_relation_open(Oid relationId, LOCKMODE lockmode, bool nowait); extern Relation relation_openrv(constRangeVar *relation, LOCKMODE lockmode); extern Relation relation_openr(const char *sysRelationName,LOCKMODE lockmode); extern void relation_close(Relation relation, LOCKMODE lockmode); Index: src/include/nodes/parsenodes.h =================================================================== RCS file: /cvsroot/pgsql-server/src/include/nodes/parsenodes.h,v retrieving revision 1.253 diff -c -r1.253 parsenodes.h *** src/include/nodes/parsenodes.h 14 Jan 2004 23:01:55 -0000 1.253 --- src/include/nodes/parsenodes.h 9 Mar 2004 12:42:39 -0000 *************** *** 1619,1624 **** --- 1619,1625 ---- NodeTag type; List *relations; /* relations to lock */ int mode; /* lock mode */ + bool nowait; /* no wait mode */ } LockStmt; /* ----------------------
Tatsuo Ishii <t-ishii@sra.co.jp> writes: > Here is the patch I promised (against current). This is missing the necessary adjustments in backend/nodes/ (copy and equal funcs). Also the NOWAIT keyword must be added to the list of nonreserved keywords near the bottom of gram.y. regards, tom lane
> Tatsuo Ishii <t-ishii@sra.co.jp> writes: > > Here is the patch I promised (against current). > > This is missing the necessary adjustments in backend/nodes/ (copy and > equal funcs). Also the NOWAIT keyword must be added to the list of > nonreserved keywords near the bottom of gram.y. Thanks for the review. I'll work on this. -- Tatsuo Ishii
> > This is missing the necessary adjustments in backend/nodes/ (copy and > > equal funcs). Also the NOWAIT keyword must be added to the list of > > nonreserved keywords near the bottom of gram.y. > > Thanks for the review. I'll work on this. Here is the revised patch. -- Tatsuo Ishii =================================================================== RCS file: /cvsroot/pgsql-server/doc/src/sgml/ref/lock.sgml,v retrieving revision 1.40 diff -c -r1.40 lock.sgml *** doc/src/sgml/ref/lock.sgml 14 Dec 2003 00:05:29 -0000 1.40 --- doc/src/sgml/ref/lock.sgml 10 Mar 2004 01:35:18 -0000 *************** *** 20,26 **** <refsynopsisdiv> <synopsis> ! LOCK [ TABLE ] <replaceable class="PARAMETER">name</replaceable> [, ...] [ IN <replaceable class="PARAMETER">lockmode</replaceable>MODE ] where <replaceable class="PARAMETER">lockmode</replaceable> is one of: --- 20,26 ---- <refsynopsisdiv> <synopsis> ! LOCK [ TABLE ] <replaceable class="PARAMETER">name</replaceable> [, ...] [ IN <replaceable class="PARAMETER">lockmode</replaceable>MODE ] [ NOWAIT ] where <replaceable class="PARAMETER">lockmode</replaceable> isone of: *************** *** 34,41 **** <para> <command>LOCK TABLE</command> obtains a table-level lock, waiting if ! necessary for any conflicting locks to be released. Once obtained, ! the lock is held for the remainder of the current transaction. (There is no <command>UNLOCK TABLE</command> command;locks are always released at transaction end.) </para> --- 34,43 ---- <para> <command>LOCK TABLE</command> obtains a table-level lock, waiting if ! necessary for any conflicting locks to be released. ! If <literal>NOWAIT</literal> is given, <command>LOCK TABLE</command> ! does not wait for acquiring lock, and throws an error instead. ! Once obtained, the lock is held for the remainder of the current transaction. (There is no <command>UNLOCK TABLE</command>command; locks are always released at transaction end.) </para> Index: src/backend/access/heap/heapam.c =================================================================== RCS file: /cvsroot/pgsql-server/src/backend/access/heap/heapam.c,v retrieving revision 1.162 diff -c -r1.162 heapam.c *** src/backend/access/heap/heapam.c 16 Jan 2004 20:51:30 -0000 1.162 --- src/backend/access/heap/heapam.c 10 Mar 2004 01:35:21 -0000 *************** *** 464,469 **** --- 464,496 ---- return r; } + Relation + conditional_relation_open(Oid relationId, LOCKMODE lockmode, bool nowait) + { + Relation r; + + Assert(lockmode >= NoLock && lockmode < MAX_LOCKMODES); + + /* The relcache does all the real work... */ + r = RelationIdGetRelation(relationId); + + if (!RelationIsValid(r)) + elog(ERROR, "could not open relation with OID %u", relationId); + + if (lockmode != NoLock) + { + if (nowait) + { + if (!ConditionalLockRelation(r, lockmode)) + elog(ERROR, "could not aquire relation lock"); + } + else + LockRelation(r, lockmode); + } + + return r; + } + /* ---------------- * relation_openrv - open any relation specified by a RangeVar * Index: src/backend/commands/lockcmds.c =================================================================== RCS file: /cvsroot/pgsql-server/src/backend/commands/lockcmds.c,v retrieving revision 1.8 diff -c -r1.8 lockcmds.c *** src/backend/commands/lockcmds.c 29 Nov 2003 19:51:47 -0000 1.8 --- src/backend/commands/lockcmds.c 10 Mar 2004 01:35:21 -0000 *************** *** 59,65 **** aclcheck_error(aclresult, ACL_KIND_CLASS, get_rel_name(reloid)); ! rel = relation_open(reloid, lockstmt->mode); /* Currently, we only allow plain tables to be locked */ if (rel->rd_rel->relkind != RELKIND_RELATION) --- 59,65 ---- aclcheck_error(aclresult, ACL_KIND_CLASS, get_rel_name(reloid)); ! rel = conditional_relation_open(reloid, lockstmt->mode, lockstmt->nowait); /* Currently, we only allowplain tables to be locked */ if (rel->rd_rel->relkind != RELKIND_RELATION) Index: src/backend/nodes/copyfuncs.c =================================================================== RCS file: /cvsroot/pgsql-server/src/backend/nodes/copyfuncs.c,v retrieving revision 1.277 diff -c -r1.277 copyfuncs.c *** src/backend/nodes/copyfuncs.c 14 Jan 2004 23:01:54 -0000 1.277 --- src/backend/nodes/copyfuncs.c 10 Mar 2004 01:35:25 -0000 *************** *** 2316,2321 **** --- 2316,2322 ---- COPY_NODE_FIELD(relations); COPY_SCALAR_FIELD(mode); + COPY_SCALAR_FIELD(nowait); return newnode; } Index: src/backend/nodes/equalfuncs.c =================================================================== RCS file: /cvsroot/pgsql-server/src/backend/nodes/equalfuncs.c,v retrieving revision 1.215 diff -c -r1.215 equalfuncs.c *** src/backend/nodes/equalfuncs.c 14 Jan 2004 23:01:55 -0000 1.215 --- src/backend/nodes/equalfuncs.c 10 Mar 2004 01:35:28 -0000 *************** *** 1252,1257 **** --- 1252,1258 ---- { COMPARE_NODE_FIELD(relations); COMPARE_SCALAR_FIELD(mode); + COMPARE_SCALAR_FIELD(nowait); return true; } Index: src/backend/parser/gram.y =================================================================== RCS file: /cvsroot/pgsql-server/src/backend/parser/gram.y,v retrieving revision 2.447 diff -c -r2.447 gram.y *** src/backend/parser/gram.y 9 Mar 2004 05:05:41 -0000 2.447 --- src/backend/parser/gram.y 10 Mar 2004 01:35:42 -0000 *************** *** 169,174 **** --- 169,175 ---- %type <ival> opt_lock lock_type cast_context %type <boolean> opt_force opt_or_replace transaction_access_mode opt_grant_grant_option opt_revoke_grant_option + opt_nowait %type <boolean> like_including_defaults *************** *** 375,381 **** MATCH MAXVALUE MINUTE_P MINVALUE MODE MONTH_P MOVE NAMES NATIONAL NATURAL NCHAR NEW NEXT NO NOCREATEDB ! NOCREATEUSER NONE NOT NOTHING NOTIFY NOTNULL NULL_P NULLIF NUMERIC OBJECT_P OF OFF OFFSET OIDS OLD ON ONLYOPERATOR OPTION OR --- 376,382 ---- MATCH MAXVALUE MINUTE_P MINVALUE MODE MONTH_P MOVE NAMES NATIONAL NATURAL NCHAR NEW NEXT NO NOCREATEDB ! NOCREATEUSER NONE NOT NOTHING NOTIFY NOTNULL NOWAIT NULL_P NULLIF NUMERIC OBJECT_P OF OFF OFFSET OIDS OLDON ONLY OPERATOR OPTION OR *************** *** 4347,4358 **** } ; ! LockStmt: LOCK_P opt_table qualified_name_list opt_lock { LockStmt *n = makeNode(LockStmt); n->relations = $3; n->mode = $4; $$ = (Node*)n; } ; --- 4348,4360 ---- } ; ! LockStmt: LOCK_P opt_table qualified_name_list opt_lock opt_nowait { LockStmt *n= makeNode(LockStmt); n->relations = $3; n->mode = $4; + n->nowait = $5; $$ = (Node *)n; } ; *************** *** 4371,4376 **** --- 4373,4382 ---- | ACCESS EXCLUSIVE { $$ = AccessExclusiveLock; } ; + opt_nowait: NOWAIT { $$ = TRUE; } + | /*EMPTY*/ { $$ = FALSE; } + ; + /***************************************************************************** * *************** *** 7683,7688 **** --- 7689,7695 ---- | LOCALTIMESTAMP | NEW | NOT + | NOWAIT | NULL_P | OFF | OFFSET Index: src/backend/parser/keywords.c =================================================================== RCS file: /cvsroot/pgsql-server/src/backend/parser/keywords.c,v retrieving revision 1.146 diff -c -r1.146 keywords.c *** src/backend/parser/keywords.c 9 Mar 2004 05:05:41 -0000 1.146 --- src/backend/parser/keywords.c 10 Mar 2004 01:35:42 -0000 *************** *** 213,218 **** --- 213,219 ---- {"nothing", NOTHING}, {"notify", NOTIFY}, {"notnull", NOTNULL}, + {"nowait", NOWAIT}, {"null", NULL_P}, {"nullif", NULLIF}, {"numeric", NUMERIC}, Index: src/include/access/heapam.h =================================================================== RCS file: /cvsroot/pgsql-server/src/include/access/heapam.h,v retrieving revision 1.86 diff -c -r1.86 heapam.h *** src/include/access/heapam.h 29 Nov 2003 22:40:55 -0000 1.86 --- src/include/access/heapam.h 10 Mar 2004 01:35:46 -0000 *************** *** 129,134 **** --- 129,135 ---- /* heapam.c */ extern Relation relation_open(Oid relationId, LOCKMODE lockmode); + extern Relation conditional_relation_open(Oid relationId, LOCKMODE lockmode, bool nowait); extern Relation relation_openrv(constRangeVar *relation, LOCKMODE lockmode); extern Relation relation_openr(const char *sysRelationName,LOCKMODE lockmode); extern void relation_close(Relation relation, LOCKMODE lockmode); Index: src/include/nodes/parsenodes.h =================================================================== RCS file: /cvsroot/pgsql-server/src/include/nodes/parsenodes.h,v retrieving revision 1.253 diff -c -r1.253 parsenodes.h *** src/include/nodes/parsenodes.h 14 Jan 2004 23:01:55 -0000 1.253 --- src/include/nodes/parsenodes.h 10 Mar 2004 01:35:49 -0000 *************** *** 1619,1624 **** --- 1619,1625 ---- NodeTag type; List *relations; /* relations to lock */ int mode; /* lock mode */ + bool nowait; /* no wait mode */ } LockStmt; /* ----------------------
> > > This is missing the necessary adjustments in backend/nodes/ (copy and > > > equal funcs). Also the NOWAIT keyword must be added to the list of > > > nonreserved keywords near the bottom of gram.y. > > > > Thanks for the review. I'll work on this. Patches committed. -- Tatsuo Ishii