Reusing return value from planner_rt_fetch

Started by Zhihong Yuabout 3 years ago2 messages
#1Zhihong Yu
zyu@yugabyte.com
1 attachment(s)

Hi,
I was reading examine_variable in src/backend/utils/adt/selfuncs.c

It seems we already have the rte coming out of the loop which starts on
line 5181.

Here is a patch which reuses the return value from `planner_rt_fetch`.

Please take a look.

Thanks

Attachments:

parent-rel-rte.patchapplication/octet-stream; name=parent-rel-rte.patchDownload
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 69e0fb98f5..23e3835b55 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -5178,17 +5178,17 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
 									Index		varno = index->rel->relid;
 
 									appinfo = root->append_rel_array[varno];
-									while (appinfo &&
-										   planner_rt_fetch(appinfo->parent_relid,
-															root)->rtekind == RTE_RELATION)
+									while (appinfo)
 									{
+										rte = planner_rt_fetch(appinfo->parent_relid, root);
+										if (rte->rtekind != RTE_RELATION)
+											break;
 										varno = appinfo->parent_relid;
 										appinfo = root->append_rel_array[varno];
 									}
 									if (varno != index->rel->relid)
 									{
 										/* Repeat access check on this rel */
-										rte = planner_rt_fetch(varno, root);
 										Assert(rte->rtekind == RTE_RELATION);
 
 										userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Zhihong Yu (#1)
Re: Reusing return value from planner_rt_fetch

Zhihong Yu <zyu@yugabyte.com> writes:

I was reading examine_variable in src/backend/utils/adt/selfuncs.c
It seems we already have the rte coming out of the loop which starts on
line 5181.
Here is a patch which reuses the return value from `planner_rt_fetch`.

planner_rt_fetch is not so expensive that we should contort the code
to avoid one call. So I'm not sure this is an improvement. It
doesn't seem to be more readable, and it adds assumptions on
whether appinfo is initially null or becomes so later.

regards, tom lane