From: | Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp |
Cc: | shigeru(dot)hanada(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: inherit support for foreign tables |
Date: | 2014-03-27 08:09:17 |
Message-ID: | 20140327.170917.91664276.horiguchi.kyotaro@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Rajeev rastogi | 2014-03-27 08:12:38 | datistemplate of pg_database does not behave as per description in documentation |
Previous Message | Dean Rasheed | 2014-03-27 08:04:56 | Re: [PATCH] Negative Transition Aggregate Functions (WIP) |