Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-29 Thread Michael Paquier
On Fri, Nov 30, 2012 at 4:43 AM, Andres Freund wrote: > On 2012-11-29 11:53:50 -0500, Tom Lane wrote: > > And here is a version for 9.1. This omits the code changes directly > > relevant to DROP INDEX CONCURRENTLY, but includes the changes to avoid > > transactional updates of the pg_index row du

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-29 Thread Andres Freund
On 2012-11-29 11:53:50 -0500, Tom Lane wrote: > And here is a version for 9.1. This omits the code changes directly > relevant to DROP INDEX CONCURRENTLY, but includes the changes to avoid > transactional updates of the pg_index row during CREATE CONCURRENTLY, > as well as the changes to prevent u

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-29 Thread Tom Lane
And here is a version for 9.1. This omits the code changes directly relevant to DROP INDEX CONCURRENTLY, but includes the changes to avoid transactional updates of the pg_index row during CREATE CONCURRENTLY, as well as the changes to prevent use of not-valid or not-ready indexes in places where i

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-29 Thread Amit Kapila
On Thursday, November 29, 2012 4:24 PM Andres Freund wrote: > On 2012-11-29 16:18:07 +0530, Amit Kapila wrote: > > On Thursday, November 29, 2012 12:39 AM Tom Lane wrote. > > > Andres Freund writes: > > > > On 2012-11-27 23:46:58 -0500, Tom Lane wrote: > > > >> Attached is a very preliminary draft

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-29 Thread Andres Freund
On 2012-11-29 16:18:07 +0530, Amit Kapila wrote: > On Thursday, November 29, 2012 12:39 AM Tom Lane wrote. > > Andres Freund writes: > > > On 2012-11-27 23:46:58 -0500, Tom Lane wrote: > > >> Attached is a very preliminary draft patch for this. I've not > > >> addressed the question of whether we

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-29 Thread Amit Kapila
On Thursday, November 29, 2012 12:39 AM Tom Lane wrote. > Andres Freund writes: > > On 2012-11-27 23:46:58 -0500, Tom Lane wrote: > >> Attached is a very preliminary draft patch for this. I've not > >> addressed the question of whether we can clear indcheckxmin during > >> transactional updates o

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-28 Thread Pavan Deolasee
On Thu, Nov 29, 2012 at 10:10 AM, Tom Lane wrote: > I wrote: > > Attached is an updated patch for HEAD that I think is about ready to go. > > I'll start making a back-patchable version shortly. > > Here is an only-lightly-tested version for 9.2. > > Looks good at a glance. I wonder though if it w

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-28 Thread Michael Paquier
On 2012/11/29, at 9:23, Andres Freund wrote: > On 2012-11-28 19:11:46 -0500, Tom Lane wrote: >> Andres Freund writes: >>> On 2012-11-28 18:41:39 -0500, Tom Lane wrote: However, this is more complicated and harder to understand. So unless somebody is really excited about being able to

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-28 Thread Tom Lane
Andres Freund writes: > On 2012-11-29 09:10:22 +0900, Michael Paquier wrote: >> and is going to need a lot of rework as well as more infrastructure >> like a better MVCC-ish SnapshotNow. > Which is a major project in itself. I wonder whether my crazy "follow > updates via t_ctid isn't the actuall

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-28 Thread Andres Freund
On 2012-11-28 19:11:46 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2012-11-28 18:41:39 -0500, Tom Lane wrote: > >> However, this is more complicated and harder to understand. So unless > >> somebody is really excited about being able to tell the difference > >> between create-in-progres

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-28 Thread Andres Freund
On 2012-11-29 09:10:22 +0900, Michael Paquier wrote: > On Thu, Nov 29, 2012 at 8:52 AM, Andres Freund wrote: > > > On 2012-11-28 18:41:39 -0500, Tom Lane wrote: > > > Andres Freund writes: > > > > On 2012-11-28 17:42:18 -0500, Tom Lane wrote: > > > >> I agree it's a judgment call, though. Anybod

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-28 Thread Tom Lane
Andres Freund writes: > On 2012-11-28 18:41:39 -0500, Tom Lane wrote: >> However, this is more complicated and harder to understand. So unless >> somebody is really excited about being able to tell the difference >> between create-in-progress and drop-in-progress, I'd rather leave the >> patch as

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-28 Thread Andres Freund
On 2012-11-28 18:41:39 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2012-11-28 17:42:18 -0500, Tom Lane wrote: > >> I agree it's a judgment call, though. Anybody want to argue for the > >> other position? > > > Hm. Seems odd to include indexes that are being dropped concurrently at > > t

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-28 Thread Tom Lane
Andres Freund writes: > On 2012-11-28 17:42:18 -0500, Tom Lane wrote: >> I agree it's a judgment call, though. Anybody want to argue for the >> other position? > Hm. Seems odd to include indexes that are being dropped concurrently at > that moment. But then, we can't really detect that situation

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-28 Thread Andres Freund
On 2012-11-28 17:42:18 -0500, Tom Lane wrote: > Andres Freund writes: > > One minor thing I haven't noticed earlier: Perhaps we should also skip > > over invalid indexes in transformTableLikeClause's > > CREATE_TABLE_LIKE_INDEXES case? > > I left that as-is intentionally: the fact that an index is

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-28 Thread Tom Lane
Andres Freund writes: > One minor thing I haven't noticed earlier: Perhaps we should also skip > over invalid indexes in transformTableLikeClause's > CREATE_TABLE_LIKE_INDEXES case? I left that as-is intentionally: the fact that an index isn't valid doesn't prevent us from cloning it. A relevant

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-28 Thread Andres Freund
On 2012-11-28 14:09:11 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2012-11-27 23:46:58 -0500, Tom Lane wrote: > >> Attached is a very preliminary draft patch for this. I've not addressed > >> the question of whether we can clear indcheckxmin during transactional > >> updates of pg_index

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-28 Thread Andres Freund
On 2012-11-27 23:46:58 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2012-11-27 16:31:15 -0500, Tom Lane wrote: > >> Andres Freund writes: > >>> Isn't inisprimary updated when an ALTER TABLE ... ADD PRIMARY KEY > >>> ... USING someindex ; is done? Also I think indoption might be written >

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-27 Thread Tom Lane
Andres Freund writes: > On 2012-11-27 16:31:15 -0500, Tom Lane wrote: >> Andres Freund writes: >>> Isn't inisprimary updated when an ALTER TABLE ... ADD PRIMARY KEY >>> ... USING someindex ; is done? Also I think indoption might be written >>> to as well. >> Ugh, you're right. Somebody wasn't p

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-27 Thread Andres Freund
On 2012-11-27 16:31:15 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2012-11-27 14:41:34 -0500, Tom Lane wrote: > >> The stuff you are allowed to ALTER is pretty much > >> irrelevant to the index's life as an index. > > > Isn't inisprimary updated when an ALTER TABLE ... ADD PRIMARY KEY >

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-27 Thread Tom Lane
Andres Freund writes: > On 2012-11-27 14:41:34 -0500, Tom Lane wrote: >> The stuff you are allowed to ALTER is pretty much >> irrelevant to the index's life as an index. > Isn't inisprimary updated when an ALTER TABLE ... ADD PRIMARY KEY > ... USING someindex ; is done? Also I think indoption mig

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-27 Thread Andres Freund
On 2012-11-27 14:41:34 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2012-11-27 13:45:08 -0500, Tom Lane wrote: > >> In short, all flag changes in pg_index should be done by > >> update-in-place, and the tuple's xmin will never change from the > >> original creating transaction (until froz

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-27 Thread Tom Lane
Andres Freund writes: > On 2012-11-27 13:45:08 -0500, Tom Lane wrote: >> In short, all flag changes in pg_index should be done by >> update-in-place, and the tuple's xmin will never change from the >> original creating transaction (until frozen). > Hm. That doesn't sound that easy to guarantee. S

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-27 Thread Andres Freund
On 2012-11-27 13:45:08 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2012-11-26 22:44:49 -0500, Tom Lane wrote: > >> I looked through the code a bit, and I think we should be able to make > >> this work, but note there are also places that update indcheckxmin using > >> heap_update, and th

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-27 Thread Tom Lane
Andres Freund writes: > On 2012-11-26 22:44:49 -0500, Tom Lane wrote: >> I looked through the code a bit, and I think we should be able to make >> this work, but note there are also places that update indcheckxmin using >> heap_update, and that's just as dangerous as changing the other two >> flag

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-27 Thread Andres Freund
On 2012-11-26 22:44:49 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2012-10-05 19:56:40 -0400, Tom Lane wrote: > >> I think this could possibly be fixed by using nontransactional > >> update-in-place when we're trying to change indisvalid and/or > >> indisready, but I've not really though

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-26 Thread Tom Lane
Andres Freund writes: > On 2012-10-05 19:56:40 -0400, Tom Lane wrote: >> I think this could possibly be fixed by using nontransactional >> update-in-place when we're trying to change indisvalid and/or >> indisready, but I've not really thought through the details. > I couldn't really think of any

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-11-24 Thread Andres Freund
On 2012-10-05 19:56:40 -0400, Tom Lane wrote: > 1. These operations think they can use ordinary heap_update operations > to change pg_index entries when they don't have exclusive lock on the > parent table. The lack of ex-lock means that another backend could be > currently loading up its list of

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-10-21 Thread Kevin Grittner
Kevin Grittner wrote: > Tom Lane wrote: > > This needs to be back-patched, no? > > Looking at that now. Back-patched to 9.2.  I don't know how I got it in my head that this was a pending 9.3 feature.  I'll check next time, even if I think I know. Thanks to both Andres and Tom for pointing that o

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-10-21 Thread Kevin Grittner
Tom Lane wrote: > This needs to be back-patched, no? Looking at that now. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-10-21 Thread Tom Lane
"Kevin Grittner" writes: > Kevin Grittner wrote: >> Will apply tomorrow if there are no further objections. > Done. This needs to be back-patched, no? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subsc

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-10-21 Thread Kevin Grittner
Kevin Grittner wrote: > Will apply tomorrow if there are no further objections. Done. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-10-20 Thread Kevin Grittner
Simon Riggs wrote: > Kevin, you're good to go on the SSI patch, or I'll apply next week > if you don't. Thanks for that. There were some hunks failing because of minor improvements to the comments you applied, so attached is a version with trivial adjustments for that.  Will apply tomorrow if ther

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-10-19 Thread Simon Riggs
On 18 October 2012 19:48, Simon Riggs wrote: > On 18 October 2012 10:20, Andres Freund wrote: >> On Thursday, October 18, 2012 06:12:02 AM Kevin Grittner wrote: >>> Kevin Grittner wrote: >>> > Hmm. The comment is probably better now, but I've been re-checking >>> > the code, and I think my actual

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-10-18 Thread Simon Riggs
On 18 October 2012 10:20, Andres Freund wrote: > On Thursday, October 18, 2012 06:12:02 AM Kevin Grittner wrote: >> Kevin Grittner wrote: >> > Hmm. The comment is probably better now, but I've been re-checking >> > the code, and I think my actual code change is completely wrong. >> > Give me a bit

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-10-18 Thread Kevin Grittner
Andres Freund wrote: > On Thursday, October 18, 2012 06:12:02 AM Kevin Grittner wrote: >> I'm having trouble seeing a way to make this work without >> rearranging the code for concurrent drop to get to a state where >> it has set indisvalid = false, made that visible to all processes, >> and ensu

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-10-18 Thread Andres Freund
On Thursday, October 18, 2012 06:12:02 AM Kevin Grittner wrote: > Kevin Grittner wrote: > > Hmm. The comment is probably better now, but I've been re-checking > > the code, and I think my actual code change is completely wrong. > > Give me a bit to sort this out. > > I'm having trouble seeing a wa

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-10-17 Thread Kevin Grittner
Kevin Grittner wrote: > Hmm. The comment is probably better now, but I've been re-checking > the code, and I think my actual code change is completely wrong. > Give me a bit to sort this out. I'm having trouble seeing a way to make this work without rearranging the code for concurrent drop to get

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-10-17 Thread Kevin Grittner
Kevin Grittner wrote: > It took me a while to spot it, but yeah -- I reversed the field > names in the comment. :-( The email was right; I fixed the comment. Hmm. The comment is probably better now, but I've been re-checking the code, and I think my actual code change is completely wrong. Give me

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-10-17 Thread Kevin Grittner
Tom Lane wrote: > "Kevin Grittner" writes: >> To put that another way, it should be done at a time when it is >> sure that no query sees indisvalid = true and no query has yet >> seen indisready = false. Patch attached. Will apply if nobody sees >> a problem with it. > > The above statement of th

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-10-17 Thread Tom Lane
"Kevin Grittner" writes: > To put that another way, it should be done at a time when it is sure > that no query sees indisvalid = true and no query has yet seen > indisready = false.  Patch attached.  Will apply if nobody sees a > problem with it. The above statement of the requirement doesn't

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-10-17 Thread Kevin Grittner
Kevin Grittner wrote: > Simon Riggs wrote: >> On 6 October 2012 00:56, Tom Lane wrote: >   >>> 2. DROP INDEX CONCURRENTLY doesn't bother to do >>> TransferPredicateLocksToHeapRelation until long after it's >>> invalidated the index. Surely that's no good? Is it even possible >>> to do that correct

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-10-17 Thread Kevin Grittner
Simon Riggs wrote: > On 6 October 2012 00:56, Tom Lane wrote: >> 2. DROP INDEX CONCURRENTLY doesn't bother to do >> TransferPredicateLocksToHeapRelation until long after it's >> invalidated the index. Surely that's no good? Is it even possible >> to do that correctly, when we don't have a lock t

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-10-09 Thread Michael Paquier
On Tue, Oct 9, 2012 at 6:11 PM, Simon Riggs wrote: > On 6 October 2012 00:56, Tom Lane wrote: > > 1. These operations think they can use ordinary heap_update operations > > to change pg_index entries when they don't have exclusive lock on the > > parent table. The lack of ex-lock means that an

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-10-09 Thread Simon Riggs
On 6 October 2012 00:56, Tom Lane wrote: > 1. These operations think they can use ordinary heap_update operations > to change pg_index entries when they don't have exclusive lock on the > parent table. The lack of ex-lock means that another backend could be > currently loading up its list of ind

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-10-07 Thread Greg Stark
On Sat, Oct 6, 2012 at 12:56 AM, Tom Lane wrote: > 1. These operations think they can use ordinary heap_update operations > to change pg_index entries when they don't have exclusive lock on the > parent table. I wonder if we need a manual that lists exhaustively what operations can be taken with

Re: [HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-10-06 Thread Simon Riggs
On 6 October 2012 00:56, Tom Lane wrote: > 1. These operations think they can use ordinary heap_update operations > to change pg_index entries when they don't have exclusive lock on the > parent table. The lack of ex-lock means that another backend could be > currently loading up its list of ind

[HACKERS] Bugs in CREATE/DROP INDEX CONCURRENTLY

2012-10-05 Thread Tom Lane
1. These operations think they can use ordinary heap_update operations to change pg_index entries when they don't have exclusive lock on the parent table. The lack of ex-lock means that another backend could be currently loading up its list of index OIDs for the table --- and since it scans pg_in