Hello, > >> analyze.c: In function ‘acquire_inherited_sample_rows’: > >> analyze.c:1461: warning: unused variable ‘saved_rel’ > > I've fixed this in the latest version (v8) of the patch.
Mmm. sorry. I missed v8 patch. Then I had a look on that and have a question. You've added a check for relkind of baserel of the foreign path to be reparameterized. If this should be an assertion - not a conditional branch -, it would be better to put the assertion in create_foreignscan_path instead of there. ===== --- a/src/backend/optimizer/util/pathnode.c +++ b/src/backend/optimizer/util/pathnode.c @@ -1723,6 +1723,7 @@ create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel, List *fdw_private) { ForeignPath *pathnode = makeNode(ForeignPath); + Assert(rel->rtekind == RTE_RELATION); pathnode->path.pathtype = T_ForeignScan; pathnode->path.parent = rel; ===== > > And for file-fdw, you made a change to re-create foreignscan node > > instead of the previous copy-and-modify. Is the reason you did it > > that you considered the cost of 're-checking whether to > > selectively perform binary conversion' is low enough? Or other > > reasons? > > The reason is that we get the result of the recheck from > path->fdw_private. Sorry, I'd forgotten it. So, I modified the code to > simply call create_foreignscan_path(). Anyway you new code seems closer to the basics and the gain by the previous optimization don't seem to be significant.. > > Finally, although I insist the necessity of the warning for child > > foreign tables on alter table, if you belive it to be put off, > > I'll compromise by putting a note to CF-app that last judgement > > is left to committer. > > OK So, if there are no objections of other, I'll mark this patch as > "ready for committer" and do that. Thank you. regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers