Re: next draft of list rewrite - Mailing list pgsql-patches

Tom Lane <tgl@sss.pgh.pa.us> writes:
> I thought the plan was to keep the API exactly the same as before,
> with the single exception of having to use ListCell* rather than
> List* as the type of pointers used to iterate through a list.

I thought that once we had decided to change the API, anything was
fair game: either we need to change every call site of the API (which
would be required if we adopted ListCell* as the type of the foreach
iteration variable), or we don't.

> It will undoubtedly break a fair amount of user-written code that we
> don't have control over

Granted, but so will changing the API at all: as above, the ListCell*
change would cause approximately the same amount of breakage for
users.

> Everyone who works on the backend is already accustomed to the
> existing names, which are mostly derived from ancient Lisp
> tradition.

The heritage of the names was part of my reason for changing them: the
Lisp names imply a certain style of list implementation (cons cells,
linear-time length and append ops but constant time prepend) that is
no longer being used. In addition, that makes names like lfirst()
(which would now be applied to a ListCell*) no longer very meaningful.

In addition, there are plenty of infelicities that it would be nice to
remove from the API. For example:

       - names like lispRemove() that aren't very meaningful (AFAICS),
         even in the present implementation

       - some names use case to separate words (e.g. LispRemove(),
         freeList()), some use underscores (e.g. set_union()), while
         some use nothing (e.g. llastnode())

       - some names begin with capital letters (e.g. FastAppend()),
         the rest do not

       - some names use a 'ptr/int/oid' prefix to denote type-specific
         variants of a function, whereas some use a 'i' or 'o' suffix
         for the same purpose.

       - some names are prefixed with 'l', some are not -- without
         apparently rhyme or reason for this distinction

       - some functions take the List* as their first argument
         (e.g. lappend()), some do not (e.g. nth()) -- again, without
         any good reason for the inconsistency that I could see

> If you want to push forward on it, we had better have a vote in
> pghackers, rather than assuming everyone concerned will see this
> discussion in -patches.

I'm still open to being convinced, BTW: I only write the code as it is
without prior discussion because I misunderstood your earlier
position. I'll send a mail to hackers if we're still in disagreement
at the end of this thread.

> I noticed one trivial bug, which is that the Asserts in
> list_member_int and list_member_oid seem backwards.

Thanks: Alvaro was already kind enough to point that out via email.

> A bigger problem is that list_length, list_head, and list_tail must
> *not* be macros --- we cannot afford double-evaluation bugs
> associated with operations as widely used as these.

Fair enough. I'll also take a look at GCC-style inline functions...

> No, you'd simply leave that cell-space wasted if the head member
> were to change.  It's doable, though it would complicate the
> deletion routine and probably nconc as well.  It's entirely likely
> that it'd be more complexity than it's worth, though, so if you
> don't want to do it that's fine with me.  Certainly it makes sense
> not to do it in the first pass.

Ok, I won't worry about it for now.

> [ a scheme for iteratively committing the changes ]
> This is a lot more pleasant way to proceed than a "big bang"
> changeover.

I agree that your method is easier procedurally, but ISTM that the
aggregate amount of work required is about the same: we still need to
change pretty much every call site of the list API. Yes, it is
slightly easier if we can do this one call site at a time, and yes,
it's slightly easier if s/List/ListCell/ is the only required change,
but there is about the same degree of work involved either
way -- and IMHO the benefit of a well named API is worth the
short-term pain.

Furthermore, it is quite possible to reduce the pain induced by my
method. For example, we could create a private CVS branch. In that
branch, it wouldn't matter if the tree is broken for a week or two, in
which time we can make the changes across the tree at our leisure, and
then merge them into HEAD in one relatively painless branch landing.

-Neil