Horton

Re: why do we need two snapshots per query?

Dimitri Fontaine <dimitri@2ndquadrant.fr>

View thread
From: Dimitri Fontaine <dimitri@2ndQuadrant.fr>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Florian Pflug <fgp@phlo.org>, Tom Lane <tgl@sss.pgh.pa.us>, Simon Riggs <simon@2ndquadrant.com>, Dimitri Fontaine <dimitri@2ndquadrant.fr>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2011-11-26T19:50:46Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix copyright notices, other minor editing in new range-types code.

  2. Redesign the plancache mechanism for more flexibility and efficiency.

Hi,

Robert Haas <robertmhaas@gmail.com> writes:
> On Sun, Nov 13, 2011 at 8:57 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>> In the -M extended case, we take a snapshot from exec_parse_message(),
>> and the same two in the exec_bind_message() call that are taken in the
>> -M prepared case.  So reducing the prepared case from two snapshots to
>> one will reduce the extended case from three snapshots to two, thus
>> saving one snapshot per query regardless of how it's executed.

I like the idea and I think it's better semantics to use the same
snapshot for planning and executing in the simple query case.

I didn't try to reproduce the performance benefits seen by Robert here,
nor did I tried to double check to compilation warnings etc.  I guess
that reviewing a commiter's patch allows for being not as thorough :)

> +	/* Done with the snapshot used for parameter I/O and parsing/planning */
> +	if (snapshot_set)
> +		PopActiveSnapshot();

This comment needs adjusting.

> diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
> index 466727b..c41272b 100644
> --- a/src/backend/tcop/pquery.c
> +++ b/src/backend/tcop/pquery.c
> @@ -455,7 +455,7 @@ FetchStatementTargetList(Node *stmt)
>   * tupdesc (if any) is known.
>   */
>  void
> -PortalStart(Portal portal, ParamListInfo params, Snapshot snapshot)
> +PortalStart(Portal portal, ParamListInfo params, bool use_active_snapshot)

You need to be editing the comments for this function.  To be specific
you didn't update this text:

 * The caller can optionally pass a snapshot to be used; pass InvalidSnapshot
 * for the normal behavior of setting a new snapshot.  This parameter is
 * presently ignored for non-PORTAL_ONE_SELECT portals (it's only intended
 * to be used for cursors).

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support