WIP: convert plpgsql to using parser hooks - Mailing list pgsql-hackers

From Tom Lane
Subject WIP: convert plpgsql to using parser hooks
Date
Msg-id 7827.1257518597@sss.pgh.pa.us
Whole thread Raw
List pgsql-hackers
The attached patch makes the basic cutover from inserting plpgsql
variables into SQL expressions by textual substitution, to handling
them via parser hooks.  Currently I only have the pre_columnref hook
written, so only the "backwards compatible" semantics are available.
However this already makes two significant changes compared to before:
1. plpgsql will no longer attempt to substitute variables where they
can't sensibly go, such as table names or column aliases.
2. variable names can't match reserved words unless quoted (notice
the one necessary change in the regression tests).

I realized though that the namespace mechanism needs further adjustment.
As-is, the scope of a variable declaration is its whole block, which
is wrong.  Consider

    declare x int;
    begin
        ...
        declare y int := x+1;
            x int := 4;

The second declaration of x will probably capture the "x" reference
in y's default, which it should not.  What I think I will do about
this is to abandon the notion of an array-per-namespace altogether,
and turn the namespace data structure into simple chains of individual
names.  Instead of linking SQL expressions to the topmost visible
namespace, they'll link to the last-added variable that they can see.
This gives per-variable granularity of visibility, which is what we
need to make this type of thing work as expected.

            regards, tom lane

Index: src/pl/plpgsql/src/gram.y
===================================================================
RCS file: /cvsroot/pgsql/src/pl/plpgsql/src/gram.y,v
retrieving revision 1.130
diff -c -r1.130 gram.y
*** src/pl/plpgsql/src/gram.y    5 Nov 2009 16:58:36 -0000    1.130
--- src/pl/plpgsql/src/gram.y    6 Nov 2009 14:31:53 -0000
***************
*** 520,526 ****

                          plpgsql_ns_setlocal(false);

!                         nsi = plpgsql_ns_lookup(name, NULL, NULL, NULL);
                          if (nsi == NULL)
                          {
                              plpgsql_error_lineno = plpgsql_scanner_lineno();
--- 520,528 ----

                          plpgsql_ns_setlocal(false);

!                         nsi = plpgsql_ns_lookup(plpgsql_ns_current(),
!                                                 name, NULL, NULL,
!                                                 NULL);
                          if (nsi == NULL)
                          {
                              plpgsql_error_lineno = plpgsql_scanner_lineno();
***************
*** 550,556 ****
                      {
                          /*
                           * Since the scanner is only searching the topmost
!                          * namestack entry, getting T_SCALAR etc can only
                           * happen if the name is already declared in this
                           * block.
                           */
--- 552,558 ----
                      {
                          /*
                           * Since the scanner is only searching the topmost
!                          * namespace entry, getting T_SCALAR etc can only
                           * happen if the name is already declared in this
                           * block.
                           */
***************
*** 1698,1710 ****
                  | K_EXCEPTION lno
                      {
                          /*
!                          * We use a mid-rule action to add these
!                          * special variables to the namespace before
!                          * parsing the WHEN clauses themselves.
                           */
                          PLpgSQL_exception_block *new = palloc(sizeof(PLpgSQL_exception_block));
                          PLpgSQL_variable *var;

                          var = plpgsql_build_variable("sqlstate", $2,
                                                       plpgsql_build_datatype(TEXTOID, -1),
                                                       true);
--- 1700,1715 ----
                  | K_EXCEPTION lno
                      {
                          /*
!                          * We use a mid-rule action to add a local namespace
!                          * containing these special variables before parsing
!                          * the WHEN clauses themselves.  The namespace ensures
!                          * the vars are only visible within the WHEN clauses.
                           */
                          PLpgSQL_exception_block *new = palloc(sizeof(PLpgSQL_exception_block));
                          PLpgSQL_variable *var;

+                         plpgsql_ns_push(NULL);
+
                          var = plpgsql_build_variable("sqlstate", $2,
                                                       plpgsql_build_datatype(TEXTOID, -1),
                                                       true);
***************
*** 1722,1728 ****
--- 1727,1735 ----
                      proc_exceptions
                      {
                          PLpgSQL_exception_block *new = $<exception_block>3;
+
                          new->exc_list = $4;
+                         plpgsql_ns_pop();

                          $$ = new;
                      }
***************
*** 1937,1944 ****
      int                    lno;
      StringInfoData        ds;
      int                    parenlevel = 0;
-     Bitmapset           *paramnos = NULL;
-     char                buf[32];
      PLpgSQL_expr        *expr;

      lno = plpgsql_scanner_lineno();
--- 1944,1949 ----
***************
*** 1986,2016 ****

          if (plpgsql_SpaceScanned)
              appendStringInfoChar(&ds, ' ');
!
!         switch (tok)
!         {
!             case T_SCALAR:
!                 snprintf(buf, sizeof(buf), " $%d ", yylval.scalar->dno + 1);
!                 appendStringInfoString(&ds, buf);
!                 paramnos = bms_add_member(paramnos, yylval.scalar->dno);
!                 break;
!
!             case T_ROW:
!                 snprintf(buf, sizeof(buf), " $%d ", yylval.row->dno + 1);
!                 appendStringInfoString(&ds, buf);
!                 paramnos = bms_add_member(paramnos, yylval.row->dno);
!                 break;
!
!             case T_RECORD:
!                 snprintf(buf, sizeof(buf), " $%d ", yylval.rec->dno + 1);
!                 appendStringInfoString(&ds, buf);
!                 paramnos = bms_add_member(paramnos, yylval.rec->dno);
!                 break;
!
!             default:
!                 appendStringInfoString(&ds, yytext);
!                 break;
!         }
      }

      if (endtoken)
--- 1991,1997 ----

          if (plpgsql_SpaceScanned)
              appendStringInfoChar(&ds, ' ');
!         appendStringInfoString(&ds, yytext);
      }

      if (endtoken)
***************
*** 2020,2026 ****
      expr->dtype            = PLPGSQL_DTYPE_EXPR;
      expr->query            = pstrdup(ds.data);
      expr->plan            = NULL;
!     expr->paramnos        = paramnos;
      pfree(ds.data);

      if (valid_sql)
--- 2001,2008 ----
      expr->dtype            = PLPGSQL_DTYPE_EXPR;
      expr->query            = pstrdup(ds.data);
      expr->plan            = NULL;
!     expr->paramnos        = NULL;
!     expr->ns            = plpgsql_ns_current();
      pfree(ds.data);

      if (valid_sql)
***************
*** 2100,2107 ****
  make_execsql_stmt(const char *sqlstart, int lineno)
  {
      StringInfoData        ds;
-     Bitmapset           *paramnos = NULL;
-     char                buf[32];
      PLpgSQL_stmt_execsql *execsql;
      PLpgSQL_expr        *expr;
      PLpgSQL_row            *row = NULL;
--- 2082,2087 ----
***************
*** 2147,2184 ****

          if (plpgsql_SpaceScanned)
              appendStringInfoChar(&ds, ' ');
!
!         switch (tok)
!         {
!             case T_SCALAR:
!                 snprintf(buf, sizeof(buf), " $%d ", yylval.scalar->dno + 1);
!                 appendStringInfoString(&ds, buf);
!                 paramnos = bms_add_member(paramnos, yylval.scalar->dno);
!                 break;
!
!             case T_ROW:
!                 snprintf(buf, sizeof(buf), " $%d ", yylval.row->dno + 1);
!                 appendStringInfoString(&ds, buf);
!                 paramnos = bms_add_member(paramnos, yylval.row->dno);
!                 break;
!
!             case T_RECORD:
!                 snprintf(buf, sizeof(buf), " $%d ", yylval.rec->dno + 1);
!                 appendStringInfoString(&ds, buf);
!                 paramnos = bms_add_member(paramnos, yylval.rec->dno);
!                 break;
!
!             default:
!                 appendStringInfoString(&ds, yytext);
!                 break;
!         }
      }

      expr = palloc0(sizeof(PLpgSQL_expr));
      expr->dtype            = PLPGSQL_DTYPE_EXPR;
      expr->query            = pstrdup(ds.data);
      expr->plan            = NULL;
!     expr->paramnos        = paramnos;
      pfree(ds.data);

      check_sql_expr(expr->query);
--- 2127,2141 ----

          if (plpgsql_SpaceScanned)
              appendStringInfoChar(&ds, ' ');
!         appendStringInfoString(&ds, yytext);
      }

      expr = palloc0(sizeof(PLpgSQL_expr));
      expr->dtype            = PLPGSQL_DTYPE_EXPR;
      expr->query            = pstrdup(ds.data);
      expr->plan            = NULL;
!     expr->paramnos        = NULL;
!     expr->ns            = plpgsql_ns_current();
      pfree(ds.data);

      check_sql_expr(expr->query);
***************
*** 2804,2810 ****
      char       *label_name;

      plpgsql_convert_ident(yytxt, &label_name, 1);
!     if (plpgsql_ns_lookup_label(label_name) == NULL)
          yyerror("label does not exist");
      return label_name;
  }
--- 2761,2767 ----
      char       *label_name;

      plpgsql_convert_ident(yytxt, &label_name, 1);
!     if (plpgsql_ns_lookup_label(plpgsql_ns_current(), label_name) == NULL)
          yyerror("label does not exist");
      return label_name;
  }
***************
*** 3005,3024 ****
       */
      if (t_expr)
      {
!         ListCell *l;
          PLpgSQL_var *t_var;
!         int        t_varno;

          /*
           * We don't yet know the result datatype of t_expr.  Build the
           * variable as if it were INT4; we'll fix this at runtime if needed.
           */
          t_var = (PLpgSQL_var *)
!             plpgsql_build_variable("*case*", lineno,
                                     plpgsql_build_datatype(INT4OID, -1),
!                                    false);
!         t_varno = t_var->dno;
!         new->t_varno = t_varno;

          foreach(l, case_when_list)
          {
--- 2962,2984 ----
       */
      if (t_expr)
      {
!         char    varname[32];
          PLpgSQL_var *t_var;
!         ListCell *l;
!
!         /* use a name unlikely to collide with any user names */
!         snprintf(varname, sizeof(varname), "__Case__Variable_%d__",
!                  plpgsql_nDatums);

          /*
           * We don't yet know the result datatype of t_expr.  Build the
           * variable as if it were INT4; we'll fix this at runtime if needed.
           */
          t_var = (PLpgSQL_var *)
!             plpgsql_build_variable(varname, lineno,
                                     plpgsql_build_datatype(INT4OID, -1),
!                                    true);
!         new->t_varno = t_var->dno;

          foreach(l, case_when_list)
          {
***************
*** 3026,3043 ****
              PLpgSQL_expr *expr = cwt->expr;
              StringInfoData    ds;

-             /* Must add the CASE variable as an extra param to expression */
-             expr->paramnos = bms_add_member(expr->paramnos, t_varno);
-
              /* copy expression query without SELECT keyword (expr->query + 7) */
              Assert(strncmp(expr->query, "SELECT ", 7) == 0);

              /* And do the string hacking */
              initStringInfo(&ds);

!             appendStringInfo(&ds, "SELECT $%d IN (%s)",
!                              t_varno + 1,
!                              expr->query + 7);

              pfree(expr->query);
              expr->query = pstrdup(ds.data);
--- 2986,2999 ----
              PLpgSQL_expr *expr = cwt->expr;
              StringInfoData    ds;

              /* copy expression query without SELECT keyword (expr->query + 7) */
              Assert(strncmp(expr->query, "SELECT ", 7) == 0);

              /* And do the string hacking */
              initStringInfo(&ds);

!             appendStringInfo(&ds, "SELECT \"%s\" IN (%s)",
!                              varname, expr->query + 7);

              pfree(expr->query);
              expr->query = pstrdup(ds.data);
Index: src/pl/plpgsql/src/pl_comp.c
===================================================================
RCS file: /cvsroot/pgsql/src/pl/plpgsql/src/pl_comp.c,v
retrieving revision 1.140
diff -c -r1.140 pl_comp.c
*** src/pl/plpgsql/src/pl_comp.c    4 Nov 2009 22:26:07 -0000    1.140
--- src/pl/plpgsql/src/pl_comp.c    6 Nov 2009 14:31:53 -0000
***************
*** 96,101 ****
--- 96,104 ----
             PLpgSQL_func_hashkey *hashkey,
             bool forValidator);
  static void add_dummy_return(PLpgSQL_function *function);
+ static Node *plpgsql_pre_column_ref(ParseState *pstate, ColumnRef *cref);
+ static Node *plpgsql_post_column_ref(ParseState *pstate, ColumnRef *cref, Node *var);
+ static Node *plpgsql_param_ref(ParseState *pstate, ParamRef *pref);
  static PLpgSQL_row *build_row_from_class(Oid classOid);
  static PLpgSQL_row *build_row_from_vars(PLpgSQL_variable **vars, int numvars);
  static PLpgSQL_type *build_datatype(HeapTuple typeTup, int32 typmod);
***************
*** 307,327 ****
      error_context_stack = &plerrcontext;

      /*
-      * Initialize the compiler, particularly the namespace stack.  The
-      * outermost namespace contains function parameters and other special
-      * variables (such as FOUND), and is named after the function itself.
-      */
-     plpgsql_ns_init();
-     plpgsql_ns_push(NameStr(procStruct->proname));
-     plpgsql_DumpExecTree = false;
-
-     datums_alloc = 128;
-     plpgsql_nDatums = 0;
-     /* This is short-lived, so needn't allocate in function's cxt */
-     plpgsql_Datums = palloc(sizeof(PLpgSQL_datum *) * datums_alloc);
-     datums_last = 0;
-
-     /*
       * Do extra syntax checks when validating the function definition. We skip
       * this when actually compiling functions for execution, for performance
       * reasons.
--- 310,315 ----
***************
*** 345,352 ****
      plpgsql_curr_compile = function;

      /*
!      * All the rest of the compile-time storage (e.g. parse tree) is kept in
!      * its own memory context, so it can be reclaimed easily.
       */
      func_cxt = AllocSetContextCreate(TopMemoryContext,
                                       "PL/PgSQL function context",
--- 333,340 ----
      plpgsql_curr_compile = function;

      /*
!      * All the permanent output of compilation (e.g. parse tree) is kept in
!      * a per-function memory context, so it can be reclaimed easily.
       */
      func_cxt = AllocSetContextCreate(TopMemoryContext,
                                       "PL/PgSQL function context",
***************
*** 363,368 ****
--- 351,372 ----
      function->fn_cxt = func_cxt;
      function->out_param_varno = -1;        /* set up for no OUT param */

+     /*
+      * Initialize the compiler, particularly the namespace stack.  The
+      * outermost namespace contains function parameters and other special
+      * variables (such as FOUND), and is named after the function itself.
+      */
+     plpgsql_ns_init();
+     plpgsql_ns_push(NameStr(procStruct->proname));
+     plpgsql_DumpExecTree = false;
+
+     datums_alloc = 128;
+     plpgsql_nDatums = 0;
+     /* This is short-lived, so needn't allocate in function's cxt */
+     plpgsql_Datums = MemoryContextAlloc(compile_tmp_cxt,
+                                         sizeof(PLpgSQL_datum *) * datums_alloc);
+     datums_last = 0;
+
      switch (is_trigger)
      {
          case false:
***************
*** 920,925 ****
--- 924,1205 ----
  }


+ /*
+  * plpgsql_parser_setup        set up parser hooks for dynamic parameters
+  *
+  * Note: although this routine, and the hook functions it prepares for,
+  * are logically part of plpgsql parsing, they actually run during function
+  * execution, when we are ready to evaluate a SQL query or expression
+  * that has not previously been parsed and planned.
+  */
+ void
+ plpgsql_parser_setup(struct ParseState *pstate, PLpgSQL_expr *expr)
+ {
+     pstate->p_pre_columnref_hook = plpgsql_pre_column_ref;
+     pstate->p_post_columnref_hook = plpgsql_post_column_ref;
+     pstate->p_paramref_hook = plpgsql_param_ref;
+     /* no need to use p_coerce_param_hook */
+     pstate->p_ref_hook_state = (void *) expr;
+ }
+
+ /*
+  * Helper for columnref parsing: build a Param referencing a plpgsql datum,
+  * and make sure that that datum is listed in the expression's paramnos.
+  */
+ static Node *
+ make_datum_param(PLpgSQL_expr *expr, int dno, int location)
+ {
+     PLpgSQL_execstate *estate;
+     Param       *param;
+     MemoryContext oldcontext;
+
+     /* see comment in plpgsql_pre_column_ref */
+     estate = expr->func->cur_estate;
+
+     Assert(dno >= 0 && dno < estate->ndatums);
+
+     /*
+      * Bitmapset must be allocated in function's permanent memory context
+      */
+     oldcontext = MemoryContextSwitchTo(expr->func->fn_cxt);
+     expr->paramnos = bms_add_member(expr->paramnos, dno);
+     MemoryContextSwitchTo(oldcontext);
+
+     param = makeNode(Param);
+     param->paramkind = PARAM_EXTERN;
+     param->paramid = dno + 1;
+     param->paramtype = exec_get_datum_type(estate, estate->datums[dno]);
+     param->paramtypmod = -1;
+     param->location = location;
+
+     return (Node *) param;
+ }
+
+ /*
+  * plpgsql_pre_column_ref        parser callback before parsing a ColumnRef
+  */
+ static Node *
+ plpgsql_pre_column_ref(ParseState *pstate, ColumnRef *cref)
+ {
+     PLpgSQL_expr *expr = (PLpgSQL_expr *) pstate->p_ref_hook_state;
+     PLpgSQL_execstate *estate;
+     PLpgSQL_nsitem *nse;
+     const char *name1;
+     const char *name2 = NULL;
+     const char *name3 = NULL;
+     const char *colname = NULL;
+     int            nnames;
+     int            nnames_scalar = 0;
+     int            nnames_wholerow = 0;
+     int            nnames_field = 0;
+
+     /*
+      * We use the function's current estate to resolve parameter data types.
+      * This is really pretty bogus because there is no provision for updating
+      * plans when those types change ...
+      */
+     estate = expr->func->cur_estate;
+
+     /*----------
+      * The allowed syntaxes are:
+      *
+      * A        Scalar variable reference, or whole-row record reference.
+      * A.B        Qualified scalar or whole-row reference, or field reference.
+      * A.B.C    Qualified record field reference.
+      * A.*        Whole-row record reference.
+      * A.B.*    Qualified whole-row record reference.
+      *----------
+      */
+     switch (list_length(cref->fields))
+     {
+         case 1:
+             {
+                 Node       *field1 = (Node *) linitial(cref->fields);
+
+                 Assert(IsA(field1, String));
+                 name1 = strVal(field1);
+                 nnames_scalar = 1;
+                 nnames_wholerow = 1;
+                 break;
+             }
+         case 2:
+             {
+                 Node       *field1 = (Node *) linitial(cref->fields);
+                 Node       *field2 = (Node *) lsecond(cref->fields);
+
+                 Assert(IsA(field1, String));
+                 name1 = strVal(field1);
+
+                 /* Whole-row reference? */
+                 if (IsA(field2, A_Star))
+                 {
+                     /* Set name2 to prevent matches to scalar variables */
+                     name2 = "*";
+                     nnames_wholerow = 1;
+                     break;
+                 }
+
+                 Assert(IsA(field2, String));
+                 name2 = strVal(field2);
+                 colname = name2;
+                 nnames_scalar = 2;
+                 nnames_wholerow = 2;
+                 nnames_field = 1;
+                 break;
+             }
+         case 3:
+             {
+                 Node       *field1 = (Node *) linitial(cref->fields);
+                 Node       *field2 = (Node *) lsecond(cref->fields);
+                 Node       *field3 = (Node *) lthird(cref->fields);
+
+                 Assert(IsA(field1, String));
+                 name1 = strVal(field1);
+                 Assert(IsA(field2, String));
+                 name2 = strVal(field2);
+
+                 /* Whole-row reference? */
+                 if (IsA(field3, A_Star))
+                 {
+                     /* Set name3 to prevent matches to scalar variables */
+                     name3 = "*";
+                     nnames_wholerow = 2;
+                     break;
+                 }
+
+                 Assert(IsA(field3, String));
+                 name3 = strVal(field3);
+                 colname = name3;
+                 nnames_field = 2;
+                 break;
+             }
+         default:
+             /* too many names, ignore */
+             return NULL;
+     }
+
+     nse = plpgsql_ns_lookup(expr->ns,
+                             name1, name2, name3,
+                             &nnames);
+
+     if (nse == NULL)
+         return NULL;            /* name not known to plpgsql */
+
+     switch (nse->itemtype)
+     {
+         case PLPGSQL_NSTYPE_VAR:
+             if (nnames == nnames_scalar)
+                 return make_datum_param(expr, nse->itemno, cref->location);
+             break;
+         case PLPGSQL_NSTYPE_REC:
+             if (nnames == nnames_wholerow)
+                 return make_datum_param(expr, nse->itemno, cref->location);
+             if (nnames == nnames_field)
+             {
+                 /* colname must be a field in this record */
+                 PLpgSQL_rec *rec = (PLpgSQL_rec *) estate->datums[nse->itemno];
+                 FieldSelect *fselect;
+                 Oid            fldtype;
+                 int            fldno;
+                 int            i;
+
+                 /* search for a datum referencing this field */
+                 for (i = 0; i < estate->ndatums; i++)
+                 {
+                     PLpgSQL_recfield *fld = (PLpgSQL_recfield *) estate->datums[i];
+
+                     if (fld->dtype == PLPGSQL_DTYPE_RECFIELD &&
+                         fld->recparentno == nse->itemno &&
+                         strcmp(fld->fieldname, colname) == 0)
+                     {
+                         return make_datum_param(expr, i, cref->location);
+                     }
+                 }
+
+                 /*
+                  * We can't readily add a recfield datum at runtime, so
+                  * instead build a whole-row Param and a FieldSelect node.
+                  * This is a bit less efficient, so we prefer the recfield
+                  * way when possible.
+                  */
+                 fldtype = exec_get_rec_fieldtype(rec, colname,
+                                                  &fldno);
+                 fselect = makeNode(FieldSelect);
+                 fselect->arg = (Expr *) make_datum_param(expr, nse->itemno,
+                                                          cref->location);
+                 fselect->fieldnum = fldno;
+                 fselect->resulttype = fldtype;
+                 fselect->resulttypmod = -1;
+                 return (Node *) fselect;
+             }
+             break;
+         case PLPGSQL_NSTYPE_ROW:
+             if (nnames == nnames_wholerow)
+                 return make_datum_param(expr, nse->itemno, cref->location);
+             if (nnames == nnames_field)
+             {
+                 /* colname must be a field in this row */
+                 PLpgSQL_row *row = (PLpgSQL_row *) estate->datums[nse->itemno];
+                 int            i;
+
+                 for (i = 0; i < row->nfields; i++)
+                 {
+                     if (row->fieldnames[i] &&
+                         strcmp(row->fieldnames[i], colname) == 0)
+                     {
+                         return make_datum_param(expr, row->varnos[i],
+                                                 cref->location);
+                     }
+                 }
+                 ereport(ERROR,
+                         (errcode(ERRCODE_UNDEFINED_COLUMN),
+                          errmsg("row \"%s\" has no field \"%s\"",
+                                 row->refname, colname)));
+             }
+             break;
+         default:
+             elog(ERROR, "unrecognized plpgsql itemtype");
+     }
+
+     /* Name format doesn't match the plpgsql variable type */
+     return NULL;
+ }
+
+ /*
+  * plpgsql_post_column_ref        parser callback after parsing a ColumnRef
+  */
+ static Node *
+ plpgsql_post_column_ref(ParseState *pstate, ColumnRef *cref, Node *var)
+ {
+     /* Not used yet */
+     return NULL;
+ }
+
+ /*
+  * plpgsql_param_ref        parser callback for ParamRefs ($n symbols)
+  */
+ static Node *
+ plpgsql_param_ref(ParseState *pstate, ParamRef *pref)
+ {
+     PLpgSQL_expr *expr = (PLpgSQL_expr *) pstate->p_ref_hook_state;
+     char        pname[32];
+     PLpgSQL_nsitem *nse;
+
+     snprintf(pname, sizeof(pname), "$%d", pref->number);
+
+     nse = plpgsql_ns_lookup(expr->ns,
+                             pname, NULL, NULL,
+                             NULL);
+
+     if (nse == NULL)
+         return NULL;            /* name not known to plpgsql */
+
+     Assert(nse->itemtype == PLPGSQL_NSTYPE_VAR);
+
+     return make_datum_param(expr, nse->itemno, pref->location);
+ }
+
+
  /* ----------
   * plpgsql_parse_word        The scanner calls this to postparse
   *                any single word not found by a
***************
*** 936,944 ****
      plpgsql_convert_ident(word, cp, 1);

      /*
!      * Do a lookup on the compiler's namestack
       */
!     nse = plpgsql_ns_lookup(cp[0], NULL, NULL, NULL);
      pfree(cp[0]);

      if (nse != NULL)
--- 1216,1226 ----
      plpgsql_convert_ident(word, cp, 1);

      /*
!      * Do a lookup in the current namespace stack
       */
!     nse = plpgsql_ns_lookup(plpgsql_ns_current(),
!                             cp[0], NULL, NULL,
!                             NULL);
      pfree(cp[0]);

      if (nse != NULL)
***************
*** 986,994 ****
      plpgsql_convert_ident(word, cp, 2);

      /*
!      * Do a lookup on the compiler's namestack
       */
!     ns = plpgsql_ns_lookup(cp[0], cp[1], NULL, &nnames);
      if (ns == NULL)
      {
          pfree(cp[0]);
--- 1268,1278 ----
      plpgsql_convert_ident(word, cp, 2);

      /*
!      * Do a lookup in the current namespace stack
       */
!     ns = plpgsql_ns_lookup(plpgsql_ns_current(),
!                            cp[0], cp[1], NULL,
!                            &nnames);
      if (ns == NULL)
      {
          pfree(cp[0]);
***************
*** 1098,1107 ****
      plpgsql_convert_ident(word, cp, 3);

      /*
!      * Do a lookup on the compiler's namestack. Must find a qualified
       * reference.
       */
!     ns = plpgsql_ns_lookup(cp[0], cp[1], cp[2], &nnames);
      if (ns == NULL || nnames != 2)
      {
          pfree(cp[0]);
--- 1382,1393 ----
      plpgsql_convert_ident(word, cp, 3);

      /*
!      * Do a lookup in the current namespace stack. Must find a qualified
       * reference.
       */
!     ns = plpgsql_ns_lookup(plpgsql_ns_current(),
!                            cp[0], cp[1], cp[2],
!                            &nnames);
      if (ns == NULL || nnames != 2)
      {
          pfree(cp[0]);
***************
*** 1201,1210 ****
      pfree(cp[1]);

      /*
!      * Do a lookup on the compiler's namestack.  Ensure we scan all levels.
       */
      old_nsstate = plpgsql_ns_setlocal(false);
!     nse = plpgsql_ns_lookup(cp[0], NULL, NULL, NULL);
      plpgsql_ns_setlocal(old_nsstate);

      if (nse != NULL)
--- 1487,1498 ----
      pfree(cp[1]);

      /*
!      * Do a lookup in the current namespace stack.  Ensure we scan all levels.
       */
      old_nsstate = plpgsql_ns_setlocal(false);
!     nse = plpgsql_ns_lookup(plpgsql_ns_current(),
!                             cp[0], NULL, NULL,
!                             NULL);
      plpgsql_ns_setlocal(old_nsstate);

      if (nse != NULL)
***************
*** 1224,1231 ****
      }

      /*
!      * Word wasn't found on the namestack. Try to find a data type with that
!      * name, but ignore shell types and complex types.
       */
      typeTup = LookupTypeName(NULL, makeTypeName(cp[0]), NULL);
      if (typeTup)
--- 1512,1519 ----
      }

      /*
!      * Word wasn't found in the namespace stack. Try to find a data type
!      * with that name, but ignore shell types and complex types.
       */
      typeTup = LookupTypeName(NULL, makeTypeName(cp[0]), NULL);
      if (typeTup)
***************
*** 1289,1300 ****
      pfree(cp[2]);

      /*
!      * Do a lookup on the compiler's namestack.  Ensure we scan all levels. We
!      * don't need to check number of names matched, because we will only
       * consider scalar variables.
       */
      old_nsstate = plpgsql_ns_setlocal(false);
!     nse = plpgsql_ns_lookup(cp[0], cp[1], NULL, NULL);
      plpgsql_ns_setlocal(old_nsstate);

      if (nse != NULL && nse->itemtype == PLPGSQL_NSTYPE_VAR)
--- 1577,1590 ----
      pfree(cp[2]);

      /*
!      * Do a lookup in the current namespace stack.  Ensure we scan all levels.
!      * We don't need to check number of names matched, because we will only
       * consider scalar variables.
       */
      old_nsstate = plpgsql_ns_setlocal(false);
!     nse = plpgsql_ns_lookup(plpgsql_ns_current(),
!                             cp[0], cp[1], NULL,
!                             NULL);
      plpgsql_ns_setlocal(old_nsstate);

      if (nse != NULL && nse->itemtype == PLPGSQL_NSTYPE_VAR)
Index: src/pl/plpgsql/src/pl_exec.c
===================================================================
RCS file: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v
retrieving revision 1.249
diff -c -r1.249 pl_exec.c
*** src/pl/plpgsql/src/pl_exec.c    4 Nov 2009 22:26:07 -0000    1.249
--- src/pl/plpgsql/src/pl_exec.c    6 Nov 2009 14:31:53 -0000
***************
*** 26,32 ****
  #include "lib/stringinfo.h"
  #include "miscadmin.h"
  #include "nodes/nodeFuncs.h"
- #include "parser/parse_node.h"
  #include "parser/scansup.h"
  #include "storage/proc.h"
  #include "tcop/tcopprot.h"
--- 26,31 ----
***************
*** 158,165 ****
                  Oid *typeid,
                  Datum *value,
                  bool *isnull);
- static Oid exec_get_datum_type(PLpgSQL_execstate *estate,
-                                PLpgSQL_datum *datum);
  static int exec_eval_integer(PLpgSQL_execstate *estate,
                    PLpgSQL_expr *expr,
                    bool *isNull);
--- 157,162 ----
***************
*** 176,183 ****
                 Portal portal, bool prefetch_ok);
  static ParamListInfo setup_param_list(PLpgSQL_execstate *estate,
                                        PLpgSQL_expr *expr);
- static void plpgsql_parser_setup(ParseState *pstate, PLpgSQL_expr *expr);
- static Node *plpgsql_param_ref(ParseState *pstate, ParamRef *pref);
  static void plpgsql_param_fetch(ParamListInfo params, int paramid);
  static void exec_move_row(PLpgSQL_execstate *estate,
                PLpgSQL_rec *rec,
--- 173,178 ----
***************
*** 3992,3998 ****
   * a tupdesc but no row value for a record variable.  (This currently can
   * happen only for a trigger's NEW/OLD records.)
   */
! static Oid
  exec_get_datum_type(PLpgSQL_execstate *estate,
                      PLpgSQL_datum *datum)
  {
--- 3987,3993 ----
   * a tupdesc but no row value for a record variable.  (This currently can
   * happen only for a trigger's NEW/OLD records.)
   */
! Oid
  exec_get_datum_type(PLpgSQL_execstate *estate,
                      PLpgSQL_datum *datum)
  {
***************
*** 4068,4073 ****
--- 4063,4098 ----
      return typeid;
  }

+ /*
+  * exec_get_rec_fieldtype                Get datatype of a PLpgSQL record field
+  *
+  * Also returns the field number to *fieldno.
+  */
+ Oid
+ exec_get_rec_fieldtype(PLpgSQL_rec *rec, const char *fieldname,
+                        int *fieldno)
+ {
+     Oid            typeid;
+     int            fno;
+
+     if (rec->tupdesc == NULL)
+         ereport(ERROR,
+                 (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+                  errmsg("record \"%s\" is not assigned yet",
+                         rec->refname),
+                  errdetail("The tuple structure of a not-yet-assigned record is indeterminate.")));
+     fno = SPI_fnumber(rec->tupdesc, fieldname);
+     if (fno == SPI_ERROR_NOATTRIBUTE)
+         ereport(ERROR,
+                 (errcode(ERRCODE_UNDEFINED_COLUMN),
+                  errmsg("record \"%s\" has no field \"%s\"",
+                         rec->refname, fieldname)));
+     typeid = SPI_gettypeid(rec->tupdesc, fno);
+
+     *fieldno = fno;
+     return typeid;
+ }
+
  /* ----------
   * exec_eval_integer        Evaluate an expression, coerce result to int4
   *
***************
*** 4591,4641 ****
  }

  /*
-  * plpgsql_parser_setup        set up parser hooks for dynamic parameters
-  */
- static void
- plpgsql_parser_setup(ParseState *pstate, PLpgSQL_expr *expr)
- {
-     pstate->p_ref_hook_state = (void *) expr;
-     pstate->p_paramref_hook = plpgsql_param_ref;
-     /* no need to use p_coerce_param_hook */
- }
-
- /*
-  * plpgsql_param_ref        parser callback for ParamRefs ($n symbols)
-  */
- static Node *
- plpgsql_param_ref(ParseState *pstate, ParamRef *pref)
- {
-     int            paramno = pref->number;
-     PLpgSQL_expr *expr = (PLpgSQL_expr *) pstate->p_ref_hook_state;
-     PLpgSQL_execstate *estate;
-     Param       *param;
-
-     /* Let's just check parameter number is in range */
-     if (!bms_is_member(paramno-1, expr->paramnos))
-         return NULL;
-
-     /*
-      * We use the function's current estate to resolve parameter data types.
-      * This is really pretty bogus because there is no provision for updating
-      * plans when those types change ...
-      */
-     estate = expr->func->cur_estate;
-     Assert(paramno <= estate->ndatums);
-
-     param = makeNode(Param);
-     param->paramkind = PARAM_EXTERN;
-     param->paramid = paramno;
-     param->paramtype = exec_get_datum_type(estate,
-                                            estate->datums[paramno-1]);
-     param->paramtypmod = -1;
-     param->location = pref->location;
-
-     return (Node *) param;
- }
-
- /*
   * plpgsql_param_fetch        paramFetch callback for dynamic parameter fetch
   */
  static void
--- 4616,4621 ----
Index: src/pl/plpgsql/src/pl_funcs.c
===================================================================
RCS file: /cvsroot/pgsql/src/pl/plpgsql/src/pl_funcs.c,v
retrieving revision 1.83
diff -c -r1.83 pl_funcs.c
*** src/pl/plpgsql/src/pl_funcs.c    5 Nov 2009 16:58:36 -0000    1.83
--- src/pl/plpgsql/src/pl_funcs.c    6 Nov 2009 14:31:53 -0000
***************
*** 21,27 ****


  /* ----------
!  * Local variables for the namestack handling
   * ----------
   */
  static PLpgSQL_ns *ns_current = NULL;
--- 21,34 ----


  /* ----------
!  * Local variables for namespace handling
!  *
!  * The namespace structure actually forms a tree, of which only one chain
!  * is accessible from any one plpgsql statement.  During initial parsing
!  * of a function, ns_current points to the chain accessible from the
!  * block currently being parsed.  We store the entire tree, however,
!  * since at runtime we will need to access the chain that's relevant to
!  * any one statement.
   * ----------
   */
  static PLpgSQL_ns *ns_current = NULL;
***************
*** 29,35 ****


  /* ----------
!  * plpgsql_ns_init            Initialize the namestack
   * ----------
   */
  void
--- 36,42 ----


  /* ----------
!  * plpgsql_ns_init            Initialize namespace processing for a new function
   * ----------
   */
  void
***************
*** 49,55 ****
   * examining a name being declared in a DECLARE section.  For that case
   * we only want to know if there is a conflicting name earlier in the
   * same DECLARE section.  So the grammar must temporarily set local mode
!  * before scanning decl_varnames.
   * ----------
   */
  bool
--- 56,64 ----
   * examining a name being declared in a DECLARE section.  For that case
   * we only want to know if there is a conflicting name earlier in the
   * same DECLARE section.  So the grammar must temporarily set local mode
!  * before scanning decl_varnames.  This should eventually go away in favor
!  * of a localmode argument to plpgsql_ns_lookup, or perhaps some less
!  * indirect method of dealing with duplicate namespace entries.
   * ----------
   */
  bool
***************
*** 64,70 ****


  /* ----------
!  * plpgsql_ns_push            Enter a new namestack level
   * ----------
   */
  void
--- 73,79 ----


  /* ----------
!  * plpgsql_ns_push            Create a new namespace level
   * ----------
   */
  void
***************
*** 79,84 ****
--- 88,94 ----
      new->upper = ns_current;
      ns_current = new;

+     /* The label will always be item zero in the namespace */
      plpgsql_ns_additem(PLPGSQL_NSTYPE_LABEL, 0, label);
  }

***************
*** 90,111 ****
  void
  plpgsql_ns_pop(void)
  {
!     int            i;
!     PLpgSQL_ns *old;

-     old = ns_current;
-     ns_current = old->upper;

!     for (i = 0; i < old->items_used; i++)
!         pfree(old->items[i]);
!     pfree(old->items);
!     pfree(old);
  }


  /* ----------
!  * plpgsql_ns_additem            Add an item to the current
!  *                    namestack level
   * ----------
   */
  void
--- 100,122 ----
  void
  plpgsql_ns_pop(void)
  {
!     ns_current = ns_current->upper;
! }


! /* ----------
!  * plpgsql_ns_current            Fetch the current namespace chain
!  * ----------
!  */
! PLpgSQL_ns *
! plpgsql_ns_current(void)
! {
!     return ns_current;
  }


  /* ----------
!  * plpgsql_ns_additem            Add an item to the current namespace level
   * ----------
   */
  void
***************
*** 140,146 ****


  /* ----------
!  * plpgsql_ns_lookup            Lookup an identifier in the namestack
   *
   * Note that this only searches for variables, not labels.
   *
--- 151,157 ----


  /* ----------
!  * plpgsql_ns_lookup            Lookup an identifier in given namespace chain
   *
   * Note that this only searches for variables, not labels.
   *
***************
*** 158,171 ****
   * ----------
   */
  PLpgSQL_nsitem *
! plpgsql_ns_lookup(const char *name1, const char *name2, const char *name3,
                    int *names_used)
  {
      PLpgSQL_ns *ns;
      int            i;

!     /* Scan each level of the namestack */
!     for (ns = ns_current; ns != NULL; ns = ns->upper)
      {
          /* Check for unqualified match to variable name */
          for (i = 1; i < ns->items_used; i++)
--- 169,183 ----
   * ----------
   */
  PLpgSQL_nsitem *
! plpgsql_ns_lookup(PLpgSQL_ns *ns_cur,
!                   const char *name1, const char *name2, const char *name3,
                    int *names_used)
  {
      PLpgSQL_ns *ns;
      int            i;

!     /* Scan each level of the namespace chain */
!     for (ns = ns_cur; ns != NULL; ns = ns->upper)
      {
          /* Check for unqualified match to variable name */
          for (i = 1; i < ns->items_used; i++)
***************
*** 217,231 ****


  /* ----------
!  * plpgsql_ns_lookup_label        Lookup a label in the namestack
   * ----------
   */
  PLpgSQL_nsitem *
! plpgsql_ns_lookup_label(const char *name)
  {
      PLpgSQL_ns *ns;

!     for (ns = ns_current; ns != NULL; ns = ns->upper)
      {
          if (strcmp(ns->items[0]->name, name) == 0)
              return ns->items[0];
--- 229,243 ----


  /* ----------
!  * plpgsql_ns_lookup_label        Lookup a label in the given namespace chain
   * ----------
   */
  PLpgSQL_nsitem *
! plpgsql_ns_lookup_label(PLpgSQL_ns *ns_cur, const char *name)
  {
      PLpgSQL_ns *ns;

!     for (ns = ns_cur; ns != NULL; ns = ns->upper)
      {
          if (strcmp(ns->items[0]->name, name) == 0)
              return ns->items[0];
Index: src/pl/plpgsql/src/plpgsql.h
===================================================================
RCS file: /cvsroot/pgsql/src/pl/plpgsql/src/plpgsql.h,v
retrieving revision 1.119
diff -c -r1.119 plpgsql.h
*** src/pl/plpgsql/src/plpgsql.h    5 Nov 2009 16:58:36 -0000    1.119
--- src/pl/plpgsql/src/plpgsql.h    6 Nov 2009 14:31:53 -0000
***************
*** 37,43 ****
  #define _(x) dgettext(TEXTDOMAIN, x)

  /* ----------
!  * Compiler's namestack item types
   * ----------
   */
  enum
--- 37,43 ----
  #define _(x) dgettext(TEXTDOMAIN, x)

  /* ----------
!  * Compiler's namespace item types
   * ----------
   */
  enum
***************
*** 193,198 ****
--- 193,201 ----
      /* function containing this expr (not set until we first parse query) */
      struct PLpgSQL_function *func;

+     /* namespace chain visible to this expr */
+     struct PLpgSQL_ns *ns;
+
      /* fields for "simple expression" fast-path execution: */
      Expr       *expr_simple_expr;        /* NULL means not a simple expr */
      int            expr_simple_generation; /* plancache generation we checked */
***************
*** 284,290 ****


  typedef struct
! {                                /* Item in the compilers namestack    */
      int            itemtype;
      int            itemno;
      char        name[1];        /* actually, as long as needed */
--- 287,293 ----


  typedef struct
! {                                /* Item in the compilers namespace tree */
      int            itemtype;
      int            itemno;
      char        name[1];        /* actually, as long as needed */
***************
*** 293,299 ****

  /* XXX: consider adapting this to use List */
  typedef struct PLpgSQL_ns
! {                                /* Compiler namestack level        */
      int            items_alloc;
      int            items_used;
      PLpgSQL_nsitem **items;
--- 296,302 ----

  /* XXX: consider adapting this to use List */
  typedef struct PLpgSQL_ns
! {                                /* Compiler namespace level        */
      int            items_alloc;
      int            items_used;
      PLpgSQL_nsitem **items;
***************
*** 795,800 ****
--- 798,805 ----
  extern PLpgSQL_function *plpgsql_compile(FunctionCallInfo fcinfo,
                  bool forValidator);
  extern PLpgSQL_function *plpgsql_compile_inline(char *proc_source);
+ extern void plpgsql_parser_setup(struct ParseState *pstate,
+                                  PLpgSQL_expr *expr);
  extern int    plpgsql_parse_word(const char *word);
  extern int    plpgsql_parse_dblword(const char *word);
  extern int    plpgsql_parse_tripword(const char *word);
***************
*** 838,856 ****
  extern void plpgsql_xact_cb(XactEvent event, void *arg);
  extern void plpgsql_subxact_cb(SubXactEvent event, SubTransactionId mySubid,
                     SubTransactionId parentSubid, void *arg);

  /* ----------
!  * Functions for namestack handling in pl_funcs.c
   * ----------
   */
  extern void plpgsql_ns_init(void);
  extern bool plpgsql_ns_setlocal(bool flag);
  extern void plpgsql_ns_push(const char *label);
  extern void plpgsql_ns_pop(void);
  extern void plpgsql_ns_additem(int itemtype, int itemno, const char *name);
! extern PLpgSQL_nsitem *plpgsql_ns_lookup(const char *name1, const char *name2,
!                   const char *name3, int *names_used);
! extern PLpgSQL_nsitem *plpgsql_ns_lookup_label(const char *name);

  /* ----------
   * Other functions in pl_funcs.c
--- 843,868 ----
  extern void plpgsql_xact_cb(XactEvent event, void *arg);
  extern void plpgsql_subxact_cb(SubXactEvent event, SubTransactionId mySubid,
                     SubTransactionId parentSubid, void *arg);
+ extern Oid exec_get_datum_type(PLpgSQL_execstate *estate,
+                                PLpgSQL_datum *datum);
+ extern Oid exec_get_rec_fieldtype(PLpgSQL_rec *rec, const char *fieldname,
+                        int *fieldno);

  /* ----------
!  * Functions for namespace handling in pl_funcs.c
   * ----------
   */
  extern void plpgsql_ns_init(void);
  extern bool plpgsql_ns_setlocal(bool flag);
  extern void plpgsql_ns_push(const char *label);
  extern void plpgsql_ns_pop(void);
+ extern PLpgSQL_ns *plpgsql_ns_current(void);
  extern void plpgsql_ns_additem(int itemtype, int itemno, const char *name);
! extern PLpgSQL_nsitem *plpgsql_ns_lookup(PLpgSQL_ns *ns_cur,
!                                          const char *name1, const char *name2,
!                                          const char *name3, int *names_used);
! extern PLpgSQL_nsitem *plpgsql_ns_lookup_label(PLpgSQL_ns *ns_cur,
!                                                const char *name);

  /* ----------
   * Other functions in pl_funcs.c
Index: src/test/regress/expected/plpgsql.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/plpgsql.out,v
retrieving revision 1.77
diff -c -r1.77 plpgsql.out
*** src/test/regress/expected/plpgsql.out    5 Nov 2009 16:58:36 -0000    1.77
--- src/test/regress/expected/plpgsql.out    6 Nov 2009 14:31:53 -0000
***************
*** 899,905 ****
          declare
          rec        record;
      begin
!         select into rec * from PLine where slotname = outer.rec.backlink;
          retval := ''Phone line '' || trim(rec.phonenumber);
          if rec.comment != '''' then
              retval := retval || '' ('';
--- 899,905 ----
          declare
          rec        record;
      begin
!         select into rec * from PLine where slotname = "outer".rec.backlink;
          retval := ''Phone line '' || trim(rec.phonenumber);
          if rec.comment != '''' then
              retval := retval || '' ('';
Index: src/test/regress/sql/plpgsql.sql
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/sql/plpgsql.sql,v
retrieving revision 1.65
diff -c -r1.65 plpgsql.sql
*** src/test/regress/sql/plpgsql.sql    5 Nov 2009 16:58:36 -0000    1.65
--- src/test/regress/sql/plpgsql.sql    6 Nov 2009 14:31:54 -0000
***************
*** 1026,1032 ****
          declare
          rec        record;
      begin
!         select into rec * from PLine where slotname = outer.rec.backlink;
          retval := ''Phone line '' || trim(rec.phonenumber);
          if rec.comment != '''' then
              retval := retval || '' ('';
--- 1026,1032 ----
          declare
          rec        record;
      begin
!         select into rec * from PLine where slotname = "outer".rec.backlink;
          retval := ''Phone line '' || trim(rec.phonenumber);
          if rec.comment != '''' then
              retval := retval || '' ('';

pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: Why do OLD and NEW have special internal names?
Next
From: Alvaro Herrera
Date:
Subject: Re: "ERROR: could not read block 6 ...: read only 0 of 8192 bytes" after autovacuum cancelled