Minor comment improvement to create_foreignscan_plan

Started by Etsuro Fujitaabout 10 years ago8 messages
#1Etsuro Fujita
fujita.etsuro@lab.ntt.co.jp
1 attachment(s)

Hi,

Here is a small patch to update an comment in create_foreignscan_plan;
add fdw_recheck_quals to the list of expressions that need the
replace_nestloop_params processing. I should have updated the comment
when I proposed the patch for the fdw_recheck_quals.

Best regards,
Etsuro Fujita

Attachments:

create-foreignscan-plan-comment-update.patchtext/x-patch; name=create-foreignscan-plan-comment-update.patchDownload
*** a/src/backend/optimizer/plan/createplan.c
--- b/src/backend/optimizer/plan/createplan.c
***************
*** 2141,2151 **** create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
  	scan_plan->fs_relids = best_path->path.parent->relids;
  
  	/*
! 	 * Replace any outer-relation variables with nestloop params in the qual
! 	 * and fdw_exprs expressions.  We do this last so that the FDW doesn't
! 	 * have to be involved.  (Note that parts of fdw_exprs could have come
! 	 * from join clauses, so doing this beforehand on the scan_clauses
! 	 * wouldn't work.)  We assume fdw_scan_tlist contains no such variables.
  	 */
  	if (best_path->path.param_info)
  	{
--- 2141,2152 ----
  	scan_plan->fs_relids = best_path->path.parent->relids;
  
  	/*
! 	 * Replace any outer-relation variables with nestloop params in the qual,
! 	 * fdw_exprs and fdw_recheck_quals expressions.  We do this last so that
! 	 * the FDW doesn't have to be involved.  (Note that parts of fdw_exprs
! 	 * or fdw_recheck_quals could have come from join clauses, so doing this
! 	 * beforehand on the scan_clauses wouldn't work.)  We assume
! 	 * fdw_scan_tlist contains no such variables.
  	 */
  	if (best_path->path.param_info)
  	{
#2Robert Haas
robertmhaas@gmail.com
In reply to: Etsuro Fujita (#1)
Re: Minor comment improvement to create_foreignscan_plan

On Mon, Nov 9, 2015 at 5:34 AM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:

Here is a small patch to update an comment in create_foreignscan_plan;
add fdw_recheck_quals to the list of expressions that need the
replace_nestloop_params processing. I should have updated the comment
when I proposed the patch for the fdw_recheck_quals.

OK, not a big deal, but thanks. 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

#3Etsuro Fujita
fujita.etsuro@lab.ntt.co.jp
In reply to: Robert Haas (#2)
Re: Minor comment improvement to create_foreignscan_plan

On 2015/11/10 3:53, Robert Haas wrote:

On Mon, Nov 9, 2015 at 5:34 AM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:

Here is a small patch to update an comment in create_foreignscan_plan;
add fdw_recheck_quals to the list of expressions that need the
replace_nestloop_params processing. I should have updated the comment
when I proposed the patch for the fdw_recheck_quals.

OK, not a big deal, but thanks. Committed.

Thanks!

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

#4Etsuro Fujita
fujita.etsuro@lab.ntt.co.jp
In reply to: Etsuro Fujita (#3)
1 attachment(s)
Re: Minor comment improvement to create_foreignscan_plan

On 2015/11/12 19:02, Etsuro Fujita wrote:

On 2015/11/10 3:53, Robert Haas wrote:

On Mon, Nov 9, 2015 at 5:34 AM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:

Here is a small patch to update an comment in create_foreignscan_plan;
add fdw_recheck_quals to the list of expressions that need the
replace_nestloop_params processing. I should have updated the comment
when I proposed the patch for the fdw_recheck_quals.

OK, not a big deal, but thanks. Committed.

Thanks!

Oops, I've found another one. I think we should update a comment in
postgresGetForeignPlan, too; add remote filtering expressions to the
list of information needed to create a ForeignScan node.

Best regards,
Etsuro Fujita

Attachments:

postgresGetForeignPlan-comment.patchtext/x-diff; name=postgresGetForeignPlan-comment.patchDownload
*** a/contrib/postgres_fdw/postgres_fdw.c
--- b/contrib/postgres_fdw/postgres_fdw.c
***************
*** 902,908 **** postgresGetForeignPlan(PlannerInfo *root,
  							 retrieved_attrs);
  
  	/*
! 	 * Create the ForeignScan node from target list, local filtering
  	 * expressions, remote parameter expressions, and FDW private information.
  	 *
  	 * Note that the remote parameter expressions are stored in the fdw_exprs
--- 902,908 ----
  							 retrieved_attrs);
  
  	/*
! 	 * Create the ForeignScan node from target list, remote/local filtering
  	 * expressions, remote parameter expressions, and FDW private information.
  	 *
  	 * Note that the remote parameter expressions are stored in the fdw_exprs
#5Robert Haas
robertmhaas@gmail.com
In reply to: Etsuro Fujita (#4)
1 attachment(s)
Re: Minor comment improvement to create_foreignscan_plan

On Sun, Nov 15, 2015 at 9:25 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:

Oops, I've found another one. I think we should update a comment in
postgresGetForeignPlan, too; add remote filtering expressions to the list of
information needed to create a ForeignScan node.

Instead of saying "remote/local", how about saying "remote and local"
or just leaving it out altogether as in the attached?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachments:

pgfdw-comment-fix.patchtext/x-diff; charset=US-ASCII; name=pgfdw-comment-fix.patchDownload
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index cd4ed0c..a6ba672 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -902,8 +902,8 @@ postgresGetForeignPlan(PlannerInfo *root,
 							 retrieved_attrs);
 
 	/*
-	 * Create the ForeignScan node from target list, local filtering
-	 * expressions, remote parameter expressions, and FDW private information.
+	 * Create the ForeignScan node from target list, filtering expressions,
+	 * remote parameter expressions, and FDW private information.
 	 *
 	 * Note that the remote parameter expressions are stored in the fdw_exprs
 	 * field of the finished plan node; we can't keep them in private state
#6Etsuro Fujita
fujita.etsuro@lab.ntt.co.jp
In reply to: Robert Haas (#5)
Re: Minor comment improvement to create_foreignscan_plan

On 2015/11/18 2:57, Robert Haas wrote:

On Sun, Nov 15, 2015 at 9:25 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:

Oops, I've found another one. I think we should update a comment in
postgresGetForeignPlan, too; add remote filtering expressions to the list of
information needed to create a ForeignScan node.

Instead of saying "remote/local", how about saying "remote and local"
or just leaving it out altogether as in the attached?

+1 for your 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

#7Robert Haas
robertmhaas@gmail.com
In reply to: Etsuro Fujita (#6)
Re: Minor comment improvement to create_foreignscan_plan

On Tue, Nov 17, 2015 at 9:29 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:

On 2015/11/18 2:57, Robert Haas wrote:

On Sun, Nov 15, 2015 at 9:25 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:

Oops, I've found another one. I think we should update a comment in
postgresGetForeignPlan, too; add remote filtering expressions to the list
of
information needed to create a ForeignScan node.

Instead of saying "remote/local", how about saying "remote and local"
or just leaving it out altogether as in the attached?

+1 for your patch.

OK, 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

#8Etsuro Fujita
fujita.etsuro@lab.ntt.co.jp
In reply to: Robert Haas (#7)
Re: Minor comment improvement to create_foreignscan_plan

On 2015/11/19 5:29, Robert Haas wrote:

On Tue, Nov 17, 2015 at 9:29 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:

On 2015/11/18 2:57, Robert Haas wrote:

On Sun, Nov 15, 2015 at 9:25 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:

Oops, I've found another one. I think we should update a comment in
postgresGetForeignPlan, too; add remote filtering expressions to the list
of
information needed to create a ForeignScan node.

Instead of saying "remote/local", how about saying "remote and local"
or just leaving it out altogether as in the attached?

+1 for your patch.

OK, committed.

Thanks!

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