On 2016/04/19 13:59, Michael Paquier wrote:
On Tue, Apr 19, 2016 at 1:14 PM, Etsuro Fujita
<fujita.ets...@lab.ntt.co.jp> wrote:
What do you think about that?

+   /* Wait for the result */
+   res = pgfdw_get_result(conn, query);
+   if (res == NULL)
+       pgfdw_report_error(ERROR, NULL, conn, false, query);
+   last_res = res;
+
+   /*
+    * Verify that there are no more results
+    *
+    * We don't use a PG_TRY block here, so be careful not to throw error
+    * without releasing the PGresult.
+    */
+   res = pgfdw_get_result(conn, query);
+   if (res != NULL)
+   {
+       PQclear(last_res);
+       pgfdw_report_error(ERROR, res, conn, true, query);
+   }

But huge objection to that because this fragilizes the current logic
postgres_fdw is based on: PQexec returns the last result to caller,
I'd rather not break that logic for 9.6 stability's sake.

IIUC, I think each query submitted by PQexec in postgres_fdw.c contains just a single command. Maybe I'm missing something, though.

A even better proof of that is the following, which just emulates what
your version of pgfdw_get_result is doing when consuming the results.
+   /* Verify that there are no more results */
+   res = pgfdw_get_result(fmstate->conn, fmstate->query);
+   if (res != NULL)
+       pgfdw_report_error(ERROR, res, fmstate->conn, true, fmstate->query);
This could even lead to incorrect errors in the future if multiple
queries are combined with those DMLs for a reason or another.

I'd like to leave such enhancements for future work...

Thanks for the comment!

Best regards,
Etsuro Fujita




--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to