Patch queue triage

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> FYI, Tom, Heikki, I need one of you to post the list of patches and
> where we think we are on each one, even if the list is imperfect.

This message is an attempt to sort out which patch queue entries have
no hope of getting into 8.3 (and so we shouldn't spend more time on them
right now), which ones can get in but are waiting on their authors for
more work, and which ones are ready for immediate review.

You'll notice that numerically quite a lot of the patches fall into the
"dead" category. This isn't actually all that surprising because if
they were apply-able they'd have gotten in. (It's not like we've
completely neglected applying patches for the last six months...)
However, many of the remaining live patches are huge ones, like HOT and
delayed commit, and are going to consume considerable review effort
(again, if they didn't, they might have been in already).

The bottom line is that we are desperately in need of more review
manpower. I'm pleased to report that Heikki Linnakangas has promised to
devote most of the next few weeks to helping review, and has already
picked out some patches he feels qualified to work on, as you'll see
below. I have also been coordinating reviewing assignments off-list with
Neil and Magnus to avoid duplication of effort. But I'd like to encourage
everyone who's done any backend hacking to look at the live patches not
shown as assigned to someone specific. The more eyeballs the better;
anything you catch is something someone else might miss. Also, several
of the open patches are in need of more performance testing, so if you
can help out in that fashion please do so.

With that, the patches:

* Re: [pgsql-patches] [PATCHES] [HACKERS] [Fwd: Index Advisor]
/Gurjeet Singh/

I don't think this can be applied in anything like its current state.
The internal interface to the planner is not very good, and ditto for the
user API. What I would suggest trying to do is get a set of plugin hooks
defined for the planner that would allow the advisor to be implemented
entirely as an external module, and then let it be worked on as a
pgfoundry project. I have some ideas about appropriate design of the
plugin hooks (as mentioned in my review message of a month ago), but I
have to say that getting that done for 8.3 is lower-priority than some of
the other patches.

* [pgsql-patches] Ctid chain following enhancement
/Pavan Deolasee/

I'm not very excited about this --- it seems to me to complicate the code
in some places that are not in fact performance-critical. While it
doesn't seem likely to break things, I'm not in favor of reducing code
readability unless a measurable performance improvement can be shown.
Can we have some tests showing this is worthwhile?

* [PATCHES] Error correction for n_dead_tuples
/ITAGAKI Takahiro/

Waiting for OldestXmin patch to be accepted or rejected. However,
regardless of the fate of that patch, I'm concerned that this one creates
an open-loop behavior in which the n_dead_tuples estimate will diverge
arbitrarily far from reality over time. I criticized the original
proposal on that basis, and I'm not convinced this version fixes it,
because of the fact that stats counter updates occur much later than the
actions they count. (My recent patch to rate-limit tabstat messages made
that problem worse, but it existed anyway.) What might make sense is for
vacuum to count the number of dead-but-not-removable tuples it skips over,
and apply that as the value of n_dead_tuples on receipt of the vacuum
message (instead of setting to zero as now). This is likely to be wrong
with respect to the actions of transactions running concurrently with the
vacuum, but I think so is the proposed patch; and at least in this form
the error certainly cannot accumulate across vacuum cycles.

* [PATCHES] Table function support /Pavel Stehule/

Neil has promised to review this. AFAIK there are no showstoppers but
there are some disagreements on the details of the functionality.

* [HACKERS] Grouped Index Tuples /Heikki Linnakangas/
* [HACKERS] Grouped Index Tuples / Clustered Indexes
/Heikki Linnakangas/

Needs review. I'm not sure how many people besides Heikki have really
looked at this (I know I haven't).

* Re: [PATCHES] POSIX shared memory support
/Chris Marcellino/

I'm not convinced that we want this at all. The original proposal was
to provide an alternative for platforms without SysV shmem support,
but the working versions of the patch fail to remove the SysV dependency,
and the portability of this code is itself quite unproven. Do we really
want to take on maintenance of yet-another shmem implementation just to
let people be lazy about changing their SHMMAX settings? As best I can
tell the main problem in that department is Darwin, and it'd be a whole
lot simpler if Apple just raised their darn default SHMMAX to something
that's sensible for the 21st century.

[BTW, there was a patch submitted recently to use SysV semaphores instead
of Posix on Darwin versions that have 'em; why isn't that in the queue?
It makes sense to me to do that to save on file descriptors ...]

* [HACKERS] Automatic adjustment of bgwriter_lru_maxpages (was:
Dead Space Map version 2) /ITAGAKI Takahiro/

Heikki is reviewing this. AIUI there are no known showstoppers but
we'd like more performance measurements, plus there are interactions
with other open patches to investigate.

* [HACKERS] Stream bitmaps /Heikki Linnakangas/

I think this is on hold unless a finished bitmap-index patch shows up;
however there was some discussion of using this in support of clustered
indexes, so maybe it's live anyway? Heikki?

* [PATCHES] Heap page diagnostic/test functions (v2)
/Simon Riggs/

Heikki is reviewing this and refactoring it as a contrib module.

* [PATCHES] non-recursive WITH clause support
/Gregory Stark/

I think the consensus is that we should wait for a more complete WITH
implementation to be submitted, since this one creates compatibility
issues without any real return in the form of added functionality.

* Re: [HACKERS] Bug: Buffer cache is not scan resistant
/Simon Riggs/

Heikki is reviewing this one, in conjunction with the sync-scan patch.
Performance testing is needed for both.

* [PATCHES] Updated bitmap index patch /Gavin Sherry/

We're still waiting to see if a finished patch will be submitted.
I'm very close to saying this has to wait for 8.4, though.

* Re: [HACKERS] [PATCHES] Bitmapscan changes
/Heikki Linnakangas/

I had objected to this on the grounds that it seemed to be covering
only a narrow territory between HOT and bitmap indexes, but given the
probability that one or even both of those won't make it, we need to
give this one a second look. So: live, needs review.

* Re: [PATCHES] Synchronized Scan WIP patch
/Simon Riggs/

Heikki is reviewing this one. Also I believe Greg Smith is doing more
performance testing.

* Re: [HACKERS] [PATCHES] xml2 contrib patch supporting default XML
namespaces /Mike Rylander/

I would like to get Peter to deal with this and the other XML-related
patch.

* [HACKERS] Indexam interface proposal /Heikki Linnakangas/

AFAICS this discussion died out with no actual patch submitted.

* [PATCHES] Load distributed checkpoint V3
/ITAGAKI Takahiro/

Heikki is reviewing this one. More performance testing is needed,
I think.

* [HACKERS] Group Commit /Heikki Linnakangas/

I believe this is on hold for 8.4; in any case it should wait till
after Simon's deferred-commit patch is in and we have more info about
the performance implications of that.

* [PATCHES] Patch for circular buffer in tuplestore to optimize
merge joins (v1) /stark/

I will handle this --- AFAIK there are no showstopper objections.

* [PATCHES] Concurrent psql v4 [WIP] /stark/

This is waiting on the author to change the API per list discussions; we
can't apply something that is about to have some commands removed ...

* Re: [PATCHES] Recalculating OldestXmin in a long-running vacuum
/Heikki Linnakangas/

This is on hold until the other vacuum-related patches are accepted
or rejected; at that point we need to redo the performance tests.
As things stand it should *not* be accepted because the only available
test shows a performance decrease, but maybe that will change after
the other patches go in. (In any case it'd be interesting to find
out why there's no performance improvement, but it seems unlikely
we have time for that investigation for 8.3.)

* Re: [HACKERS] Modifying TOAST thresholds /Tom Lane/

At this point it seems nothing will be done about this issue for 8.3.

* [PATCHES] plpgpsm /Pavel Stehule/

I think this has to be held for 8.4: it was submitted too late for the 8.3
feature deadline, and in fact I don't recall that there was any community
discussion/consensus on accepting it into core at all. Another big
problem is that it largely copies-and-pastes the plpgsql code, which I
think is an unacceptable maintenance burden in the long run. We need to
consider whether we can't refactor to avoid code duplication.

* [HACKERS] tsearch_core patch for inclusion
/Teodor Sigaev/

Have we resolved whether the API and the dump/restore strategy are
acceptable? The code needs review too, but not till we've settled
on the basic question whether we like the feature set.

* Re: [PATCHES] [HACKERS] Full page writes improvement, code update
/Koichi Suzuki/

I feel that we have to insist that this be modified to not create any WAL
bloat in the pre-compression form. That will be more efficient and will
avoid the whole argument about whether a switch is needed. There was also
doubt about whether the external programs (pg_compresslog etc) were ready
for prime time. At this point we could accept a patch that makes whatever
small tweaks are needed to ensure that an incremental-format WAL record
can be extracted from a full-page-write record, at least for the most
common WAL record types for which compression is actually important. I
think the actual compression/decompression programs could undergo further
development on pgfoundry with an eye to merging them into core before 8.4
release.

* [PATCHES] Dead Space Map version 3 (simplified)
/ITAGAKI Takahiro/

I want to reject this in its current form. It's been clear almost since
the FSM code was first released to the field that its dependence on a
fixed-size shared memory area was a serious design blunder (one for which
I take full blame; I thought it'd be workable, I was wrong). We can't
sanely take a patch that repeats that mistake. It seems fairly clear that
DSM could be redesigned to be stored in "special space" in every Nth heap
page and thereby avoid the problem. We should do that and look at whether
FSM couldn't be integrated into the same data structure. (I'm not
entirely sure how to handle FSM for indexes in this fashion, but maybe we
should just throw that problem back onto the shoulders of the index AMs;
for instance make them keep linked lists of free index pages.)
Obviously this won't happen for 8.3, but I don't think DSM as it stands
would be a net plus for users.

* Re: [PATCHES] COPY-able sql log outputs /FAST PostgreSQL/

Are we agreed on the API for this? AFAIR there were no showstoppers
but there was some considerable discussion about API details.

* [PATCHES] Updateable cursors patch /FAST PostgreSQL/

This is incomplete, and I fear at this point has to be held over to 8.4.

* [PATCHES] Preliminary GSSAPI Patches /Henry B. Hotz/

Magnus is reviewing this one.

* [PATCHES] Logging checkpoints and other slowdown causes
/Greg Smith/

Heikki is reviewing this one.

* Re: [PATCHES] [Fwd: Deferred Transactions, Transaction Guarantee
and COMMITwithout waiting] /Simon Riggs/

Simon is on the hook to submit an updated patch. I hope this one
makes it in, as it looks like a really nice performance improvement
for those who can use it; but the original patch seems overcomplicated.

* Re: [HACKERS] Arrays of Complex Types /David Fetter/

Waiting on the author to respond to open issues; in particular the
problem of not making array types for system catalogs.

* Re: [HACKERS] Implicit casts to text /Peter Eisentraut/

I had volunteered to finish this, but don't be surprised if it gets
put off till 8.4.

* Re: [GENERAL] dropping role w/dependent objects
/Ed L./

Patch seems ok, comment maybe overkill. Will apply this shortly.

* Re: [HACKERS] pg_index updates and SI invalidation
/Pavan Deolasee/

Needs some review, I don't think there are any showstoppers though.

* [PATCHES] HOT Patch - Ready for review /Pavan Deolasee/

This needs a *lot* of review. Can we break it down into more manageable
chunks? I'm not sure that anyone's got a full grasp of the implications
of this patch, and that's a scary thought.

* [PATCHES] Minor recovery changes /Simon Riggs/

The submission mentions open issues --- when will those be resolved?
Should we apply the patch-so-far anyway?

* Re: [PATCHES] UTF8MatchText /ITAGAKI Takahiro/

I'd like someone who uses UTF8 more than I do to double-check whether this
is sane. Also, can we avoid having so much code duplication between the
various forms of LIKE?

* [PATCHES] Clear up strxfrm() in UTF-8 with locale on Windows
/ITAGAKI Takahiro/

Someone else needs to look at this; I can't test it. Magnus?

* Re: [PATCHES] xpath_array with namespaces support
/Nikolay Samokhvalov/

I would like to get Peter to deal with this and the other XML-related
patch.

* Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
/Trevor Hardcastle/

Neil will review this.

* [PORTS] m32r tas() implementation /Martin Pitt/

This lacks a configure probe for sys/tas.h, but I have no other
objection to it.

* [HACKERS] [PATCH] A crash and subsequent recovery of the master
can cause theslave to get out-of-sync /Florian G. Pflug/

This seems like a real problem that needs review. The given patch
was done hastily and might or might not be OK, though.

* [COMMITTERS] pgsql: Remove some of the most blatant brain-fade in
the recent guc /Tom Lane/

The recent guc patch to reset variables when they are removed from
postgresql.conf was broken. Again :-(. It needs to be fixed
or reverted. (Why are we having such problems here? Maybe the
GUC code as a whole is too complex to understand?)

* [HACKERS] Avoiding unnecessary reads in recovery
/Heikki Linnakangas/

I think we need to apply this, or else do something else with the
wired-in assumption about full page writes that was noted in the
discussion.

* Re: [PATCHES] [HACKERS] autovacuum does not start in HEAD
/ITAGAKI Takahiro/

I assume Alvaro will deal with this.

* Re: [PATCHES] LIMIT/SORT optimization /Gregory Stark/
* [PATCHES] updated SORT/LIMIT patch /Gregory Stark/

I will look at this. I recall being concerned about whether there
wasn't a cleaner way to introduce the required inter-node communication.

* Re: [PATCHES] New version of GENERATED/IDENTITY, was Re: parser
dilemma /Zoltan Boszormenyi/

I reviewed this earlier and can deal with the update.

There is one large item that is not in the patch queue at all, because
it's not been written, and that's the change to support per-function
settings of search_path. I think this is critical to get done for 8.3,
but adding a column to pg_proc will create merge issues for any patch that
adds anything to pg_proc, and a lot of the open ones do. So I've been
holding off writing it until more of them are in. I think it's going to
be straightforward so I'm not too worried about the delay, but it needs to
happen.

regards, tom lane