Re: [HACKERS] Allow INSTEAD OF DELETE triggers to modify the tuple for RETURNING

[Marko Tiikkaja]

On Thu, Nov 2, 2023 at 12:24 PM Shlok Kyal  wrote:
> I went through the CFbot and found that docs build run is giving some
> error (link: https://cirrus-ci.com/task/5712582359646208):
>
> Just wanted to make sure you are aware of the issue.

I am now.  Thanks! :-)  Will try to keep an eye on the builds in the future.

Attached v4 of the patch which should fix the issue.


.m


instead_of_delete_returning_v4.patch
Description: Binary data

Re: [HACKERS] Allow INSTEAD OF DELETE triggers to modify the tuple for RETURNING

[Marko Tiikkaja]

Hi,

Thank you for the feedback.

Apparently it took me six years, but I've attached the latest version
of the patch which I believe addresses all issues.  I'll add it to the
open commitfest.


.m


instead_of_delete_returning_v3.patch
Description: Binary data

Re: Avoid unused value (src/fe_utils/print.c)

[Marko Tiikkaja]

On Thu, Jul 6, 2023 at 5:37 PM Karina Litskevich
 wrote:
> My point is, technically right now you won't see any difference in output
> if you remove the line. Because if we get to that line the need_recordsep
> is already true. However, understanding why it is true is complicated. That's
> why if you remove the line people who read the code will wonder why we don't
> need a separator after "fputs"ing a footer. So keeping that line will make
> the code more readable.
> Moreover, removing the line will possibly complicate the future maintenance.
> As I wrote in the part you just quoted, if the function changes in the way
> that need_recordsep is not true right before printing footers any more, then
> output will be unexpected.

I agree with Karina here.  Either this patch should keep the
"need_recordsep = true;" line, thus removing the no-op assignment to
false and making the code slightly less unreadable; or the entire
function should be refactored for readability.


.m

Re: [PATCH] Implement INSERT SET syntax

[Marko Tiikkaja]

On Wed, Mar 23, 2022 at 5:33 PM Tom Lane  wrote:
>
> Justin Pryzby  writes:
> > You have to either include the pre-requisite patches as 0001, and your 
> > patch as
> > 0002 (as I'm doing now), or name your patch something other than *.diff or
> > *.patch, so cfbot doesn't think it's a new version of the patch to be 
> > tested.
>
> This patch has been basically ignored for a full two years now.
> (Remarkably, it's still passing in the cfbot.)
>
> I have to think that that means there's just not enough interest
> to justify committing it.  Should we mark it rejected and move on?
> If not, what needs to happen to get it unstuck?

I can help with review and/or other work here.  Please give me a
couple of weeks.


.m

Re: Partial index "microvacuum"

[Marko Tiikkaja]

On Wed, Sep 15, 2021 at 7:25 PM Peter Geoghegan  wrote:
> What about v14? There were significant changes to the
> microvacuum/index deletion stuff in that release:
>
> https://www.postgresql.org/docs/14/btree-implementation.html#BTREE-DELETION

Huh.  Interesting.  I'm sorry, I wasn't aware of this work and didn't
have version 14 at hand.  But it looks like both the partial index as
well as the secondary index on (id::text) get cleaned up nicely there.
I even tried a version where I have a snapshot open for the entire
run, and the subsequents SELECTs clean the bloat up.  I'll need to
read up on the details a bit to understand exactly what changed, but
it appears that at least this particular pattern has already been
fixed.

Thank you so much for your work on this!


.m

Partial index "microvacuum"

[Marko Tiikkaja]

So I've been looking at issues we used to have in production some time
ago which eventually lead us to migrating away from partial indexes in
some cases.  In the end, I'm surprised how easy this (or at least a
similar case) was to reproduce.  The attached program does some
UPDATEs where around every third update deletes the row from the
partial index since it doesn't match indpred anymore.  In that case
the row is immediately UPDATEd back to match the index WHERE clause
again.  This roughly emulates what some of our processes do in
production.

Today, running the program for a few minutes (until the built-in
262144 iteration limit), I usually end up with a partial index through
which producing the only row takes milliseconds on a cold cache, and
over a millisecond on a hot one.  Finding the row through the primary
key is still fast, because the bloat there gets cleaned up.  As far as
I can tell, after the index has gotten into this state, there's no way
to clean it up except VACUUMing the entire table or a REINDEX.  Both
solutions are pretty bad.

My working theory was that this has to do with the fact that
HeapTupleSatisfiesMVCC doesn't set the HEAP_XMAX_COMMITTED bit here,
but I'm not so sure anymore.  Has anyone seen something like this?  If
that really is what's happening here, then I can see why we wouldn't
want to slow down SELECTs with expensive visibility checks.  But that
really leaves me wishing for something like VACUUM INDEX partial_idx.
Otherwise your elephant just keeping getting slower and slower until
you get called at 2 AM to play REINDEX.

(I've tested this on 9.6, v11 and v13.  13 seems to be a bit better
here, but not "fixed", I think.)


.m
#!perl

use strict;
use warnings;

use DBI;
use DBD::Pg;

my @connect = ("dbi:Pg:", '', '', {pg_enable_utf8 => 1, RaiseError => 1, PrintError => 0, AutoCommit => 1});
my $dbh = DBI->connect(@connect) or die;

$dbh->do(q{
DROP TABLE IF EXISTS t1;

CREATE TABLE t1 (
	id serial PRIMARY KEY,
	partial boolean NOT NULL,
	data bigint NOT NULL
);
ALTER TABLE t1 SET (autovacuum_enabled = false);

INSERT INTO t1(partial, data) SELECT FALSE, -gs.i FROM generate_series(1, 100) gs(i);

CREATE INDEX i_love_partial_indexes ON t1 (id) WHERE partial;
CREATE INDEX pkish ON t1 ((id::text));

INSERT INTO t1 (partial, data) VALUES (TRUE, 0);
});

$dbh->do(q{VACUUM ANALYZE t1});

my $rows = $dbh->selectall_arrayref(q{SELECT id FROM t1 WHERE partial});
die unless (scalar(@$rows) == 1 && scalar(@{$rows->[0]}) == 1);
my $partial_id = $rows->[0]->[0];

for (my $i = 0; $i < 256 * 1024; ++$i) {
my $rows = $dbh->selectall_arrayref(
q{UPDATE t1 SET data = data + 1, partial = random() < 0.7 WHERE id = $1 RETURNING partial::int},
undef,
$partial_id,
);
die unless (scalar(@$rows) == 1 && scalar(@{$rows->[0]}) == 1);
my $partial = $rows->[0]->[0];
if ($partial == 1) {
$dbh->do(
q{UPDATE t1 SET partial = TRUE WHERE id = $1},
undef,
$partial_id,
);
}
}

Re: UNIQUE null treatment option

[Marko Tiikkaja]

On Fri, Aug 27, 2021 at 3:38 PM Peter Eisentraut <
peter.eisentr...@enterprisedb.com> wrote:

> In the SQL:202x draft, this
> has been formalized by making this implementation-defined and adding
> an option on unique constraint definitions UNIQUE [ NULLS [NOT]
> DISTINCT ] to choose a behavior explicitly.
>
> The CREATE UNIQUE INDEX syntax extension is not from the standard,
> it's my own invention.
>

For the unique index syntax, should this be selectable per
column/expression, rather than for the entire index as a whole?


.m

Re: security_definer_search_path GUC

[Marko Tiikkaja]

On Thu, Jun 3, 2021 at 7:30 PM Mark Dilger 
wrote:

> > On Jun 3, 2021, at 9:03 AM, Pavel Stehule 
> wrote:
> >
> > I agree so some possibility of locking search_path or possibility to
> control who and when can change it can increase security. This should be a
> core feature. It's maybe more generic issue - same functionality can be
> required for work_mem setting, maybe max_paralel_workers_per_gather, and
> other GUC
>
> Chapman already suggested a mechanism in [1] to allow chaining together
> additional validators for GUCs.
>
> When setting search_path, the check_search_path(char **newval, void
> **extra, GucSource source) function is invoked.  As I understand Chapman's
> proposal, additional validators could be added to any GUC.  You could
> implement search_path restrictions by defining additional validators that
> enforce whatever restriction you like.
>
> Marko, does his idea sound workable for your needs?  I understood your
> original proposal as only restricting the value of search_path within
> security definer functions.  This idea would allow you to restrict it
> everywhere, and not tailored to just that context.
>

Yeah, that would work for my use case just as well.


.m

Re: security_definer_search_path GUC

[Marko Tiikkaja]

On Thu, Jun 3, 2021 at 9:14 AM Joel Jacobson  wrote:

> On Thu, Jun 3, 2021, at 00:55, Marko Tiikkaja wrote:
>
> They still show up everywhere when looking at "public".  So this is only
> slightly better, and a maintenance burden.
>
>
> Good point. I find this annoying as well sometimes.
>
> It's easy to get a list of all objects for an extension, via \dx+
>
> But it's hard to see what objects in a schema, that are provided by
> different extensions, via e.g. \df public.*
>
> What about adding a new "Extension" column next to "Schema" to the
> relevant commands, such as \df?
>

That's just one part of it.  The other part of my original proposal was to
avoid having to  SET search_path  for all SECURITY DEFINER functions.  I
still think either being able to lock search_path or the separate prosecdef
search_path is the best option here.


.m

Re: security_definer_search_path GUC

[Marko Tiikkaja]

On Wed, Jun 2, 2021 at 11:32 PM Joel Jacobson  wrote:

> On Wed, Jun 2, 2021, at 18:36, Marko Tiikkaja wrote:
>
> The use case is: version upgrades.  I want to be able to have a
> search_path of something like 'pg_catalog, compat, public'.  That way we
> can provide compatibility versions of newer functions in the "compat"
> schema, which get taken over by pg_catalog when running on a newer
> version.  That way all the compatibility crap is clearly separated from the
> stuff that should be in "public".
>
>
> That's a neat trick, probably the best solution in a really old PostgreSQL
> version, before we had extensions.
>
> But if running a recent PostgreSQL version, with support for extensions, I
> think an even cleaner solution
> would be to package such compatibility versions in a "compat" extension,
> that would just install them into the public schema.
>

Writing, verifying and shipping extension upgrade scripts is not pleasant.
I'd much prefer something that's integrated to the workflow I already have.


> And if you wonder what functions in public come from the compat extension,
> you can use use \dx+.
>

They still show up everywhere when looking at "public".  So this is only
slightly better, and a maintenance burden.


.m

Re: security_definer_search_path GUC

[Marko Tiikkaja]

On Wed, Jun 2, 2021 at 10:20 PM Alvaro Herrera 
wrote:

> On 2021-Jun-02, Marko Tiikkaja wrote:
>
> > The use case is: version upgrades.  I want to be able to have a
> search_path
> > of something like 'pg_catalog, compat, public'.  That way we can provide
> > compatibility versions of newer functions in the "compat" schema, which
> get
> > taken over by pg_catalog when running on a newer version.  That way all
> the
> > compatibility crap is clearly separated from the stuff that should be in
> > "public".
>
> Can't you achieve that with "ALTER DATABASE .. SET search_path"?
>

No, because I have a thousand SECURITY DEFINER functions which have to
override search_path or they'd be insecure.


.m

Re: security_definer_search_path GUC

[Marko Tiikkaja]

On Wed, Jun 2, 2021 at 3:46 PM Joel Jacobson  wrote:

> If a database object is to be accessed unqualified by all users, isn't the
> 'public' schema a perfect fit for it? How will it be helpful to create
> different database objects in different schemas, if also adding all such
> schemas to the search_path so they can be accessed unqualified? In such a
> scenario you risk unintentionally creating conflicting objects, and
> whatever schema happened to be first in the search_path will be resolved.
> Seems insecure and messy to me.
>

Heh.  This is actually exactly what I wanted to do.

The use case is: version upgrades.  I want to be able to have a search_path
of something like 'pg_catalog, compat, public'.  That way we can provide
compatibility versions of newer functions in the "compat" schema, which get
taken over by pg_catalog when running on a newer version.  That way all the
compatibility crap is clearly separated from the stuff that should be in
"public".


.m

Re: security_definer_search_path GUC

[Marko Tiikkaja]

On Sat, May 29, 2021 at 11:06 PM Joel Jacobson  wrote:

> Glad you bring this problem up for discussion, something should be done to
> improve the situation.
>
> Personally, as I really dislike search_path and consider using it an
> anti-pattern.
> I would rather prefer a GUC to hard-code search_path to a constant default
> value of just ‘public’ that cannot be changed by anyone or any function.
> Trying to change it to a different value would raise an exception.
>

That would work, too!  I think it's a nice idea, perhaps even better than
what I proposed.  I would be happy to see either one incorporated.


.m

security_definer_search_path GUC

[Marko Tiikkaja]

Hi,

Since writing SECURITY DEFINER functions securely requires annoying
incantations[1], wouldn't it be nice if we provided a way for the superuser
to override the default search path via a GUC in postgresql.conf?  That way
you can set search_path if you want to override the default, but if you
leave it out you're not vulnerable, assuming security_definer_search_path
only contains secure schemas.


.m

Re: how to correctly cast json value to text?

[Marko Tiikkaja]

On Mon, May 3, 2021 at 12:24 PM Pavel Stehule 
wrote:

> Is it possible to do this with built functionality?
>
> I miss the cast function for json scalar string value to string.
>

#>>'{}'


.m

Re: [PATCH] Implement motd for PostgreSQL

[Marko Tiikkaja]

 Hi Joel

On Fri, Apr 2, 2021 at 11:47 PM Joel Jacobson  wrote:

> PostgreSQL has since long been suffering from the lack of a proper UNIX
> style motd (message of the day).
>

First of all, thanks for your work on this!  I think this is an important
feature to have, but I would love to see a way to have a set of strings
from which you choose a random one to display.  That way you could brighten
your day with random positive messages.


-marko

Re: INSERT ON CONFLICT and RETURNING

[Marko Tiikkaja]

There's prior art on this: https://commitfest.postgresql.org/15/1241/


.m

Re: Local visibility with logical decoding

[Marko Tiikkaja]

On Mon, Jul 20, 2020 at 7:36 PM Antonin Houska  wrote:

> Marko Tiikkaja  wrote:
>
> > It appears that when logical decoding sends out the data from the output
> > plugin, it is not guaranteed that the decoded transaction's effects are
> > visible on the source server.  Is this the way it's supposed to work?
>
> Can you please share the test that indicates this behavior? As far as I
> understand, the transaction must have been committed before the output
> plugin
> starts to receive the changes.
>

I don't have a reliable test program, but you can reproduce quite easily
with test_decoding if you put a breakpoint before the SyncRepWaitForLSN()
call in src/backend/access/transam/xact.c.  pg_logicalrecv will see the
changes while the session is sitting on the breakpoint, and not finishing
its commit.


-marko

Local visibility with logical decoding

[Marko Tiikkaja]

Hi,

It appears that when logical decoding sends out the data from the output
plugin, it is not guaranteed that the decoded transaction's effects are
visible on the source server.  Is this the way it's supposed to work?

If so, would doing something like this in the output plugin be reasonable?

TransactionId xid = transaction->xid;
if (transaction->is_known_as_subxact)
xid = transaction->toplevel_xid;

if (TransactionIdIsInProgress(xid))
XactLockTableWait(xid, NULL, NULL, XLTW_None);


-marko

Re: [PATCH] Implement INSERT SET syntax

[Marko Tiikkaja]

On Fri, Nov 1, 2019 at 6:31 PM Robert Haas  wrote:

> On Sun, Aug 18, 2019 at 11:00 AM Tom Lane  wrote:
> > Perhaps the way to resolve Peter's objection is to make the syntax
> > more fully like UPDATE:
> >
> > INSERT INTO target SET c1 = x, c2 = y+z, ... FROM tables-providing-x-y-z
> >
> > (with the patch as-submitted corresponding to the case with an empty
> > FROM clause, hence no variables in the expressions-to-be-assigned).
> >
> > Of course, this is not functionally distinct from
> >
> > INSERT INTO target(c1,c2,...) SELECT x, y+z, ... FROM
> tables-providing-x-y-z
> >
> > and it's fair to question whether it's worth supporting a nonstandard
> > syntax just to allow the target column names to be written closer to
> > the expressions-to-be-assigned.
>
> For what it's worth, I think this would be useful enough to justify
> its existence. Back in days of yore when dragons roamed the earth and
> I wrote database-driven applications instead of hacking on the
> database itself, I often wondered why I had to write two
> completely-different looking SQL statements, one to insert the data
> which a user had entered into a webform into the database, and another
> to update previously-entered data. This feature would allow those
> queries to be written in the same way, which would have pleased me,
> back in the day.
>

I still do, and this would be a big help.  I don't care if it's
non-standard.


.m

Re: [PATCH] Implement INSERT SET syntax

[Marko Tiikkaja]

On Wed, Jul 17, 2019 at 7:30 AM Gareth Palmer 
wrote:

> Attached is a patch that adds the option of using SET clause to specify
> the columns and values in an INSERT statement in the same manner as that
> of an UPDATE statement.
>

Cool!  Thanks for working on this, I'd love to see the syntax in PG.

There was a brief discussion regarding INSERT SET on pgsql-hackers in late
> August 2009 [1].
>

There was also at least one slightly more recent adventure:
https://www.postgresql.org/message-id/709e06c0-59c9-ccec-d216-21e38cb5ed61%40joh.to

You might want to check that thread too, in case any of the criticism there
applies to this patch as well.


.m

Re: New committer: David Rowley

[Marko Tiikkaja]

On Thu, May 30, 2019 at 6:39 PM Magnus Hagander  wrote:
For those of you that have not read the minutes from the developer meeting
ahead of pgcon (can be found at
https://wiki.postgresql.org/wiki/PgCon_2019_Developer_Meeting), we'd like
to announce here as well that David Rowley has joined the ranks of
PostgreSQL committers.

Congratulations to David, may the buildfarm be gentle to him, and his first
revert far away!

Yee.

>

Re: Follow-up on INSERT INTO ... SET ...

[Marko Tiikkaja]

On Tue, Jan 29, 2019 at 6:26 PM Sven Berkvens-Matthijsse <
s...@postgresql.berkvens.net> wrote:

> On 29/01/2019 07.20, Tom Lane wrote:
> > Looking at the thread, it seems like Marko lost interest for some
> > reason, and never submitted a revised patch.
>
> That was my conclusion too, but I didn't know whether there had been
> some off-list discussion that eventually led to the abandonment of the
> patch and proposal.
>

There has not.  I just don't have the energy or interest to pursue this at
the moment.


.m

Re: Early WIP/PoC for inlining CTEs

[Marko Tiikkaja]

On Sat, Jan 26, 2019 at 12:22 AM Tom Lane  wrote:

> Therefore, I'm reversing my previous opinion that we should not have
> an explicit NOT MATERIALIZED option.  I think we should add that, and
> the behavior ought to be:
>
> * No option given: inline if there's exactly one reference.
>
> * With MATERIALIZED: never inline.
>
> * With NOT MATERIALIZED: inline regardless of the number of references.
>

This much has been obvious to most people for a long time.


.m

Re: fine tune v11 release notes

[Marko Tiikkaja]

I like broccoli

Re: BUG #15182: Canceling authentication due to timeout aka Denial of Service Attack

[Marko Tiikkaja]

On Fri, Jul 20, 2018 at 2:17 AM, Jeremy Schneider 
wrote:

> I'd like to bump this old bug that Lloyd filed for more discussion. It
> seems serious enough to me that we should at least talk about it.
>
> Anyone with simply the login privilege and the ability to run SQL can
> instantly block all new incoming connections to a DB including new
> superuser connections.
>

So..  don't VACUUM FULL pg_authid without lock_timeout?

I can come up with dozens of ways to achieve the same effect, all of them
silly.


.m

Re: Two round for Client Authentication

[Marko Tiikkaja]

On Thu, Jun 14, 2018 at 7:12 AM, Yinjie Lin  wrote:

> Currently I am reading and testing code about Client Authentication, but I
> find that there are two progresses forked if I login using psql, while only
> one progress is forked if using pgAdmin.
>

If psql finds the server asks for a password, it closes the first
connection, displays a password prompt to the user, and then does another
connection attempt with the password the user entered.  You can avoid the
first attempt with the -W flag; though there's usually no reason to do that
in practice.


.m

Re: Diagonal storage model

[Marko Tiikkaja]

On Sun, Apr 1, 2018 at 3:48 PM, Konstantin Knizhnik <
k.knizh...@postgrespro.ru> wrote:

> I want to announce new model, "diagonal storage" which combines benefits
> of both approaches.
> The idea is very simple: we first store column 1 of first record, then
> column 2 of second record, ... and so on until we reach the last column.
> After it we store second column of first record, third column of the
> second record,...
>

I'm a little worried about the fact that even with this model we're still
limited to only two dimensions.  That's bound to cause problems sooner or
later.


.m

get_actual_variable_range vs idx_scan/idx_tup_fetch, again

[Marko Tiikkaja]

(Sorry for not continuing the thread in 54418d75.2000...@joh.to , but I
don't have the original email anymore.)

So I'm in the same pickle again.  According to pg_stat_user_indexes an
index is being used all the time.  However, it's only being used by
mergejoinscansel() to compare these two plans:

=> explain analyze
select *
from orders child
join orders parent on (parent.orderid = child.parentorderid)
where child.orderid = 1161771612;
 QUERY PLAN

 Nested Loop  (cost=0.00..15.56 rows=1 width=2910) (actual
time=0.401..0.402 rows=1 loops=1)
   ->  Index Scan using orders_pkey on orders child  (cost=0.00..7.78
rows=1 width=1455) (actual time=0.367..0.367 rows=1 loops=1)
 Index Cond: (orderid = 1161771612)
   ->  Index Scan using orders_pkey on orders parent  (cost=0.00..7.78
rows=1 width=1455) (actual time=0.027..0.028 rows=1 loops=1)
 Index Cond: (orderid = child.parentorderid)
 Total runtime: 0.852 ms
(6 rows)

=> set enable_nestloop to false; set enable_hashjoin to false;
SET
SET
=> explain
select *
from orders child
join orders parent on (parent.orderid = child.parentorderid)
where child.orderid = 1161771612;
   QUERY PLAN
-
 Merge Join  (cost=1804805.57..97084775.33 rows=1 width=2910)
   Merge Cond: (parent.orderid = child.parentorderid)
   ->  Index Scan using orders_pkey on orders parent
(cost=0.00..96776686.40 rows=123232448 width=1455)
   ->  Sort  (cost=7.79..7.79 rows=1 width=1455)
 Sort Key: child.parentorderid
 ->  Index Scan using orders_pkey on orders child  (cost=0.00..7.78
rows=1 width=1455)
   Index Cond: (orderid = 1161771612)
(7 rows)

The merge join plan is pretty obviously shit and the fact that the planner
got a better estimate for it by peeking through the index had zero effect.

I think it would be really important to have a way to turn off
get_actual_variable_range() for a specific index during runtime.  Would a C
level hook be acceptable for this?


.m

Re: Using scalar function as set-returning: bug or feature?

[Marko Tiikkaja]

On Tue, Feb 13, 2018 at 12:30 PM, Tels 
wrote:

> I'm not sure if you mean exactly the scenario as in the attached test
> case, but this works in plpgsql, too, and would be a shame to lose.
>
> OTOH, one could also write:
>
>   SELECT INTO ba, bb  a,b FROM foo(1);
>
> and it would still work, or wouldn't it?
>

Yes.  The code in test.psql shouldn't pass code review.


.m

Re: Using scalar function as set-returning: bug or feature?

[Marko Tiikkaja]

On Fri, Feb 9, 2018 at 9:58 AM, Konstantin Knizhnik <
k.knizh...@postgrespro.ru> wrote:

> Attached please find patch reporting error in case of empty attribute list
> for SELECT INTO
>

This is quite short-sighted.  The better way to do this is to complain if
the number of expressions is different from the number of target variables
(and the target variable is not a record-ish type).  There's been at least
two patches for this earlier (one my me, and one by, I think Pavel
Stehule).  I urge you to dig around in the archives to avoid wasting your
time.


.m

Re: [HACKERS] INSERT .. ON CONFLICT DO SELECT [FOR ..]

[Marko Tiikkaja]

On Thu, Nov 30, 2017 at 6:39 AM, Peter Geoghegan  wrote:

> On Wed, Nov 29, 2017 at 8:34 PM, Michael Paquier
>  wrote:
> > The patch does not currently apply. I am noticing as well that Peter
> > Geoghegan has registered himself as a reviewer a couple of hours back,
> > so moved to next CF with waiting on author as status.
>
> Marko unregistered me, so I promptly reregistered. You'll have to ask him
> why.


Oops.  I didn't mean to do what I did, and I apologize.  I had my months
mixed up and I thought this commit fest had ended without you having looked
at the patch, so I assumed you would register in the next CF if you were
still interested, or someone else could pick it up.

In any case, *I* don't have any interest in pursuing this patch at this
point.  I think this is a really good feature, and as far as I know the
patch is technically solid, or at least was when it still applied.  Can
someone else take it from here?


.m