pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedMinor code improvement to postgresGetForeignJoinPaths
10+ messages / 5 participants
[nested] [flat]
* Minor code improvement to postgresGetForeignJoinPaths
@ 2016-10-24 02:26 Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
2016-10-24 06:45 ` Re: Minor code improvement to postgresGetForeignJoinPaths Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
0 siblings, 1 reply; 10+ messages in thread
From: Tatsuro Yamada @ 2016-10-24 02:26 UTC (permalink / raw)
To: pgsql-hackers
Hi,
The last argument of create_foreignscan_path called by postgresGetForeignJoinPaths is
set to NULL, but it would be suitable to set it to NIL because the argument type is List.
Please find attached a patch.
Tatsuro Yamada
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
Attachments:
postgresGetForeignJoinPaths.patch (text/x-patch, 550B)
download | inline diff:
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index daf0438..a5de611 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -4331,7 +4331,7 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
NIL, /* no pathkeys */
NULL, /* no required_outer */
epq_path,
- NULL); /* no fdw_private */
+ NIL); /* no fdw_private */
/* Add generated path into joinrel by add_path(). */
add_path(joinrel, (Path *) joinpath);
* Re: Minor code improvement to postgresGetForeignJoinPaths
2016-10-24 02:26 Minor code improvement to postgresGetForeignJoinPaths Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
@ 2016-10-24 06:45 ` Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
2016-10-25 05:59 ` Re: Minor code improvement to postgresGetForeignJoinPaths Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
0 siblings, 1 reply; 10+ messages in thread
From: Ashutosh Bapat @ 2016-10-24 06:45 UTC (permalink / raw)
To: Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>; +Cc: pgsql-hackers
On Mon, Oct 24, 2016 at 7:56 AM, Tatsuro Yamada
<yamada.tatsuro@lab.ntt.co.jp> wrote:
> Hi,
>
> The last argument of create_foreignscan_path called by postgresGetForeignJoinPaths is
> set to NULL, but it would be suitable to set it to NIL because the argument type is List.
>
> Please find attached a patch.
>
You are right. Every call except that one is using NIL, so better to
fix it. The pattern was repeated in the recent aggregate pushdown
support. Here's patch to fix create_foreignscan_path() call in
add_foreign_grouping_paths() as well.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachments:
postgresGetForeignJoinPaths_v2.patch (text/x-diff, 1.7K)
download | inline diff:
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 906d6e6..2cfb82b 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -4424,21 +4424,21 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
*/
joinpath = create_foreignscan_path(root,
joinrel,
NULL, /* default pathtarget */
rows,
startup_cost,
total_cost,
NIL, /* no pathkeys */
NULL, /* no required_outer */
epq_path,
- NULL); /* no fdw_private */
+ NIL); /* no fdw_private */
/* Add generated path into joinrel by add_path(). */
add_path(joinrel, (Path *) joinpath);
/* Consider pathkeys for the join relation */
add_paths_with_pathkeys_for_rel(root, joinrel, epq_path);
/* XXX Consider parameterized paths for the join relation */
}
@@ -4741,21 +4741,21 @@ add_foreign_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
/* Create and add foreign path to the grouping relation. */
grouppath = create_foreignscan_path(root,
grouped_rel,
grouping_target,
rows,
startup_cost,
total_cost,
NIL, /* no pathkeys */
NULL, /* no required_outer */
NULL,
- NULL); /* no fdw_private */
+ NIL); /* no fdw_private */
/* Add generated path into grouped_rel by add_path(). */
add_path(grouped_rel, (Path *) grouppath);
}
/*
* Create a tuple from the specified row of the PGresult.
*
* rel is the local representation of the foreign table, attinmeta is
* conversion data for the rel's tupdesc, and retrieved_attrs is an
* Re: Minor code improvement to postgresGetForeignJoinPaths
2016-10-24 02:26 Minor code improvement to postgresGetForeignJoinPaths Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
2016-10-24 06:45 ` Re: Minor code improvement to postgresGetForeignJoinPaths Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
@ 2016-10-25 05:59 ` Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
2016-11-03 15:29 ` Re: Minor code improvement to postgresGetForeignJoinPaths Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Tatsuro Yamada @ 2016-10-25 05:59 UTC (permalink / raw)
To: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>; +Cc: pgsql-hackers
Hi Ashutosh,
> You are right. Every call except that one is using NIL, so better to
> fix it. The pattern was repeated in the recent aggregate pushdown
> support. Here's patch to fix create_foreignscan_path() call in
> add_foreign_grouping_paths() as well.
Thanks for the review!
Tatsuro Yamada
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
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Minor code improvement to postgresGetForeignJoinPaths
2016-10-24 02:26 Minor code improvement to postgresGetForeignJoinPaths Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
2016-10-24 06:45 ` Re: Minor code improvement to postgresGetForeignJoinPaths Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
2016-10-25 05:59 ` Re: Minor code improvement to postgresGetForeignJoinPaths Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
@ 2016-11-03 15:29 ` Robert Haas <robertmhaas@gmail.com>
2016-11-09 01:09 ` Re: Minor code improvement to postgresGetForeignJoinPaths Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
0 siblings, 1 reply; 10+ messages in thread
From: Robert Haas @ 2016-11-03 15:29 UTC (permalink / raw)
To: Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>; +Cc: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>; pgsql-hackers
On Tue, Oct 25, 2016 at 1:59 AM, Tatsuro Yamada
<yamada.tatsuro@lab.ntt.co.jp> wrote:
> Hi Ashutosh,
>> You are right. Every call except that one is using NIL, so better to
>> fix it. The pattern was repeated in the recent aggregate pushdown
>> support. Here's patch to fix create_foreignscan_path() call in
>> add_foreign_grouping_paths() as well.
>
> Thanks for the review!
Committed.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Minor code improvement to postgresGetForeignJoinPaths
2016-10-24 02:26 Minor code improvement to postgresGetForeignJoinPaths Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
2016-10-24 06:45 ` Re: Minor code improvement to postgresGetForeignJoinPaths Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
2016-10-25 05:59 ` Re: Minor code improvement to postgresGetForeignJoinPaths Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
2016-11-03 15:29 ` Re: Minor code improvement to postgresGetForeignJoinPaths Robert Haas <robertmhaas@gmail.com>
@ 2016-11-09 01:09 ` Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
0 siblings, 0 replies; 10+ messages in thread
From: Tatsuro Yamada @ 2016-11-09 01:09 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>; pgsql-hackers
On 2016/11/04 0:29, Robert Haas wrote:
> Committed.
Thanks!
Tatsuro Yamada
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
^ permalink raw reply [nested|flat] 10+ messages in thread
* Minor code improvement to postgresGetForeignPlan
@ 2017-04-07 04:12 Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
2017-09-01 09:30 ` Re: Minor code improvement to postgresGetForeignPlan Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
2017-09-06 21:52 ` Re: Minor code improvement to postgresGetForeignPlan Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 10+ messages in thread
From: Tatsuro Yamada @ 2017-04-07 04:12 UTC (permalink / raw)
To: pgsql-hackers
Hi,
The declaration of postgresGetForeignPlan uses baserel, but
the actual definition uses foreignrel. It would be better to sync.
Please find attached a patch.
Tatsuro Yamada
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
Attachments:
fix_declaration_of_postgresGetForeignPlan.patch (text/x-patch, 541B)
download | inline diff:
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 2851869..54edf4d 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -280,7 +280,7 @@ static void postgresGetForeignPaths(PlannerInfo *root,
RelOptInfo *baserel,
Oid foreigntableid);
static ForeignScan *postgresGetForeignPlan(PlannerInfo *root,
- RelOptInfo *baserel,
+ RelOptInfo *foreignrel,
Oid foreigntableid,
ForeignPath *best_path,
List *tlist,
* Re: Minor code improvement to postgresGetForeignPlan
2017-04-07 04:12 Minor code improvement to postgresGetForeignPlan Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
@ 2017-09-01 09:30 ` Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
2017-09-04 06:45 ` Re: Minor code improvement to postgresGetForeignPlan Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
1 sibling, 1 reply; 10+ messages in thread
From: Etsuro Fujita @ 2017-09-01 09:30 UTC (permalink / raw)
To: Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>; pgsql-hackers
On 2017/04/07 13:12, Tatsuro Yamada wrote:> The declaration of
postgresGetForeignPlan uses baserel, but
> the actual definition uses foreignrel. It would be better to sync.
Agreed.
> Please find attached a patch.
The patch looks good to me, so I'll mark this as Ready for Committer.
(I'm not sure we should do the same thing to the function declaration in
other places such as fdwapi.h and the documentation for consistency, but
if so, I'd vote for leaving that for another 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
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Minor code improvement to postgresGetForeignPlan
2017-04-07 04:12 Minor code improvement to postgresGetForeignPlan Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
2017-09-01 09:30 ` Re: Minor code improvement to postgresGetForeignPlan Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
@ 2017-09-04 06:45 ` Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
0 siblings, 0 replies; 10+ messages in thread
From: Tatsuro Yamada @ 2017-09-04 06:45 UTC (permalink / raw)
To: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>; pgsql-hackers
Hi Fujita-san,
> The patch looks good to me, so I'll mark this as Ready for Committer.
Thanks for reviewing my patch.
> (I'm not sure we should do the same thing to the function declaration in other places such as fdwapi.h and the documentation for consistency, but if so, I'd vote for leaving that for another patch.)
+1
Regards,
Tatsuro Yamada
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Minor code improvement to postgresGetForeignPlan
2017-04-07 04:12 Minor code improvement to postgresGetForeignPlan Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
@ 2017-09-06 21:52 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-09-07 00:22 ` Re: Minor code improvement to postgresGetForeignPlan Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
1 sibling, 1 reply; 10+ messages in thread
From: Tom Lane @ 2017-09-06 21:52 UTC (permalink / raw)
To: Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>; +Cc: pgsql-hackers
Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp> writes:
> The declaration of postgresGetForeignPlan uses baserel, but
> the actual definition uses foreignrel. It would be better to sync.
Pushed, thanks.
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
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Minor code improvement to postgresGetForeignPlan
2017-04-07 04:12 Minor code improvement to postgresGetForeignPlan Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
2017-09-06 21:52 ` Re: Minor code improvement to postgresGetForeignPlan Tom Lane <tgl@sss.pgh.pa.us>
@ 2017-09-07 00:22 ` Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
0 siblings, 0 replies; 10+ messages in thread
From: Tatsuro Yamada @ 2017-09-07 00:22 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers
On 2017/09/07 6:52, Tom Lane wrote:
> Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp> writes:
>> The declaration of postgresGetForeignPlan uses baserel, but
>> the actual definition uses foreignrel. It would be better to sync.
>
> Pushed, thanks.
>
> regards, tom lane
Thanks!
Regards,
Tatsuro Yamada
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2017-09-07 00:22 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2016-10-24 02:26 Minor code improvement to postgresGetForeignJoinPaths Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
2016-10-24 06:45 ` Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
2016-10-25 05:59 ` Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
2016-11-03 15:29 ` Robert Haas <robertmhaas@gmail.com>
2016-11-09 01:09 ` Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
2017-04-07 04:12 Minor code improvement to postgresGetForeignPlan Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
2017-09-01 09:30 ` Re: Minor code improvement to postgresGetForeignPlan Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
2017-09-04 06:45 ` Re: Minor code improvement to postgresGetForeignPlan Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
2017-09-06 21:52 ` Re: Minor code improvement to postgresGetForeignPlan Tom Lane <tgl@sss.pgh.pa.us>
2017-09-07 00:22 ` Re: Minor code improvement to postgresGetForeignPlan Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox