Changing baserel to foreignrel in postgres_fdw functions

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.

On Wed, Nov 22, 2023 at 3:27 AM Bruce Momjian <bruce@momjian.us> wrote:

Should this patch be applied?

I think so.

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

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.

The patch is more than 5 years old. So it might need adjustments. E.g. the
appendOrderByClause() does not require the change anymore. But the other
two functions still require the changes. There may be other new places that
require change. I have not checked that. If we are accepting this change, I
can update the patch.

--
Best Wishes,
Ashutosh