On Wed, Dec 10, 2014 at 1:20 AM, Heikki Linnakangas <
hlinnakangas@vmware.com> wrote:
>> 6) I am getting many regression failures after applying this patch and
>> running the tests on OSX, please see attached.
>
>
> Attached is a new version [1], with a lot of small fixes here and there. It
> passes all the regression tests for me. Can you try again with this version?
> If it's still failing on OS X, will need to investigate.
With this version regression tests are passing on all my OSX/Linux dev machines. At least I do not see failures directly related to it.
Few comments:
1) Here an error message would be nice:
+ /*
+ * XXX: we don't check the result here. Should we? We're rolling back,
+ * so it's not clear what else we can do on error. Giving an error
+ * message to the application would be nice though.
+ */
+ if (pgres != NULL)
+ {
+ PQclear(pgres);
+ pgres = NULL;
+ }
2) Is this planned to be updated after this patch?
+
+ /*
+ * Update our copy of the transaction status.
+ *
+ * XXX: Once we stop using the socket directly, and do everything with
+ * libpq, we can get rid of the transaction_status field altogether
+ * and always ask libpq for it.
+ */
+ LIBPQ_update_transaction_status(self);
3) Did you do some performance tests here?
+ /*
+ * XXX: We need to do Prepare + Describe as two different round-trips
+ * to the server, while without libpq we send a Parse and Describe
+ * message followed by a single Sync.
+ */
Except that this looks good, and numbers speak by themselves:
$ git diff master --stat | tail -n1
53 files changed, 1699 insertions(+), 8171 deletions(-)
Regards,
--
Michael