Etsuro Fujita <fujita.ets...@lab.ntt.co.jp> writes: > [ EvalPlanQual-v6.patch ]
I've started to study this in a little more detail, and I'm not terribly happy with some of the API decisions in it. In particular, I find the addition of "void *fdw_state" to ExecRowMark to be pretty questionable. That does not seem like a good place to keep random state. (I realize that WHERE CURRENT OF keeps some state in ExecRowMark, but that's a crock not something to emulate.) ISTM that in most scenarios, the state that LockForeignRow/FetchForeignRow would like to get at is probably the FDW state associated with the ForeignScanState that the tuple came from. Which this API doesn't help with particularly. I wonder if we should instead add a "ScanState*" field and expect the core code to set that up (ExecOpenScanRelation could do it with minor API changes...). I'm also a bit tempted to pass the TIDs to LockForeignRow and FetchForeignRow as Datums not ItemPointers. We have the Datum format available already at the call sites, so this is free as far as the core code is concerned, and would only cost another line or so for the FDWs. This is by no means sufficient to allow FDWs to use some other type than "tid" for row identifiers; but it would be a down payment on that problem, and at least would avoid nailing the rowids-are-tids assumption into yet another global API. Thoughts? Also, as I mentioned, I'd be a whole lot happier if we had a way to test this... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers