Re: Changing baserel to foreignrel in postgres_fdw functions - Mailing list pgsql-hackers

Should this patch be applied?

---------------------------------------------------------------------------

On Thu, Feb 15, 2018 at 06:57:50PM +0530, Ashutosh Bapat wrote:
> Hi,
> I noticed that functions is_foreign_expr(), classifyConditions() and
> appendOrderByClause() had variables/arguments named baserel when the
> relations passed to those could be join or upper relation as well.
> Here's patch renaming those as foreignrel.
> -- 
> Best Wishes,
> Ashutosh Bapat
> EnterpriseDB Corporation
> The Postgres Database Company

> diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
> index e111b09..bcf9bea 100644
> --- a/contrib/postgres_fdw/deparse.c
> +++ b/contrib/postgres_fdw/deparse.c
> @@ -198,7 +198,7 @@ static void get_relation_column_alias_ids(Var *node, RelOptInfo *foreignrel,
>   */
>  void
>  classifyConditions(PlannerInfo *root,
> -                   RelOptInfo *baserel,
> +                   RelOptInfo *foreignrel,
>                     List *input_conds,
>                     List **remote_conds,
>                     List **local_conds)
> @@ -212,7 +212,7 @@ classifyConditions(PlannerInfo *root,
>      {
>          RestrictInfo *ri = lfirst_node(RestrictInfo, lc);
>  
> -        if (is_foreign_expr(root, baserel, ri->clause))
> +        if (is_foreign_expr(root, foreignrel, ri->clause))
>              *remote_conds = lappend(*remote_conds, ri);
>          else
>              *local_conds = lappend(*local_conds, ri);
> @@ -224,29 +224,29 @@ classifyConditions(PlannerInfo *root,
>   */
>  bool
>  is_foreign_expr(PlannerInfo *root,
> -                RelOptInfo *baserel,
> +                RelOptInfo *foreignrel,
>                  Expr *expr)
>  {
>      foreign_glob_cxt glob_cxt;
>      foreign_loc_cxt loc_cxt;
> -    PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) (baserel->fdw_private);
> +    PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) (foreignrel->fdw_private);
>  
>      /*
>       * Check that the expression consists of nodes that are safe to execute
>       * remotely.
>       */
>      glob_cxt.root = root;
> -    glob_cxt.foreignrel = baserel;
> +    glob_cxt.foreignrel = foreignrel;
>  
>      /*
>       * For an upper relation, use relids from its underneath scan relation,
>       * because the upperrel's own relids currently aren't set to anything
>       * meaningful by the core code.  For other relation, use their own relids.
>       */
> -    if (IS_UPPER_REL(baserel))
> +    if (IS_UPPER_REL(foreignrel))
>          glob_cxt.relids = fpinfo->outerrel->relids;
>      else
> -        glob_cxt.relids = baserel->relids;
> +        glob_cxt.relids = foreignrel->relids;
>      loc_cxt.collation = InvalidOid;
>      loc_cxt.state = FDW_COLLATE_NONE;
>      if (!foreign_expr_walker((Node *) expr, &glob_cxt, &loc_cxt))
> @@ -301,7 +301,7 @@ foreign_expr_walker(Node *node,
>      if (node == NULL)
>          return true;
>  
> -    /* May need server info from baserel's fdw_private struct */
> +    /* May need server info from foreignrel's fdw_private struct */
>      fpinfo = (PgFdwRelationInfo *) (glob_cxt->foreignrel->fdw_private);
>  
>      /* Set up inner_cxt for possible recursion to child nodes */
> @@ -2965,7 +2965,7 @@ appendGroupByClause(List *tlist, deparse_expr_cxt *context)
>  }
>  
>  /*
> - * Deparse ORDER BY clause according to the given pathkeys for given base
> + * Deparse ORDER BY clause according to the given pathkeys for given foreign
>   * relation. From given pathkeys expressions belonging entirely to the given
>   * base relation are obtained and deparsed.
>   */
> @@ -2975,7 +2975,7 @@ appendOrderByClause(List *pathkeys, deparse_expr_cxt *context)
>      ListCell   *lcell;
>      int            nestlevel;
>      char       *delim = " ";
> -    RelOptInfo *baserel = context->scanrel;
> +    RelOptInfo *foreignrel = context->scanrel;
>      StringInfo    buf = context->buf;
>  
>      /* Make sure any constants in the exprs are printed portably */
> @@ -2987,7 +2987,7 @@ appendOrderByClause(List *pathkeys, deparse_expr_cxt *context)
>          PathKey    *pathkey = lfirst(lcell);
>          Expr       *em_expr;
>  
> -        em_expr = find_em_expr_for_rel(pathkey->pk_eclass, baserel);
> +        em_expr = find_em_expr_for_rel(pathkey->pk_eclass, foreignrel);
>          Assert(em_expr != NULL);
>  
>          appendStringInfoString(buf, delim);


-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.