From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Neil Conway <neilc(at)samurai(dot)com> |
Cc: | Patches <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: tuplestore_putvalues() |
Date: | 2008-03-23 01:35:35 |
Message-ID: | 25827.1206236135@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-patches |
Neil Conway <neilc(at)samurai(dot)com> writes:
> Attached is a patch that allows an array of Datums + nulls to be
> inserted into a tuplestore without first creating a HeapTuple, per
> recent suggestion on -hackers. This avoids making an unnecessary copy.
> There isn't a really analogous optimization to be applied to tuplesort:
> it takes either a TTS, an IndexTuple or a basic Datum, none of which
> involve an extra copy.
After a quick read, looks sane except for one stylistic gripe:
in exec_stmt_return_next, you added an initialization of tuple = NULL
in order to remove a couple of lines like
tuple = NULL; /* keep compiler quiet */
I think this is not good coding style. The point of not having the
initialization is so that the compiler will warn us if there are
any paths through the function in which we fail to set "tuple".
You've just disabled that bit of early-warning error detection.
It's better if each switch arm has to set tuple for itself.
(If only a minority of them needed to do it, I might think
differently, but in this case I'd vote for sticking with the
way it's being done.)
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2008-03-23 02:21:50 | Re: Logging conflicted queries on deadlocks |
Previous Message | Tom Lane | 2008-03-23 01:21:51 | Re: pg_dump -i wording |