Re: Speed dblink using alternate libpq tuple storage

From: Marko Kreen <markokr(at)gmail(dot)com>
To: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)oss(dot)ntt(dot)co(dot)jp>
Cc: shigeru(dot)hanada(at)gmail(dot)com, mmoncure(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org, greg(at)2ndquadrant(dot)com
Subject: Re: Speed dblink using alternate libpq tuple storage
Date: 2012-02-16 15:17:25
Message-ID: 20120216151725.GA9208@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Feb 16, 2012 at 05:49:34PM +0900, Kyotaro HORIGUCHI wrote:
> I added the function PQskipRemainingResult() and use it in
> dblink. This reduces the number of executing try-catch block from
> the number of rows to one per query in dblink.

This implementation is wrong - you must not simply call PQgetResult()
when connection is in async mode. And the resulting PGresult must
be freed.

Please just make PGsetRowProcessorErrMsg() callable outside from
callback. That also makes clear to code that sees final PGresult
what happened. As a bonus, this will simply make the function
more robust and less special.

Although it's easy to create some PQsetRowSkipFlag() function
that will silently skip remaining rows, I think such logic
is best left to user to handle. It creates unnecessary special
case when handling final PGresult, so in the big picture
it creates confusion.

> This patch is based on the patch above and composed in the same
> manner - main three patches include all modifications and the '2'
> patch separately.

I think there is not need to carry the early-exit patch separately.
It is visible in archives and nobody screamed about it yet,
so I guess it's acceptable. Also it's so simple, there is no
point in spending time rebasing it.

> diff --git a/src/interfaces/libpq/#fe-protocol3.c# b/src/interfaces/libpq/#fe-protocol3.c#

There is some backup file in your git repo.

--
marko

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kohei KaiGai 2012-02-16 17:02:59 Re: pgsql_fdw, FDW for PostgreSQL server
Previous Message Albe Laurenz 2012-02-16 15:15:55 Re: pgsql_fdw, FDW for PostgreSQL server