On 2016/01/20 3:42, Robert Haas wrote:
On Tue, Jan 19, 2016 at 1:59 AM, Etsuro Fujita
<fujita.ets...@lab.ntt.co.jp> wrote:
I've run into an issue:
*# UPDATE master_customers SET id = 22 WHERE id = 16 RETURNING
tableoid::regclass;
ERROR:
CONTEXT: Remote SQL command: UPDATE public.customers SET id = 22
WHERE ((id = 16)) RETURNING NULL
While working on this, I noticed that the existing postgres_fdw system
shows similar behavior, so I changed the subject.
IIUC, the reason for that is when the local query specifies "RETURNING
tableoid::regclass", the FDW has fmstate->has_returning=false while the
remote query executed at ModifyTable has "RETURNING NULL", as shown in
the above example; that would cause an abnormal exit in executing the
remote query in postgresExecForeignUpdate, since that the FDW would get
PGRES_TUPLES_OK as a result of the query while the FDW would think that
the right result to get should be PGRES_COMMAND_OK, from the flag
fmstate->has_returning=false.
Attached is a patch to fix that.
I added this to the next CF.
https://commitfest.postgresql.org/9/483/
Uggh, what a mess. How about passing an additional boolean
"is_returning" to deparseTargetList()? If false, then
deparseTargetList() behaves as now. If false, then
deparseTargetList() doesn't append anything at all if there are no
columns to return, instead of (as at present) adding NULL. On the
other hand, if there are columns to return, then it appends "
RETURNING " before the first column. Then, deparseReturningList could
skip adding RETURNING itself, and just pass true to
deparseTargetList(). The advantage of this approach is that we don't
end up with two copies of the code that have to stay synchronized -
Thanks for the review! I think that is important.
the decision is made inside deparseTargetList(), and
deparseReturningList() accepts the results.
My concern about that is that would make the code in deparseTargetList()
complicated.
Essentially, I think your propossal needs a two-pass algorithm for
deparseTargetList; (1) create an integer List of the columns being
retrieved from the given attrs_used (getRetrievedAttrs()), and (2) print
those columns (printRetrievedAttrs()). How about sharing those two
functions between deparseTargetList and deparseReturningList?:
* In deparseTargetList, perform getRetrievedAttrs(). If
getRetrievedAttrs()!=NIL, perform printRetrievedAttrs(). Otherwise,
print NULL.
* In deparseReturningList, perform getRetrievedAttrs() before adding
RETURNING. If getRetrievedAttrs()!=NIL, print RETURNING and perform
printRetrievedAttrs(). Otherwise, do nothing.
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
