Re: tuplestore_putvalues()

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

In response to

Responses

Browse pgsql-patches by date

  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