Обсуждение: COPY and turning off autocommit

I've found that since COPY does not use the sendQueryPreamble method
in QueryExecutorImpl, it fails to issue the necessary BEGIN if it's
the first statement after autocommit is set to off.

A workaround is to just issue a "SELECT 1" before the COPY, but this
is pretty ugly.

I'm not quite sure what the proper fix is: does COPY need to go
through the full sendQueryPreamble code path (specifically,
processDeadParsedQueries() and processDeadPortals())? Also, that code
is tied into the ResultHandler interface, and COPY does not really fit
well into that paradigm. I don't think we can we just call
sendQueryPreamble(), sendSync(), processResults(), and *then* move to
COPY code, since sendQueryPreamble() might do nothing if we're already
in a transaction. I suppose we could pass in a null ResultHandler and
see if the handler returned is null (in which case there is no BEGIN
to process), but that seems egregiously ugly.

Any thoughts?

--
Maciek Sakrejda | Software Engineer | Truviso

1065 E. Hillsdale Blvd., Suite 230
Foster City, CA 94404
(650) 242-3500 Main
(650) 242-3501 F
www.truviso.com

Attaching a patch to add a failing test case that illustrates the
problem against CVS HEAD.

I'll try mucking about with the possible answers to my above questions
and see if I can get the suite to pass, but I'd appreciate feedback
from anyone more familiar with the protocol.

Thanks,
--
Maciek Sakrejda | Software Engineer | Truviso

1065 E. Hillsdale Blvd., Suite 230
Foster City, CA 94404
(650) 242-3500 Main
(650) 242-3501 F
www.truviso.com

This time with attachment.

--
Maciek Sakrejda | Software Engineer | Truviso

1065 E. Hillsdale Blvd., Suite 230
Foster City, CA 94404
(650) 242-3500 Main
(650) 242-3501 F
www.truviso.com

The attached fixes the issue by adapting the internal API to mimic the
Fastpath subprotocol with respect to transactions. It does not include
the test patch above.

The full suite (with the added test) passes with postgresql 8.3 under
both Java 5 and 6. I don't have 1.4 handy, but I did make the
necessary (trivial) change to the v2 QueryExecutorImpl.

Any thoughts?

--
Maciek Sakrejda | Software Engineer | Truviso

1065 E. Hillsdale Blvd., Suite 230
Foster City, CA 94404
(650) 242-3500 Main
(650) 242-3501 F
www.truviso.com

On Thu, 19 Nov 2009, Maciek Sakrejda wrote:

> The attached fixes the issue by adapting the internal API to mimic the
> Fastpath subprotocol with respect to transactions. It does not include
> the test patch above.
>
> Any thoughts?

This looks like the right approach at a quick glance.  I'm going on
vacation next week, but when I return I will review and commit.

Kris Jurka

On Thu, 19 Nov 2009, Maciek Sakrejda wrote:

> The attached fixes the issue by adapting the internal API to mimic the
> Fastpath subprotocol with respect to transactions. It does not include
> the test patch above.

Committed.  Good catch, good fix, thanks.

Kris Jurka