Re: Increase value of OUTER_VAR - Mailing list pgsql-hackers

Here's a more fleshed-out version of this patch.  I ran around and
fixed all the places where INNER_VAR etc. were being assigned directly to
a variable or parameter of type Index, and also grepped for 'Index.*varno'
to find suspicious declarations.  (I didn't change every last instance
of the latter though; just places that could possibly be looking at
post-setrefs.c Vars.)

I concluded that we don't really need to change the type of
CurrentOfExpr.cvarno, because that's never set to a special value.

The main thing I remain concerned about is whether there are more
places like set_pathtarget_cost_width(), where we could be making
an inequality comparison on "varno" that would now be wrong.
I tried to catch this by enabling -Wsign-compare and -Wsign-conversion,
but that produced so many thousands of uninteresting warnings that
I soon gave up.  I'm not sure there's any good way to catch remaining
places like that except to commit the patch and wait for trouble
reports.

So I'm inclined to propose pushing this and seeing what happens.

            regards, tom lane

diff --git a/src/backend/executor/execScan.c b/src/backend/executor/execScan.c
index 69ab34573e..9f1d8b6d1e 100644
--- a/src/backend/executor/execScan.c
+++ b/src/backend/executor/execScan.c
@@ -282,7 +282,7 @@ ExecAssignScanProjectionInfo(ScanState *node)
  *        As above, but caller can specify varno expected in Vars in the tlist.
  */
 void
-ExecAssignScanProjectionInfoWithVarno(ScanState *node, Index varno)
+ExecAssignScanProjectionInfoWithVarno(ScanState *node, int varno)
 {
     TupleDesc    tupdesc = node->ss_ScanTupleSlot->tts_tupleDescriptor;

diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index ad11392b99..4ab1302313 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -65,7 +65,7 @@
 #include "utils/typcache.h"


-static bool tlist_matches_tupdesc(PlanState *ps, List *tlist, Index varno, TupleDesc tupdesc);
+static bool tlist_matches_tupdesc(PlanState *ps, List *tlist, int varno, TupleDesc tupdesc);
 static void ShutdownExprContext(ExprContext *econtext, bool isCommit);


@@ -553,7 +553,7 @@ ExecAssignProjectionInfo(PlanState *planstate,
  */
 void
 ExecConditionalAssignProjectionInfo(PlanState *planstate, TupleDesc inputDesc,
-                                    Index varno)
+                                    int varno)
 {
     if (tlist_matches_tupdesc(planstate,
                               planstate->plan->targetlist,
@@ -579,7 +579,7 @@ ExecConditionalAssignProjectionInfo(PlanState *planstate, TupleDesc inputDesc,
 }

 static bool
-tlist_matches_tupdesc(PlanState *ps, List *tlist, Index varno, TupleDesc tupdesc)
+tlist_matches_tupdesc(PlanState *ps, List *tlist, int varno, TupleDesc tupdesc)
 {
     int            numattrs = tupdesc->natts;
     int            attrno;
diff --git a/src/backend/executor/nodeCustom.c b/src/backend/executor/nodeCustom.c
index c82060e6d1..1dfa53c381 100644
--- a/src/backend/executor/nodeCustom.c
+++ b/src/backend/executor/nodeCustom.c
@@ -31,7 +31,7 @@ ExecInitCustomScan(CustomScan *cscan, EState *estate, int eflags)
     CustomScanState *css;
     Relation    scan_rel = NULL;
     Index        scanrelid = cscan->scan.scanrelid;
-    Index        tlistvarno;
+    int            tlistvarno;

     /*
      * Allocate the CustomScanState object.  We let the custom scan provider
diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c
index 9dc38d47ea..cd93d1d768 100644
--- a/src/backend/executor/nodeForeignscan.c
+++ b/src/backend/executor/nodeForeignscan.c
@@ -128,7 +128,7 @@ ExecInitForeignScan(ForeignScan *node, EState *estate, int eflags)
     ForeignScanState *scanstate;
     Relation    currentRelation = NULL;
     Index        scanrelid = node->scan.scanrelid;
-    Index        tlistvarno;
+    int            tlistvarno;
     FdwRoutine *fdwroutine;

     /* check for unsupported flags */
diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index 01c110cd2f..7d1a01d1ed 100644
--- a/src/backend/nodes/makefuncs.c
+++ b/src/backend/nodes/makefuncs.c
@@ -63,7 +63,7 @@ makeSimpleA_Expr(A_Expr_Kind kind, char *name,
  *      creates a Var node
  */
 Var *
-makeVar(Index varno,
+makeVar(int varno,
         AttrNumber varattno,
         Oid vartype,
         int32 vartypmod,
@@ -85,7 +85,7 @@ makeVar(Index varno,
      * them, but just initialize them to the given varno/varattno.  This
      * reduces code clutter and chance of error for most callers.
      */
-    var->varnosyn = varno;
+    var->varnosyn = (Index) varno;
     var->varattnosyn = varattno;

     /* Likewise, we just set location to "unknown" here */
@@ -100,7 +100,7 @@ makeVar(Index varno,
  *        TargetEntry
  */
 Var *
-makeVarFromTargetEntry(Index varno,
+makeVarFromTargetEntry(int varno,
                        TargetEntry *tle)
 {
     return makeVar(varno,
@@ -131,7 +131,7 @@ makeVarFromTargetEntry(Index varno,
  */
 Var *
 makeWholeRowVar(RangeTblEntry *rte,
-                Index varno,
+                int varno,
                 Index varlevelsup,
                 bool allowScalar)
 {
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index e32b92e299..bcb116ae8d 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -1116,7 +1116,7 @@ _outVar(StringInfo str, const Var *node)
 {
     WRITE_NODE_TYPE("VAR");

-    WRITE_UINT_FIELD(varno);
+    WRITE_INT_FIELD(varno);
     WRITE_INT_FIELD(varattno);
     WRITE_OID_FIELD(vartype);
     WRITE_INT_FIELD(vartypmod);
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index f0b34ecfac..4c3ffa482c 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -577,7 +577,7 @@ _readVar(void)
 {
     READ_LOCALS(Var);

-    READ_UINT_FIELD(varno);
+    READ_INT_FIELD(varno);
     READ_INT_FIELD(varattno);
     READ_OID_FIELD(vartype);
     READ_INT_FIELD(vartypmod);
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 8577c7b138..fd9566bcc7 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -5963,7 +5963,8 @@ set_pathtarget_cost_width(PlannerInfo *root, PathTarget *target)
             Assert(var->varlevelsup == 0);

             /* Try to get data from RelOptInfo cache */
-            if (var->varno < root->simple_rel_array_size)
+            if (!IS_SPECIAL_VARNO(var->varno) &&
+                var->varno < root->simple_rel_array_size)
             {
                 RelOptInfo *rel = root->simple_rel_array[var->varno];

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 439e6b6426..1244723ffe 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -4808,7 +4808,8 @@ replace_nestloop_params_mutator(Node *node, PlannerInfo *root)
         /* Upper-level Vars should be long gone at this point */
         Assert(var->varlevelsup == 0);
         /* If not to be replaced, we can just return the Var unmodified */
-        if (!bms_is_member(var->varno, root->curOuterRels))
+        if (IS_SPECIAL_VARNO(var->varno) ||
+            !bms_is_member(var->varno, root->curOuterRels))
             return node;
         /* Replace the Var with a nestloop Param */
         return (Node *) replace_nestloop_param_var(root, var);
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 61ccfd300b..5e4a1a856b 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -31,7 +31,7 @@

 typedef struct
 {
-    Index        varno;            /* RT index of Var */
+    int            varno;            /* RT index of Var */
     AttrNumber    varattno;        /* attr number of Var */
     AttrNumber    resno;            /* TLE position of Var */
 } tlist_vinfo;
@@ -66,7 +66,7 @@ typedef struct
 {
     PlannerInfo *root;
     indexed_tlist *subplan_itlist;
-    Index        newvarno;
+    int            newvarno;
     int            rtoffset;
     double        num_exec;
 } fix_upper_expr_context;
@@ -143,15 +143,15 @@ static void set_dummy_tlist_references(Plan *plan, int rtoffset);
 static indexed_tlist *build_tlist_index(List *tlist);
 static Var *search_indexed_tlist_for_var(Var *var,
                                          indexed_tlist *itlist,
-                                         Index newvarno,
+                                         int newvarno,
                                          int rtoffset);
 static Var *search_indexed_tlist_for_non_var(Expr *node,
                                              indexed_tlist *itlist,
-                                             Index newvarno);
+                                             int newvarno);
 static Var *search_indexed_tlist_for_sortgroupref(Expr *node,
                                                   Index sortgroupref,
                                                   indexed_tlist *itlist,
-                                                  Index newvarno);
+                                                  int newvarno);
 static List *fix_join_expr(PlannerInfo *root,
                            List *clauses,
                            indexed_tlist *outer_itlist,
@@ -163,7 +163,7 @@ static Node *fix_join_expr_mutator(Node *node,
 static Node *fix_upper_expr(PlannerInfo *root,
                             Node *node,
                             indexed_tlist *subplan_itlist,
-                            Index newvarno,
+                            int newvarno,
                             int rtoffset, double num_exec);
 static Node *fix_upper_expr_mutator(Node *node,
                                     fix_upper_expr_context *context);
@@ -468,16 +468,6 @@ add_rte_to_flat_rtable(PlannerGlobal *glob, RangeTblEntry *rte)

     glob->finalrtable = lappend(glob->finalrtable, newrte);

-    /*
-     * Check for RT index overflow; it's very unlikely, but if it did happen,
-     * the executor would get confused by varnos that match the special varno
-     * values.
-     */
-    if (IS_SPECIAL_VARNO(list_length(glob->finalrtable)))
-        ereport(ERROR,
-                (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
-                 errmsg("too many range table entries")));
-
     /*
      * If it's a plain relation RTE, add the table to relationOids.
      *
@@ -1921,10 +1911,8 @@ fix_scan_expr_mutator(Node *node, fix_scan_expr_context *context)
     {
         CurrentOfExpr *cexpr = (CurrentOfExpr *) copyObject(node);

-        Assert(cexpr->cvarno != INNER_VAR);
-        Assert(cexpr->cvarno != OUTER_VAR);
-        if (!IS_SPECIAL_VARNO(cexpr->cvarno))
-            cexpr->cvarno += context->rtoffset;
+        Assert(!IS_SPECIAL_VARNO((int) cexpr->cvarno));
+        cexpr->cvarno += context->rtoffset;
         return (Node *) cexpr;
     }
     if (IsA(node, PlaceHolderVar))
@@ -2421,7 +2409,7 @@ build_tlist_index(List *tlist)
  * (so nothing other than Vars and PlaceHolderVars can be matched).
  */
 static indexed_tlist *
-build_tlist_index_other_vars(List *tlist, Index ignore_rel)
+build_tlist_index_other_vars(List *tlist, int ignore_rel)
 {
     indexed_tlist *itlist;
     tlist_vinfo *vinfo;
@@ -2473,9 +2461,9 @@ build_tlist_index_other_vars(List *tlist, Index ignore_rel)
  */
 static Var *
 search_indexed_tlist_for_var(Var *var, indexed_tlist *itlist,
-                             Index newvarno, int rtoffset)
+                             int newvarno, int rtoffset)
 {
-    Index        varno = var->varno;
+    int            varno = var->varno;
     AttrNumber    varattno = var->varattno;
     tlist_vinfo *vinfo;
     int            i;
@@ -2513,7 +2501,7 @@ search_indexed_tlist_for_var(Var *var, indexed_tlist *itlist,
  */
 static Var *
 search_indexed_tlist_for_non_var(Expr *node,
-                                 indexed_tlist *itlist, Index newvarno)
+                                 indexed_tlist *itlist, int newvarno)
 {
     TargetEntry *tle;

@@ -2555,7 +2543,7 @@ static Var *
 search_indexed_tlist_for_sortgroupref(Expr *node,
                                       Index sortgroupref,
                                       indexed_tlist *itlist,
-                                      Index newvarno)
+                                      int newvarno)
 {
     ListCell   *lc;

@@ -2773,7 +2761,7 @@ static Node *
 fix_upper_expr(PlannerInfo *root,
                Node *node,
                indexed_tlist *subplan_itlist,
-               Index newvarno,
+               int newvarno,
                int rtoffset,
                double num_exec)
 {
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index 62a1668796..2bf2459395 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -80,7 +80,7 @@ static Node *pull_up_simple_union_all(PlannerInfo *root, Node *jtnode,
 static void pull_up_union_leaf_queries(Node *setOp, PlannerInfo *root,
                                        int parentRTindex, Query *setOpQuery,
                                        int childRToffset);
-static void make_setop_translation_list(Query *query, Index newvarno,
+static void make_setop_translation_list(Query *query, int newvarno,
                                         AppendRelInfo *appinfo);
 static bool is_simple_subquery(PlannerInfo *root, Query *subquery,
                                RangeTblEntry *rte,
@@ -1365,7 +1365,7 @@ pull_up_union_leaf_queries(Node *setOp, PlannerInfo *root, int parentRTindex,
  *      Also create the rather trivial reverse-translation array.
  */
 static void
-make_setop_translation_list(Query *query, Index newvarno,
+make_setop_translation_list(Query *query, int newvarno,
                             AppendRelInfo *appinfo)
 {
     List       *vars = NIL;
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 3719755a0d..12d92fd7a0 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -6941,7 +6941,7 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
     AttrNumber    attnum;
     int            netlevelsup;
     deparse_namespace *dpns;
-    Index        varno;
+    int            varno;
     AttrNumber    varattno;
     deparse_columns *colinfo;
     char       *refname;
@@ -6987,7 +6987,7 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
          */
         if (context->appendparents && dpns->appendrels)
         {
-            Index        pvarno = varno;
+            int            pvarno = varno;
             AttrNumber    pvarattno = varattno;
             AppendRelInfo *appinfo = dpns->appendrels[pvarno];
             bool        found = false;
@@ -7297,7 +7297,7 @@ get_name_for_var_field(Var *var, int fieldno,
     AttrNumber    attnum;
     int            netlevelsup;
     deparse_namespace *dpns;
-    Index        varno;
+    int            varno;
     AttrNumber    varattno;
     TupleDesc    tupleDesc;
     Node       *expr;
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 3dc03c913e..cd57a704ad 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -459,7 +459,7 @@ typedef bool (*ExecScanRecheckMtd) (ScanState *node, TupleTableSlot *slot);
 extern TupleTableSlot *ExecScan(ScanState *node, ExecScanAccessMtd accessMtd,
                                 ExecScanRecheckMtd recheckMtd);
 extern void ExecAssignScanProjectionInfo(ScanState *node);
-extern void ExecAssignScanProjectionInfoWithVarno(ScanState *node, Index varno);
+extern void ExecAssignScanProjectionInfoWithVarno(ScanState *node, int varno);
 extern void ExecScanReScan(ScanState *node);

 /*
@@ -552,7 +552,7 @@ extern const TupleTableSlotOps *ExecGetResultSlotOps(PlanState *planstate,
 extern void ExecAssignProjectionInfo(PlanState *planstate,
                                      TupleDesc inputDesc);
 extern void ExecConditionalAssignProjectionInfo(PlanState *planstate,
-                                                TupleDesc inputDesc, Index varno);
+                                                TupleDesc inputDesc, int varno);
 extern void ExecFreeExprContext(PlanState *planstate);
 extern void ExecAssignScanType(ScanState *scanstate, TupleDesc tupDesc);
 extern void ExecCreateScanSlotFromOuterPlan(EState *estate,
diff --git a/src/include/nodes/makefuncs.h b/src/include/nodes/makefuncs.h
index 48a7ebfe45..eea87f847d 100644
--- a/src/include/nodes/makefuncs.h
+++ b/src/include/nodes/makefuncs.h
@@ -24,18 +24,18 @@ extern A_Expr *makeA_Expr(A_Expr_Kind kind, List *name,
 extern A_Expr *makeSimpleA_Expr(A_Expr_Kind kind, char *name,
                                 Node *lexpr, Node *rexpr, int location);

-extern Var *makeVar(Index varno,
+extern Var *makeVar(int varno,
                     AttrNumber varattno,
                     Oid vartype,
                     int32 vartypmod,
                     Oid varcollid,
                     Index varlevelsup);

-extern Var *makeVarFromTargetEntry(Index varno,
+extern Var *makeVarFromTargetEntry(int varno,
                                    TargetEntry *tle);

 extern Var *makeWholeRowVar(RangeTblEntry *rte,
-                            Index varno,
+                            int varno,
                             Index varlevelsup,
                             bool allowScalar);

diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 9ae851d847..3a3d9e7669 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -172,12 +172,12 @@ typedef struct Expr
  * in the planner and doesn't correspond to any simple relation column may
  * have varnosyn = varattnosyn = 0.
  */
-#define    INNER_VAR        65000    /* reference to inner subplan */
-#define    OUTER_VAR        65001    /* reference to outer subplan */
-#define    INDEX_VAR        65002    /* reference to index column */
-#define    ROWID_VAR        65003    /* row identity column during planning */
+#define    INNER_VAR        (-1)    /* reference to inner subplan */
+#define    OUTER_VAR        (-2)    /* reference to outer subplan */
+#define    INDEX_VAR        (-3)    /* reference to index column */
+#define    ROWID_VAR        (-4)    /* row identity column during planning */

-#define IS_SPECIAL_VARNO(varno)        ((varno) >= INNER_VAR)
+#define IS_SPECIAL_VARNO(varno)        ((varno) < 0)

 /* Symbols for the indexes of the special RTE entries in rules */
 #define    PRS2_OLD_VARNO            1
@@ -186,8 +186,8 @@ typedef struct Expr
 typedef struct Var
 {
     Expr        xpr;
-    Index        varno;            /* index of this var's relation in the range
-                                 * table, or INNER_VAR/OUTER_VAR/INDEX_VAR */
+    int            varno;            /* index of this var's relation in the range
+                                 * table, or INNER_VAR/OUTER_VAR/etc */
     AttrNumber    varattno;        /* attribute number of this var, or zero for
                                  * all attrs ("whole-row Var") */
     Oid            vartype;        /* pg_type OID for the type of this var */
@@ -1341,6 +1341,7 @@ typedef struct SetToDefault
  * of the target relation being constrained; this aids placing the expression
  * correctly during planning.  We can assume however that its "levelsup" is
  * always zero, due to the syntactic constraints on where it can appear.
+ * Also, cvarno will always be a true RT index, never INNER_VAR etc.
  *
  * The referenced cursor can be represented either as a hardwired string
  * or as a reference to a run-time parameter of type REFCURSOR.  The latter