Moving ExecInsertIndexTuples and friends to new file

Started by Heikki Linnakangasover 10 years ago6 messages
#1Heikki Linnakangas
hlinnaka@iki.fi

While looking at Peter's INSERT ... ON CONFLICT patch, I started to feel
that ExecInsertIndexTuples() and friends would deserve a file of their
own, and not be buried in the middle of execUtils.c. I propose that we
split execUtils.c into two, moving ExecOpenIndices(), ExecCloseIndices()
ExecInsertIndexTuples() and related functions into a new file called
executor/execIndexing.c.

Moving functions makes backpatching harder, so it's not something to be
done lightly, but I think it would be worth it in this case. There have
been few changes to those functions in years, so I doubt they'll need
much backpatching in the near future either.

For comparison, this is what the file sizes of executor/exec*.c will
look like after the split:

-rw-r--r-- 1 heikki heikki 14710 Apr 22 09:07 execAmi.c
-rw-r--r-- 1 heikki heikki 9711 Apr 22 09:07 execCurrent.c
-rw-r--r-- 1 heikki heikki 16659 Apr 22 09:07 execGrouping.c
-rw-r--r-- 1 heikki heikki 16023 Apr 23 21:57 execIndexing.c
-rw-r--r-- 1 heikki heikki 8276 Apr 22 09:07 execJunk.c
-rw-r--r-- 1 heikki heikki 80102 Apr 22 09:07 execMain.c
-rw-r--r-- 1 heikki heikki 18694 Apr 22 09:07 execProcnode.c
-rw-r--r-- 1 heikki heikki 160700 Apr 22 09:07 execQual.c
-rw-r--r-- 1 heikki heikki 9957 Apr 22 09:07 execScan.c
-rw-r--r-- 1 heikki heikki 37796 Apr 22 09:07 execTuples.c
-rw-r--r-- 1 heikki heikki 28004 Apr 23 21:50 execUtils.c

Any objections? I'm planning to do this as a separate commit, before the
INSERT ... ON CONFLICT patch goes in.

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Peter Geoghegan
pg@heroku.com
In reply to: Heikki Linnakangas (#1)
Re: Moving ExecInsertIndexTuples and friends to new file

On Thu, Apr 23, 2015 at 12:05 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

While looking at Peter's INSERT ... ON CONFLICT patch, I started to feel
that ExecInsertIndexTuples() and friends would deserve a file of their own,
and not be buried in the middle of execUtils.c. I propose that we split
execUtils.c into two, moving ExecOpenIndices(), ExecCloseIndices()
ExecInsertIndexTuples() and related functions into a new file called
executor/execIndexing.c.

That split makes a lot of sense to me.

--
Peter Geoghegan

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Stephen Frost
sfrost@snowman.net
In reply to: Peter Geoghegan (#2)
Re: Moving ExecInsertIndexTuples and friends to new file

* Peter Geoghegan (pg@heroku.com) wrote:

On Thu, Apr 23, 2015 at 12:05 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

While looking at Peter's INSERT ... ON CONFLICT patch, I started to feel
that ExecInsertIndexTuples() and friends would deserve a file of their own,
and not be buried in the middle of execUtils.c. I propose that we split
execUtils.c into two, moving ExecOpenIndices(), ExecCloseIndices()
ExecInsertIndexTuples() and related functions into a new file called
executor/execIndexing.c.

That split makes a lot of sense to me.

No objections here.

Thanks!

Stephen

#4Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Stephen Frost (#3)
Re: Moving ExecInsertIndexTuples and friends to new file

On 04/24/2015 06:30 AM, Stephen Frost wrote:

* Peter Geoghegan (pg@heroku.com) wrote:

On Thu, Apr 23, 2015 at 12:05 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

While looking at Peter's INSERT ... ON CONFLICT patch, I started to feel
that ExecInsertIndexTuples() and friends would deserve a file of their own,
and not be buried in the middle of execUtils.c. I propose that we split
execUtils.c into two, moving ExecOpenIndices(), ExecCloseIndices()
ExecInsertIndexTuples() and related functions into a new file called
executor/execIndexing.c.

That split makes a lot of sense to me.

No objections here.

Ok, moved.

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Heikki Linnakangas (#4)
1 attachment(s)
Re: Moving ExecInsertIndexTuples and friends to new file

On 04/24/2015 09:36 AM, Heikki Linnakangas wrote:

On 04/24/2015 06:30 AM, Stephen Frost wrote:

* Peter Geoghegan (pg@heroku.com) wrote:

On Thu, Apr 23, 2015 at 12:05 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

While looking at Peter's INSERT ... ON CONFLICT patch, I started to feel
that ExecInsertIndexTuples() and friends would deserve a file of their own,
and not be buried in the middle of execUtils.c. I propose that we split
execUtils.c into two, moving ExecOpenIndices(), ExecCloseIndices()
ExecInsertIndexTuples() and related functions into a new file called
executor/execIndexing.c.

That split makes a lot of sense to me.

No objections here.

Ok, moved.

I wrote a little overview text on how unique and exclusion constraints
are enforced. Most of the information can be gleaned from comments
elsewhere, but I think it's helpful to have it in one place. Makes it
easier to compare how unique and exclusion constraints work. The
harmless deadlocks with exclusion constraints are not explained
elsewhere AFAICS.

This is also in preparation for Peter's INSERT ON CONFLICT patch. That
will add another section explaining how the deadlocks and livelocks are
avoided. That's easier to understand after you grok the potential for
deadlocks with exclusion constraints.

This also removes a comment from 1989 claiming that the code should be
moved elsewhere. I think the code is in the right place.

- Heikki

Attachments:

how-unique-and-exclusion-constraints-are-enforced.patchapplication/x-patch; name=how-unique-and-exclusion-constraints-are-enforced.patchDownload
diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c
index de8fcdb..a697682 100644
--- a/src/backend/executor/execIndexing.c
+++ b/src/backend/executor/execIndexing.c
@@ -1,7 +1,55 @@
 /*-------------------------------------------------------------------------
  *
  * execIndexing.c
- *	  executor support for maintaining indexes
+ *	  routines for inserting index tuples and enforcing unique and
+ *	  exclusive constraints.
+ *
+ * ExecInsertIndexTuples() is the main entry point.  It's called after
+ * inserting a tuple to the heap, and it inserts corresponding index tuples
+ * into all indexes.  At the same time, it enforces any unique and
+ * exclusion constraints:
+ *
+ * Unique Indexes
+ * --------------
+ *
+ * Enforcing a unique constraint is straightforward.  When the index AM
+ * inserts the tuple to the index, it also checks that there are no
+ * conflicting tuples in the index already.  It does so atomically, so that
+ * even if two backends try to insert the same key concurrently, only one
+ * of them will succeed.  All the logic to ensure atomicity, and to wait
+ * for in-progress transactions to finish, is handled by the index AM.
+ *
+ * If a unique constraint is deferred, we request the index AM to not
+ * throw an error if a conflict is found.  Instead, we make note that there
+ * was a conflict and return the list of indexes with conflicts to the
+ * caller.  The caller must re-check them later, by calling index_insert()
+ * with the UNIQUE_CHECK_EXISTING option.
+ *
+ * Exclusion Constraints
+ * ---------------------
+ *
+ * Exclusion constraints are different from unique indexes in that when the
+ * tuple is inserted to the index, the index AM does not check for
+ * duplicate keys at the same time.  After the insertion, we perform a
+ * separate scan on the index to check for conflicting tuples, and if one
+ * is found, we throw an error and the transaction is aborted.  If the
+ * conflicting tuple's inserter or deleter is in-progress, we wait for it
+ * to finish first.
+ *
+ * There is a chance of deadlock, if two backends insert a tuple at the
+ * same time, and then perform the scan to check for conflicts.  They will
+ * find each other's tuple, and both try to wait for each other.  The
+ * deadlock detector will detect that, and abort one of the transactions.
+ * That's fairly harmless, as one of them was bound to abort with a
+ * "duplicate key error" anyway, although you get a different error
+ * message.
+ *
+ * If an exclusion constraint is deferred, we still perform the conflict
+ * checking scan immediately after inserting the index tuple.  But instead
+ * of throwing an error if a conflict is found, we return that information
+ * to the caller.  The caller must re-check them later by calling
+ * check_exclusion_constraint().
+ *
  *
  * Portions Copyright (c) 1996-2015, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
@@ -134,13 +182,10 @@ ExecCloseIndices(ResultRelInfo *resultRelInfo)
  *		This routine takes care of inserting index tuples
  *		into all the relations indexing the result relation
  *		when a heap tuple is inserted into the result relation.
- *		Much of this code should be moved into the genam
- *		stuff as it only exists here because the genam stuff
- *		doesn't provide the functionality needed by the
- *		executor.. -cim 9/27/89
  *
- *		This returns a list of index OIDs for any unique or exclusion
- *		constraints that are deferred and that had
+ *		Unique and exclusion constraints are enforced at the same
+ *		time.  This returns a list of index OIDs for any unique or
+ *		exclusion constraints that are deferred and that had
  *		potential (unconfirmed) conflicts.
  *
  *		CAUTION: this must not be called for a HOT update.
#6Peter Geoghegan
pg@heroku.com
In reply to: Heikki Linnakangas (#5)
Re: Moving ExecInsertIndexTuples and friends to new file

On Fri, Apr 24, 2015 at 6:38 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

I wrote a little overview text on how unique and exclusion constraints are
enforced. Most of the information can be gleaned from comments elsewhere,
but I think it's helpful to have it in one place. Makes it easier to compare
how unique and exclusion constraints work. The harmless deadlocks with
exclusion constraints are not explained elsewhere AFAICS.

FWIW, both Jeff Davis and Tom Lane were well aware of this issue back
when exclusion constraints went in - it was judged to be acceptable at
the time, which I agree with. I happened to discuss this with Jeff in
New York recently. I agree that it should definitely be documented
like this (and the fact that ordinary unique indexes are unaffected,
too).

This is also in preparation for Peter's INSERT ON CONFLICT patch. That will
add another section explaining how the deadlocks and livelocks are avoided.
That's easier to understand after you grok the potential for deadlocks with
exclusion constraints.

Makes sense.

This also removes a comment from 1989 claiming that the code should be moved
elsewhere. I think the code is in the right place.

+1

--
Peter Geoghegan

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers