Обсуждение: Moving ExecInsertIndexTuples and friends to new file
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
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
* 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
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
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
Вложения
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