Minor code improvement to postgresGetForeignPlan

Started by Tatsuro Yamadaalmost 9 years ago5 messages
#1Tatsuro Yamada
yamada.tatsuro@lab.ntt.co.jp
1 attachment(s)

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

Attachments:

fix_declaration_of_postgresGetForeignPlan.patchtext/x-patch; name=fix_declaration_of_postgresGetForeignPlan.patchDownload
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,
#2Etsuro Fujita
fujita.etsuro@lab.ntt.co.jp
In reply to: Tatsuro Yamada (#1)
Re: Minor code improvement to postgresGetForeignPlan

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

#3Tatsuro Yamada
yamada.tatsuro@lab.ntt.co.jp
In reply to: Etsuro Fujita (#2)
Re: Minor code improvement to postgresGetForeignPlan

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

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tatsuro Yamada (#1)
Re: Minor code improvement to postgresGetForeignPlan

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

#5Tatsuro Yamada
yamada.tatsuro@lab.ntt.co.jp
In reply to: Tom Lane (#4)
Re: Minor code improvement to postgresGetForeignPlan

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