Re: patch queue

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Here is Peter's analysis of our current 7.3 backlog:

Some followup comments:

> * Patch to add CREATE OPERATOR CLASS
> I don't like that syntax.

Me either. I'm quite certain that there was followup discussion to this
version of the patch; I don't recall if Bill had submitted a revised
patch, but this version should not be applied.

> * ECPG patches: get descriptor NULL alloc, external names

> Don't know. (Although Cristof's ECPG patches tend to be good.)

Michael Meskes should have the responsibility to review and apply ecpg
patches (actually I think he did already apply everything from Cristof).

> * Support for QNX6, POSIX IPC and PTHREAD-style locking

> Patch needs serious cleanups in build system. Should be split into two or
> more patches for generic QNX6 support and for the locking and IPC changes.

Agreed. I would like to see more discussion about what sorts of
alternate locking/IPC mechanisms we want to support before, rather than
after, someone plugs in new code. That should in turn lead to some kind
of implementation-independent API for the locking primitives, rather
than piling one set of implementation dependencies atop another, as
here. This patch is a good starting point for that discussion --- but
I don't want to apply it as-is.

> * Undocumented feature costs a lot of performance in COPY

> Not sure what this is supposed to do.

The idea was to make the syntax of the COPY options a little more
consistent, which is fine --- but where's the documentation update
to go with this? Ask submitter to resubmit with doco patch included.

> * contrib: int_array_aggregator() int_array_enum()

> I can't understand the description given.

Seems marginally useful as a contrib example ... though I agree the
README could be improved, and some attention to spelling would be good.

> * Locale support for postgresql regex (src)

> Does not support multibyte. But the issue might be worth a TODO item.

Agreed, this cannot be applied as-is.

> * Fix for non-blocking connections in libpq

> Run this by the list one more time for review.

There was followup discussion on this when it was submitted; I don't
recall if we decided it was okay as-is. I do know that I've been
unhappy with the nonblocking extension to libpq since day one.

> * hashing improvements

> Tom Lane didn't protest the last patch, so it seems OK.

This patch is OK to apply.

> * Updated TODO item [Gavin Sherry's patch to add OWNER option to CREATE DB]

> Have the author throw the latest patch up for review.

It looks okay to me, except for the // comments and a couple other
trivial details, and the lack of documentation patches; but basically
it's a good feature, and we need it so we can fix up pg_dumpall to not
assume database owners have createdb privilege. I will take
responsibility for fixing and applying this one.

> * Make equals sign optional in CREATE DATABASE WITH param = 'val'

> Seems OK.

This is a subset of Gavin Sherry's patch above; we don't need it.

> patches2 (unfiltered queue)
> ========

> * CLUSTER TODO item

> Ask Tom. Probably should look at the whole patch at least once more.

Gavin made clear that that patch wasn't ready to apply. Remove from queue.

> * Select * from cursor foo

> No idea.

Not ready for prime time either.

> * Pl/Tcl problem
> Andreas Zeugwetter had a patch for this, which we should work with.
> * Problem compiling postgres sql --with-tcl
> same

Yes, I think Andreas' patch should be the starting point for these
issues. Not sure if we were ready to apply it or not, though.

> * Casting Varchar to Numeric

> ... is just a bad idea.

At least till we have a distinction between explicitly and implicitly
invokable casts. An explicit-only text->numeric cast would be fine with me.

> * guc

> This patch was effectively rejected. The TODO item "Add SET REAL_FORMAT
> and SET DOUBLE_PRECISION_FORMAT using printf args" has effectively been
> rejected with it. What we want is to be able to print floating-point
> numbers in a portable binary representation (sprintf("%a")) for the
> purposes of pg_dump.

Well, Peter may not like the concept but I still do. I agree that this
particular patch was rejected.

> The remaining items are recent and possibly still under discussion. I
> don't know the latest status on them.

I believe Fernando Nasser's "Allow arbitrary levels of
analyze/rewriting" is fine. I wasn't happy with the details of Brent's
"Problem reloading regression database" patch; that still needs another
iteration. Not sure about the rest. Could you clean out the cruft
(non-patch items and stuff we've already agreed to reject) so it's
easier to see what still needs review?

regards, tom lane