From 1c5b902018bc3b045e933817603ccdb7c0205c48 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 23 May 2000 16:56:37 +0000 Subject: [PATCH] Fix problem in which sloppily-coded test in ExecInitIndexScan would 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 | 63 +++++++------------------ src/backend/optimizer/plan/createplan.c | 33 +++++++------ 2 files changed, 36 insertions(+), 60 deletions(-) diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c index d6fdce309d..472378de44 100644 --- a/src/backend/executor/nodeIndexscan.c +++ b/src/backend/executor/nodeIndexscan.c @@ -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 */ } /* ---------------- diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 9cd8a11159..4109ab2536 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -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); } /* -- 2.40.0