Re: Outstanding patches

Bruce Momjian <pgman@candle.pha.pa.us> writes:
> OK, now that we have started 7.2 development, I am going to go through
> the outstanding patches and start to apply them or reject them.  They
> are at:
>     http://candle.pha.pa.us/cgi-bin/pgpatches
> I could use help in identifying which patches are a problem.  Most of
> the ones there now have been reviewed by me or have received the
> recommendation of another developer.

Okay, I looked through these ...

I do not like Ian Taylor's plpgsql cursor patch; trying to do cursors
inside plpgsql with no SPI-level support is too much of a kluge.  We
should first add cursor support to SPI, then fix plpgsql.  Much of the
parsing work he's done could be salvaged, but the implementation can't
be.  (And I don't want to apply it now and back it out later --- it adds
too many warts.)

Fernando Nasser's ANALYZE patch is superseded by already-applied work,
though if he wants to do the promised test additions I would be happy.

The PAM support patch concerns me --- it looks like yet another chunk
of code that will tie up the postmaster in a single-threaded
conversation with a remote daemon that may or may not respond promptly.
I recommend holding off on this until we think about whether we
shouldn't restructure the postmaster to do all authentication work in
per-client subprocesses.

We need to discuss whether we like the %TYPE feature proposed by Ian
Taylor.  It seems awfully nonstandard to me, and I'm not sure that the
value is great enough to be worth inventing a nonstandard feature.
ISTM that people don't normally tie functions to tables so tightly that
it's better to define a function in terms of "the type of column foo
of table bar" than just in terms of the type itself.  Ian claims that
this is helpful, but is it really likely that you can change that column
type without making *any* other mods to the function?  Moreover, in
exchange for this possible benefit you are opening yourself to breaking
the function if you choose to rename either the column or the table.
The potential net gain seems really small.  (If we do like the
functionality, then the patch itself seems OK with the exception of the
gram.y definition of func_type; the table name should be TokenId not
just IDENT.)

I did not look at any of the JDBC or libpq++ patches.  The other stuff
seemed OK on a first glance.
        regards, tom lane