Re: POC: converting Lists into arrays

Hi,

On 2019-02-25 13:59:36 -0500, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > On Mon, Feb 25, 2019 at 1:17 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> I'm not following your point here.  If we change key data structures
> >> (i.e. parsetrees, plan trees, execution trees) to use some other list-ish
> >> API, that *in itself* breaks everything that accesses those data
> >> structures.  The approach I propose here isn't zero-breakage, but it
> >> requires far fewer places to be touched than a complete API replacement
> >> would do.
> 
> > Sure, but if you have third-party code that touches those things,
> > it'll fail to compile.  With your proposed approach, there seems to be
> > a risk that it will compile but not work.
> 
> Failing to compile isn't really a benefit IMO.

It's a huge benefit. It's a lot of effort to look through all source
code for potential breakages. Especially if all list usages, rather than
some planner detail that comparatively few extensions touch, needs to be
audited.


> >> I completely disagree.  Your proposal is probably an order of magnitude
> >> more painful than the approach I suggest here, while not really offering
> >> any additional performance benefit (or if you think there would be some,
> >> you haven't explained how).  Strictly on cost/benefit grounds, it isn't
> >> ever going to happen that way.
> 
> > Why would it be ten times more painful, exactly?
> 
> Because it involves touching ten times more code (and that's a very
> conservative estimate).  Excluding changes in pg_list.h + list.c,
> what I posted touches approximately 600 lines of code (520 insertions,
> 644 deletions to be exact).  For comparison's sake, there are about
> 1800 uses of foreach in the tree, each of which would require at least
> 3 changes to replace (the foreach itself, the ListCell variable
> declaration, and at least one lfirst() reference in the loop body).
> So we've already blown past 5000 lines worth of changes if we want to
> do it another way ... and that's just *one* component of the List API.
> Nor is there any reason to think the changes would be any more mechanical
> than what I had to do here.  (No fair saying that I already found the
> trouble spots, either.  A different implementation would likely break
> assumptions in different ways.)

FWIW, rewrites of this kind can be quite nicely automated using
coccinelle [1]. One sometimes needs to do a bit of mop-up with variable
names, but otherwise it should be mostly complete.

Greetings,

Andres Freund

[1] http://coccinelle.lip6.fr/