small makeVar refactoring
Started by Peter Eisentrautover 15 years ago2 messages
While hacking around, I noticed that a lot of makeVar() calls could be
refactored into some convenience functions, to save some redundancy and
so that the unusual call patterns stand out better. Useful?
Attachments:
makevar-refactor.patchtext/x-patch; charset=UTF-8; name=makevar-refactor.patchDownload
Index: src/backend/commands/tablecmds.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.341
diff -u -3 -p -r1.341 tablecmds.c
--- src/backend/commands/tablecmds.c 18 Aug 2010 18:35:19 -0000 1.341
+++ src/backend/commands/tablecmds.c 21 Aug 2010 21:09:30 -0000
@@ -6077,9 +6077,7 @@ ATPrepAlterColumnType(List **wqueue,
}
else
{
- transform = (Node *) makeVar(1, attnum,
- attTup->atttypid, attTup->atttypmod,
- 0);
+ transform = (Node *) makeVarFromAttribute(1, attnum, attTup);
}
transform = coerce_to_target_type(pstate,
Index: src/backend/nodes/makefuncs.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/nodes/makefuncs.c,v
retrieving revision 1.66
diff -u -3 -p -r1.66 makefuncs.c
--- src/backend/nodes/makefuncs.c 2 Jan 2010 16:57:46 -0000 1.66
+++ src/backend/nodes/makefuncs.c 21 Aug 2010 21:09:30 -0000
@@ -17,6 +17,7 @@
#include "catalog/pg_type.h"
#include "nodes/makefuncs.h"
+#include "nodes/nodeFuncs.h"
#include "utils/lsyscache.h"
@@ -91,6 +92,39 @@ makeVar(Index varno,
}
/*
+ * makeVarFromTargetEntry -
+ * convenience function to create a same-level Var node from a
+ * TargetEntry
+ */
+Var *
+makeVarFromTargetEntry(Index varno,
+ TargetEntry *tle)
+{
+ return makeVar(varno,
+ tle->resno,
+ exprType((Node *) tle->expr),
+ exprTypmod((Node *) tle->expr),
+ 0);
+}
+
+/*
+ * makeVarFromAttribute -
+ * convenience function to create a same-level Var node from a
+ * pg_attribute tuple structure
+ */
+Var *
+makeVarFromAttribute(Index varno,
+ AttrNumber varattno,
+ Form_pg_attribute att)
+{
+ return makeVar(varno,
+ varattno,
+ att->atttypid,
+ att->atttypmod,
+ 0);
+}
+
+/*
* makeTargetEntry -
* creates a TargetEntry node
*/
Index: src/backend/optimizer/path/pathkeys.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/optimizer/path/pathkeys.c,v
retrieving revision 1.101
diff -u -3 -p -r1.101 pathkeys.c
--- src/backend/optimizer/path/pathkeys.c 26 Feb 2010 02:00:45 -0000 1.101
+++ src/backend/optimizer/path/pathkeys.c 21 Aug 2010 21:09:30 -0000
@@ -629,12 +629,7 @@ convert_subquery_pathkeys(PlannerInfo *r
Assert(list_length(sub_eclass->ec_members) == 1);
sub_member = (EquivalenceMember *) linitial(sub_eclass->ec_members);
- outer_expr = (Expr *)
- makeVar(rel->relid,
- tle->resno,
- exprType((Node *) tle->expr),
- exprTypmod((Node *) tle->expr),
- 0);
+ outer_expr = (Expr *) makeVarFromTargetEntry(rel->relid, tle);
/*
* Note: it might look funny to be setting sortref = 0 for a
@@ -712,12 +707,7 @@ convert_subquery_pathkeys(PlannerInfo *r
if (equal(tle->expr, sub_expr))
{
/* Exact match */
- outer_expr = (Expr *)
- makeVar(rel->relid,
- tle->resno,
- exprType((Node *) tle->expr),
- exprTypmod((Node *) tle->expr),
- 0);
+ outer_expr = (Expr *) makeVarFromTargetEntry(rel->relid, tle);
}
else
{
@@ -730,12 +720,7 @@ convert_subquery_pathkeys(PlannerInfo *r
if (equal(tle_stripped, sub_stripped))
{
/* Match after discarding RelabelType */
- outer_expr = (Expr *)
- makeVar(rel->relid,
- tle->resno,
- exprType((Node *) tle->expr),
- exprTypmod((Node *) tle->expr),
- 0);
+ outer_expr = (Expr *) makeVarFromTargetEntry(rel->relid, tle);
if (exprType((Node *) outer_expr) !=
exprType((Node *) sub_expr))
outer_expr = (Expr *)
Index: src/backend/optimizer/plan/setrefs.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/optimizer/plan/setrefs.c,v
retrieving revision 1.161
diff -u -3 -p -r1.161 setrefs.c
--- src/backend/optimizer/plan/setrefs.c 12 Jul 2010 17:01:06 -0000 1.161
+++ src/backend/optimizer/plan/setrefs.c 21 Aug 2010 21:09:30 -0000
@@ -1283,11 +1283,7 @@ search_indexed_tlist_for_non_var(Node *n
/* Found a matching subplan output expression */
Var *newvar;
- newvar = makeVar(newvarno,
- tle->resno,
- exprType((Node *) tle->expr),
- exprTypmod((Node *) tle->expr),
- 0);
+ newvar = makeVarFromTargetEntry(newvarno, tle);
newvar->varnoold = 0; /* wasn't ever a plain Var */
newvar->varoattno = 0;
return newvar;
@@ -1325,11 +1321,7 @@ search_indexed_tlist_for_sortgroupref(No
/* Found a matching subplan output expression */
Var *newvar;
- newvar = makeVar(newvarno,
- tle->resno,
- exprType((Node *) tle->expr),
- exprTypmod((Node *) tle->expr),
- 0);
+ newvar = makeVarFromTargetEntry(newvarno, tle);
newvar->varnoold = 0; /* wasn't ever a plain Var */
newvar->varoattno = 0;
return newvar;
Index: src/backend/optimizer/plan/subselect.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/optimizer/plan/subselect.c,v
retrieving revision 1.163
diff -u -3 -p -r1.163 subselect.c
--- src/backend/optimizer/plan/subselect.c 12 Jul 2010 17:01:06 -0000 1.163
+++ src/backend/optimizer/plan/subselect.c 21 Aug 2010 21:09:30 -0000
@@ -737,11 +737,7 @@ generate_subquery_vars(PlannerInfo *root
if (tent->resjunk)
continue;
- var = makeVar(varno,
- tent->resno,
- exprType((Node *) tent->expr),
- exprTypmod((Node *) tent->expr),
- 0);
+ var = makeVarFromTargetEntry(varno, tent);
result = lappend(result, var);
}
Index: src/backend/optimizer/prep/prepjointree.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/optimizer/prep/prepjointree.c,v
retrieving revision 1.73
diff -u -3 -p -r1.73 prepjointree.c
--- src/backend/optimizer/prep/prepjointree.c 6 Jul 2010 19:18:56 -0000 1.73
+++ src/backend/optimizer/prep/prepjointree.c 21 Aug 2010 21:09:30 -0000
@@ -991,11 +991,7 @@ make_setop_translation_list(Query *query
if (tle->resjunk)
continue;
- vars = lappend(vars, makeVar(newvarno,
- tle->resno,
- exprType((Node *) tle->expr),
- exprTypmod((Node *) tle->expr),
- 0));
+ vars = lappend(vars, makeVarFromTargetEntry(newvarno, tle));
}
*translated_vars = vars;
Index: src/backend/optimizer/prep/preptlist.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/optimizer/prep/preptlist.c,v
retrieving revision 1.100
diff -u -3 -p -r1.100 preptlist.c
--- src/backend/optimizer/prep/preptlist.c 26 Feb 2010 02:00:46 -0000 1.100
+++ src/backend/optimizer/prep/preptlist.c 21 Aug 2010 21:09:30 -0000
@@ -304,8 +304,6 @@ expand_targetlist(List *tlist, int comma
* confuse code comparing the finished plan to the target
* relation, however.
*/
- Oid atttype = att_tup->atttypid;
- int32 atttypmod = att_tup->atttypmod;
Node *new_expr;
switch (command_type)
@@ -313,7 +311,7 @@ expand_targetlist(List *tlist, int comma
case CMD_INSERT:
if (!att_tup->attisdropped)
{
- new_expr = (Node *) makeConst(atttype,
+ new_expr = (Node *) makeConst(att_tup->atttypid,
-1,
att_tup->attlen,
(Datum) 0,
@@ -321,7 +319,7 @@ expand_targetlist(List *tlist, int comma
att_tup->attbyval);
new_expr = coerce_to_domain(new_expr,
InvalidOid, -1,
- atttype,
+ att_tup->atttypid,
COERCE_IMPLICIT_CAST,
-1,
false,
@@ -341,11 +339,9 @@ expand_targetlist(List *tlist, int comma
case CMD_UPDATE:
if (!att_tup->attisdropped)
{
- new_expr = (Node *) makeVar(result_relation,
- attrno,
- atttype,
- atttypmod,
- 0);
+ new_expr = (Node *) makeVarFromAttribute(result_relation,
+ attrno,
+ att_tup);
}
else
{
Index: src/backend/optimizer/util/plancat.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/optimizer/util/plancat.c,v
retrieving revision 1.163
diff -u -3 -p -r1.163 plancat.c
--- src/backend/optimizer/util/plancat.c 30 Mar 2010 21:58:10 -0000 1.163
+++ src/backend/optimizer/util/plancat.c 21 Aug 2010 21:09:30 -0000
@@ -538,11 +538,7 @@ get_relation_constraints(PlannerInfo *ro
{
NullTest *ntest = makeNode(NullTest);
- ntest->arg = (Expr *) makeVar(varno,
- i,
- att->atttypid,
- att->atttypmod,
- 0);
+ ntest->arg = (Expr *) makeVarFromAttribute(varno, i, att);
ntest->nulltesttype = IS_NOT_NULL;
ntest->argisrow = type_is_rowtype(att->atttypid);
result = lappend(result, ntest);
@@ -701,11 +697,7 @@ build_physical_tlist(PlannerInfo *root,
break;
}
- var = makeVar(varno,
- attrno,
- att_tup->atttypid,
- att_tup->atttypmod,
- 0);
+ var = makeVarFromAttribute(varno, attrno, att_tup);
tlist = lappend(tlist,
makeTargetEntry((Expr *) var,
@@ -727,11 +719,7 @@ build_physical_tlist(PlannerInfo *root,
* A resjunk column of the subquery can be reflected as
* resjunk in the physical tlist; we need not punt.
*/
- var = makeVar(varno,
- tle->resno,
- exprType((Node *) tle->expr),
- exprTypmod((Node *) tle->expr),
- 0);
+ var = makeVarFromTargetEntry(varno, tle);
tlist = lappend(tlist,
makeTargetEntry((Expr *) var,
Index: src/backend/parser/analyze.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/analyze.c,v
retrieving revision 1.402
diff -u -3 -p -r1.402 analyze.c
--- src/backend/parser/analyze.c 26 Feb 2010 02:00:49 -0000 1.402
+++ src/backend/parser/analyze.c 21 Aug 2010 21:09:30 -0000
@@ -484,11 +484,7 @@ transformInsertStmt(ParseState *pstate,
expr = tle->expr;
else
{
- Var *var = makeVar(rtr->rtindex,
- tle->resno,
- exprType((Node *) tle->expr),
- exprTypmod((Node *) tle->expr),
- 0);
+ Var *var = makeVarFromTargetEntry(rtr->rtindex, tle);
var->location = exprLocation((Node *) tle->expr);
expr = (Expr *) var;
Index: src/include/nodes/makefuncs.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/nodes/makefuncs.h,v
retrieving revision 1.70
diff -u -3 -p -r1.70 makefuncs.h
--- src/include/nodes/makefuncs.h 2 Jan 2010 16:58:04 -0000 1.70
+++ src/include/nodes/makefuncs.h 21 Aug 2010 21:09:31 -0000
@@ -14,6 +14,7 @@
#ifndef MAKEFUNC_H
#define MAKEFUNC_H
+#include "catalog/pg_attribute.h"
#include "nodes/parsenodes.h"
@@ -29,6 +30,13 @@ extern Var *makeVar(Index varno,
int32 vartypmod,
Index varlevelsup);
+extern Var *makeVarFromTargetEntry(Index varno,
+ TargetEntry *tle);
+
+extern Var *makeVarFromAttribute(Index varno,
+ AttrNumber varattno,
+ Form_pg_attribute att);
+
extern TargetEntry *makeTargetEntry(Expr *expr,
AttrNumber resno,
char *resname,
Re: small makeVar refactoring
Peter Eisentraut <peter_e@gmx.net> writes:
While hacking around, I noticed that a lot of makeVar() calls could be
refactored into some convenience functions, to save some redundancy and
so that the unusual call patterns stand out better. Useful?
I'm not real thrilled with importing catalog/pg_attribute.h into
makefuncs.h; that seems like a lot of namespace pollution for not much
return. So -1 to makeVarFromAttribute. The other is okay although I'm
not convinced it's useful enough to bother with.
regards, tom lane