OSDN Git Service

Fix problem in which sloppily-coded test in ExecInitIndexScan would
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 23 May 2000 16:56:37 +0000 (16:56 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 23 May 2000 16:56:37 +0000 (16:56 +0000)
think that both sides of indexqual look like index keys.  An example is
create table inside (f1 float8 primary key);
create table outside (g1 float8, g2 float8);
select * from inside,outside where f1 = atan2(g1+1, g2);
ERROR:  ExecInitIndexScan: both left and right ops are rel-vars
(note that failure is potentially platform-dependent).  Solution is a
cleanup I had had in mind to make anyway: functional index keys should
be represented as Var nodes in the fixed indexqual, just like regular
index keys.

src/backend/executor/nodeIndexscan.c
src/backend/optimizer/plan/createplan.c

index d6fdce3..472378d 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/executor/nodeIndexscan.c,v 1.49 2000/04/12 17:15:09 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/executor/nodeIndexscan.c,v 1.50 2000/05/23 16:56:37 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -734,8 +734,8 @@ ExecInitIndexScan(IndexScan *node, EState *estate, Plan *parent)
                 */
                for (j = 0; j < n_keys; j++)
                {
-                       Expr       *clause; /* one part of index qual */
-                       Oper       *op;         /* operator used in scan.. */
+                       Expr       *clause; /* one clause of index qual */
+                       Oper       *op;         /* operator used in clause */
                        Node       *leftop; /* expr on lhs of operator */
                        Node       *rightop;/* expr on rhs ... */
                        bits16          flags = 0;
@@ -794,6 +794,7 @@ ExecInitIndexScan(IndexScan *node, EState *estate, Plan *parent)
                         */
 
                        scanvar = NO_OP;
+                       run_keys[j] = NO_OP;
 
                        /* ----------------
                         *      determine information in leftop
@@ -803,7 +804,7 @@ ExecInitIndexScan(IndexScan *node, EState *estate, Plan *parent)
 
                        Assert(leftop != NULL);
 
-                       if (IsA(leftop, Var) &&var_is_rel((Var *) leftop))
+                       if (IsA(leftop, Var) && var_is_rel((Var *) leftop))
                        {
                                /* ----------------
                                 *      if the leftop is a "rel-var", then it means
@@ -814,19 +815,6 @@ ExecInitIndexScan(IndexScan *node, EState *estate, Plan *parent)
                                varattno = ((Var *) leftop)->varattno;
                                scanvar = LEFT_OP;
                        }
-                       else if (is_funcclause(leftop) &&
-                                        var_is_rel(lfirst(((Expr *) leftop)->args)))
-                       {
-                               /* ----------------
-                                *      if the leftop is a func node then it means
-                                *      it identifies the value to place in our scan key.
-                                *      Since functional indices have only one attribute
-                                *      the attno must always be set to 1.
-                                * ----------------
-                                */
-                               varattno = 1;
-                               scanvar = LEFT_OP;
-                       }
                        else if (IsA(leftop, Const))
                        {
                                /* ----------------
@@ -834,8 +822,9 @@ ExecInitIndexScan(IndexScan *node, EState *estate, Plan *parent)
                                 *      it identifies the value to place in our scan key.
                                 * ----------------
                                 */
-                               run_keys[j] = NO_OP;
                                scanvalue = ((Const *) leftop)->constvalue;
+                               if (((Const *) leftop)->constisnull)
+                                       flags |= SK_ISNULL;
                        }
                        else if (IsA(leftop, Param))
                        {
@@ -850,32 +839,31 @@ ExecInitIndexScan(IndexScan *node, EState *estate, Plan *parent)
                                /* Life was so easy before ... subselects */
                                if (((Param *) leftop)->paramkind == PARAM_EXEC)
                                {
+                                       /* treat Param as runtime key */
                                        have_runtime_keys = true;
                                        run_keys[j] = LEFT_OP;
                                        execParam = lappendi(execParam, ((Param *) leftop)->paramid);
                                }
                                else
                                {
+                                       /* treat Param like a constant */
                                        scanvalue = ExecEvalParam((Param *) leftop,
                                                                                scanstate->cstate.cs_ExprContext,
                                                                                          &isnull);
                                        if (isnull)
                                                flags |= SK_ISNULL;
-
-                                       run_keys[j] = NO_OP;
                                }
                        }
                        else
                        {
                                /* ----------------
-                                *      otherwise, the leftop contains information usable
+                                *      otherwise, the leftop contains an expression evaluable
                                 *      at runtime to figure out the value to place in our
                                 *      scan key.
                                 * ----------------
                                 */
                                have_runtime_keys = true;
                                run_keys[j] = LEFT_OP;
-                               scanvalue = Int32GetDatum((int32) true);
                        }
 
                        /* ----------------
@@ -886,7 +874,7 @@ ExecInitIndexScan(IndexScan *node, EState *estate, Plan *parent)
 
                        Assert(rightop != NULL);
 
-                       if (IsA(rightop, Var) &&var_is_rel((Var *) rightop))
+                       if (IsA(rightop, Var) && var_is_rel((Var *) rightop))
                        {
                                /* ----------------
                                 *      here we make sure only one op identifies the
@@ -906,23 +894,6 @@ ExecInitIndexScan(IndexScan *node, EState *estate, Plan *parent)
                                varattno = ((Var *) rightop)->varattno;
                                scanvar = RIGHT_OP;
                        }
-                       else if (is_funcclause(rightop) &&
-                                        var_is_rel(lfirst(((Expr *) rightop)->args)))
-                       {
-                               /* ----------------
-                                *      if the rightop is a func node then it means
-                                *      it identifies the value to place in our scan key.
-                                *      Since functional indices have only one attribute
-                                *      the attno must always be set to 1.
-                                * ----------------
-                                */
-                               if (scanvar == LEFT_OP)
-                                       elog(ERROR, "ExecInitIndexScan: %s",
-                                                "both left and right ops are rel-vars");
-
-                               varattno = 1;
-                               scanvar = RIGHT_OP;
-                       }
                        else if (IsA(rightop, Const))
                        {
                                /* ----------------
@@ -930,8 +901,9 @@ ExecInitIndexScan(IndexScan *node, EState *estate, Plan *parent)
                                 *      it identifies the value to place in our scan key.
                                 * ----------------
                                 */
-                               run_keys[j] = NO_OP;
                                scanvalue = ((Const *) rightop)->constvalue;
+                               if (((Const *) rightop)->constisnull)
+                                       flags |= SK_ISNULL;
                        }
                        else if (IsA(rightop, Param))
                        {
@@ -946,32 +918,31 @@ ExecInitIndexScan(IndexScan *node, EState *estate, Plan *parent)
                                /* Life was so easy before ... subselects */
                                if (((Param *) rightop)->paramkind == PARAM_EXEC)
                                {
+                                       /* treat Param as runtime key */
                                        have_runtime_keys = true;
                                        run_keys[j] = RIGHT_OP;
                                        execParam = lappendi(execParam, ((Param *) rightop)->paramid);
                                }
                                else
                                {
+                                       /* treat Param like a constant */
                                        scanvalue = ExecEvalParam((Param *) rightop,
                                                                                scanstate->cstate.cs_ExprContext,
                                                                                          &isnull);
                                        if (isnull)
                                                flags |= SK_ISNULL;
-
-                                       run_keys[j] = NO_OP;
                                }
                        }
                        else
                        {
                                /* ----------------
-                                *      otherwise, the rightop contains information usable
+                                *      otherwise, the rightop contains an expression evaluable
                                 *      at runtime to figure out the value to place in our
                                 *      scan key.
                                 * ----------------
                                 */
                                have_runtime_keys = true;
                                run_keys[j] = RIGHT_OP;
-                               scanvalue = Int32GetDatum((int32) true);
                        }
 
                        /* ----------------
@@ -992,7 +963,7 @@ ExecInitIndexScan(IndexScan *node, EState *estate, Plan *parent)
                                                                   varattno,    /* attribute number to
                                                                                                 * scan */
                                                                   (RegProcedure) opid, /* reg proc to use */
-                                                                  (Datum) scanvalue);  /* constant */
+                                                                  scanvalue);  /* constant */
                }
 
                /* ----------------
index 9cd8a11..4109ab2 100644 (file)
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.89 2000/04/12 17:15:21 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.90 2000/05/23 16:56:36 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -27,6 +27,7 @@
 #include "optimizer/planmain.h"
 #include "optimizer/restrictinfo.h"
 #include "optimizer/tlist.h"
+#include "parser/parse_expr.h"
 #include "utils/lsyscache.h"
 #include "utils/syscache.h"
 
@@ -722,7 +723,7 @@ create_hashjoin_node(HashPath *best_path,
  *       machinery needs.
  *
  * We have three tasks here:
- *     * Var nodes representing index keys must have varattno equal to the
+ *     * Index keys must be represented by Var nodes with varattno set to the
  *       index's attribute number, not the attribute number in the original rel.
  *     * indxpath.c may have selected an index that is binary-compatible with
  *       the actual expression operator, but not exactly the same datatype.
@@ -789,7 +790,7 @@ fix_indxqual_references(List *indexquals, IndexPath *index_path)
  * Fix the sublist of indexquals to be used in a particular scan.
  *
  * For each qual clause, commute if needed to put the indexkey operand on the
- * left, and then change its varno.  (We do not need to change the other side
+ * left, and then fix its varattno.  (We do not need to change the other side
  * of the clause.)     Also change the operator if necessary.
  */
 static List *
@@ -863,8 +864,16 @@ static Node *
 fix_indxqual_operand(Node *node, int baserelid, Form_pg_index index,
                                         Oid *opclass)
 {
+       /*
+        * We represent index keys by Var nodes having the varno of the base
+        * table but varattno equal to the index's attribute number (index
+        * column position).  This is a bit hokey ... would be cleaner to use
+        * a special-purpose node type that could not be mistaken for a regular
+        * Var.  But it will do for now.
+        */
        if (IsA(node, Var))
        {
+               /* If it's a var, find which index key position it occupies */
                if (((Var *) node)->varno == baserelid)
                {
                        int                     varatt = ((Var *) node)->varattno;
@@ -877,6 +886,7 @@ fix_indxqual_operand(Node *node, int baserelid, Form_pg_index index,
                                        Node       *newnode = copyObject(node);
 
                                        ((Var *) newnode)->varattno = pos + 1;
+                                       /* return the correct opclass, too */
                                        *opclass = index->indclass[pos];
                                        return newnode;
                                }
@@ -890,22 +900,17 @@ fix_indxqual_operand(Node *node, int baserelid, Form_pg_index index,
        }
 
        /*
-        * Else, it must be a func expression representing a functional index.
-        *
-        * Currently, there is no need for us to do anything here for functional
-        * indexes.  If nodeIndexscan.c sees a func clause as the left or
-        * right-hand toplevel operand of an indexqual, it assumes that that
-        * is a reference to the functional index's value and makes the
-        * appropriate substitution.  (It would be cleaner to make the
-        * substitution here, I think --- suspect this issue if a join clause
-        * involving a function call misbehaves...)
+        * Else, it must be a func expression matching a functional index.
+        * Since we currently only support single-column functional indexes,
+        * the returned varattno must be 1.
         */
 
+       Assert(is_funcclause(node)); /* not a very thorough check, but easy */
+
        /* indclass[0] is the only class of a functional index */
        *opclass = index->indclass[0];
 
-       /* return the unmodified node */
-       return node;
+       return (Node *) makeVar(baserelid, 1, exprType(node), -1, 0);
 }
 
 /*