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 and...@2ndquadrant.com 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

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 and...@2ndquadrant.com 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

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 and...@2ndquadrant.com writes: On 2012-11-27 23:46:58 -0500, Tom Lane wrote: Attached is a very

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

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 use

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 and...@2ndquadrant.comwrote: 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

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 and...@2ndquadrant.com writes: On 2012-11-27 16:31:15 -0500, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: Isn't inisprimary updated when an ALTER TABLE ... ADD PRIMARY KEY ... USING someindex ; is done? Also I

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 and...@2ndquadrant.com 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

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

2012-11-28 Thread Tom Lane
Andres Freund and...@2ndquadrant.com 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

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 and...@2ndquadrant.com 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

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

2012-11-28 Thread Tom Lane
Andres Freund and...@anarazel.de 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

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 and...@anarazel.de 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

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

2012-11-28 Thread Tom Lane
Andres Freund and...@2ndquadrant.com 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

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 and...@2ndquadrant.comwrote: On 2012-11-28 18:41:39 -0500, Tom Lane wrote: Andres Freund and...@anarazel.de writes: On 2012-11-28 17:42:18 -0500, Tom Lane wrote: I agree it's a

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 and...@2ndquadrant.com 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

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

2012-11-28 Thread Tom Lane
Andres Freund and...@2ndquadrant.com 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

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

2012-11-28 Thread Michael Paquier
On 2012/11/29, at 9:23, Andres Freund and...@2ndquadrant.com wrote: On 2012-11-28 19:11:46 -0500, Tom Lane wrote: Andres Freund and...@2ndquadrant.com 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

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 t...@sss.pgh.pa.us 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

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 and...@2ndquadrant.com 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

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

2012-11-27 Thread Tom Lane
Andres Freund and...@2ndquadrant.com 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

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 and...@2ndquadrant.com 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

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

2012-11-27 Thread Tom Lane
Andres Freund and...@2ndquadrant.com 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

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 and...@2ndquadrant.com 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

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

2012-11-27 Thread Tom Lane
Andres Freund and...@2ndquadrant.com 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

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 and...@2ndquadrant.com 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

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

2012-11-27 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes: On 2012-11-27 16:31:15 -0500, Tom Lane wrote: Andres Freund and...@2ndquadrant.com 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,

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

2012-11-26 Thread Tom Lane
Andres Freund and...@2ndquadrant.com 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

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: 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-21 Thread Tom Lane
Kevin Grittner kgri...@mail.com 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

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 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 out.

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 there

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

2012-10-19 Thread Simon Riggs
On 18 October 2012 19:48, Simon Riggs si...@2ndquadrant.com wrote: On 18 October 2012 10:20, Andres Freund and...@2ndquadrant.com 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

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 way to

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 ensured that

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

2012-10-18 Thread Simon Riggs
On 18 October 2012 10:20, Andres Freund and...@2ndquadrant.com 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.

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 t...@sss.pgh.pa.us 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

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 t...@sss.pgh.pa.us 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

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

2012-10-17 Thread Tom Lane
Kevin Grittner kgri...@mail.com 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

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
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 to

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

2012-10-09 Thread Simon Riggs
On 6 October 2012 00:56, Tom Lane t...@sss.pgh.pa.us 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

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 t...@sss.pgh.pa.us 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

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

2012-10-06 Thread Simon Riggs
On 6 October 2012 00:56, Tom Lane t...@sss.pgh.pa.us 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

[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