Internal state in sequence.c is a bad idea

From a recent bug report:

test72=# create sequence s;
CREATE
test72=# begin;
BEGIN
test72=# select nextval('s');nextval
---------      1
(1 row)

test72=# drop sequence s;
DROP
test72=# end;
NOTICE:  LockRelease: no such lock
COMMIT

The NOTICE is a symptom of bad problems underneath (memory corruption
eventually leading to coredump, in the bug report's example).

The problem is that after nextval(), sequence.c is holding on to a
pointer to the open relcache entry for the sequence, which it intends
to close at end of transaction.  After the sequence is dropped, the
pointer is just a dangling pointer, and so the eventual close is working
on freed memory, with usually-unpleasant consequences.

I think the best solution for this is to get rid of sequence.c's private
state list.  We could allow the state info for a sequence to be kept
right in the relcache entry (or more likely, in a struct owned and
pointed to by the relcache entry --- that way, the overhead added for
non-sequence relcache entries would only be one more pointer field).
Relcache deletion would know to free the sequence state object along
with the rest of the relcache entry.  As for the action of closing the
relcache entry, I don't see any good reason for sequence.c to hold
relcache entries open between calls anyway.  It is good to hold the
AccessShareLock till commit, but the lock manager can release that lock
at commit by itself; there's no reason at all for sequence.c to do it.

Comments?
        regards, tom lane