Moving ExecInsertIndexTuples and friends to new file

Started by Heikki Linnakangasabout 11 years ago6 messageshackers
Jump to latest
#1Heikki Linnakangas
heikki.linnakangas@enterprisedb.com

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

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
heikki.linnakangas@enterprisedb.com
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
heikki.linnakangas@enterprisedb.com
In reply to: Heikki Linnakangas (#4)
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+52-7
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