Hanada-san, > Thanks for further review, but I found two bugs in v10 patch. > I’ve fixed them and wrapped up v11 patch here. > > * Fix bug about illegal column order > > Scan against a base relation returns columns in order of column definition, > but > its target list might require different order. This can be resolved by tuple > projection in usual cases, but pushing down joins skips the step, so we need > to > treat it in remote query. > > Before this fix, deparseProjectionSql() was called only for queries which have > ctid or whole-row reference in its target list, but it was a too-much > optimization. > We always need to call it, because usual column list might require ordering > conversion. Checking ordering is not impossible, but it seems useless effort. > > Another way to resolve this issue is to reorder SELECT clause of a query for > base > relation if it was a source of a join, but it requires stepping back in > planning, > so the fix above was chosen. > > "three tables join" test case is also changed to check this behavior. > Sorry for my oversight. Yep, var-node reference on join relation cannot expect any column orders predefined like as base relations. All reasonable way I know is, relying on the targetlist of the RelOptInfo that contains all the referenced columns in the later stage.
> * Fix bug of duplicate fdw_ps_tlist contents. > > I coded that deparseSelectSql passes fdw_ps_tlist to deparseSelectSql for > underlying RelOptInfo, but it causes redundant entries in fdw_ps_tlist in > cases > of joining more than two foreign tables. I changed to pass NULL as > fdw_ps_tlist > for recursive call of deparseSelectSql. > It's reasonable, and also makes performance benefit because descriptor constructed based on the ps_tlist will match expected result tuple, so it allows to avoid unnecessary projection. > * Fix typos > > Please review the v11 patch, and mark it as “ready for committer” if it’s ok. > It's OK for me, and wants to be reviewed by other people to get it committed. > In addition to essential features, I tried to implement relation listing in > EXPLAIN > output. > > Attached explain_forein_join.patch adds capability to show join combination of > a ForeignScan in EXPLAIN output as an additional item “Relations”. I thought > that using array to list relations is a good way too, but I chose one string > value > because users would like to know order and type of joins too. > A bit different from my expectation... I expected to display name of the local foreign tables (and its alias), not remote one, because all the local join logic displays local foreign tables name. Is it easy to adjust isn't it? Probably, all you need to do is, putting a local relation name on the text buffer (at deparseSelectSql) instead of the deparsed remote relation. Thanks, -- NEC Business Creation Division / PG-Strom Project KaiGai Kohei <kai...@ak.jp.nec.com> -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
