On 2015/04/08 7:44, Tom Lane wrote: > Etsuro Fujita <fujita.ets...@lab.ntt.co.jp> writes: >> To support ROW_MARK_REFERENCE on (postgres_fdw) foreign tables, I'd like >> to propose the following FDW APIs: > >> RowMarkType >> GetForeignRowMarkType(Oid relid, >> LockClauseStrength strength); > >> Decide which rowmark type to use for a foreign table (that has strength >> = LCS_NONE), ie, ROW_MARK_REFERENCE or ROW_MARK_COPY. (For now, the >> second argument takes LCS_NONE only, but is intended to be used for the >> possible extension to the other cases.) This is called during >> select_rowmark_type() in the planner. > > Why would you restrict that to LCS_NONE? Seems like the point is to give > the FDW control of the rowmark behavior, not only partial control.
The reason is because I think it's comparatively more promissing to customize the ROW_MARK type for LCS_NONE than other cases since in many workloads no re-fetch is needed, and because I think other cases would need more discussions. So, as a first step, I restricted that to LCS_NONE. But I've got the point, and agree that we should give the full control to the FDW. > (For the same reason I disagree with the error check in the patch that > restricts which ROW_MARK options this function can return. If the FDW has > TIDs, seems like it could reasonably use whatever options tables can use.) Will fix. >> void >> BeginForeignFetch(EState *estate, >> ExecRowMark *erm, >> List *fdw_private, >> int eflags); > >> Begin a remote fetch. This is called during InitPlan() in the executor. > > The begin/end functions seem like useless extra mechanism. Why wouldn't > the FDW just handle this during its regular scan setup? It could look to > see whether the foreign table is referenced by any ExecRowMarks (possibly > there's room to add some convenience functions to help with that). What's > more, that design would make it simpler if the basic row fetch needs to be > modified. The reason is just because it's easy to understand the structure at least to me since the begin/exec/end are all done in a higher level of the executor. What's more, the begin/end can be done once per foreign scan node for the multi-table update case. But I feel inclined to agree with you on this point also. >> And I'd also like to propose to add a table/server option, >> row_mark_reference, to postgres_fdw. When a user sets the option to >> true for eg a foreign table, ROW_MARK_REFERENCE will be used for the >> table, not ROW_MARK_COPY. > > Why would we leave that in the hands of the user? Hardly anybody is going > to know what to do with the option, or even that they should do something > with it. It's basically only of value for debugging AFAICS, Agreed. (When begining to update postgres_fdw docs, I also started to think so.) I'll update the patch, which will contain only an infrastructure for this in the PG core, and will not contain any postgres_fdw change. Thank you for taking the time to review the patch! 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
