Re: [HACKERS] Open items for 8.2 - Mailing list pgsql-patches

From Tom Lane
Subject Re: [HACKERS] Open items for 8.2
Date
Msg-id 20616.1157561009@sss.pgh.pa.us
Whole thread Raw
List pgsql-patches
Tom Lane <tgl@sss.pgh.pa.us> writes:
> Peter Eisentraut <peter_e@gmx.net> writes:
>> Am Dienstag, 5. September 2006 05:58 schrieb Tom Lane:
>>> A couple of recently discussed FE/BE protocol issues are: not storing a
>>> plan at all for unnamed-statement cases, and thus allowing bind
>>> parameters to be treated as constants; allowing parameter types to go
>>> unresolved rather than throwing an error.  Perhaps it's too late to
>>> consider these for 8.2, but they seem no more invasive than some other
>>> items on the open-issues list.

>> Do we have a patch for that today?

> We could have a patch for the first one today --- I was thinking about
> it last night and intending to code it today.

Proposed patch attached --- just the code, haven't looked at what to
change in the documentation yet.  There's not a lot to it.

            regards, tom lane


*** src/backend/commands/explain.c.orig    Tue Aug  1 21:59:45 2006
--- src/backend/commands/explain.c    Wed Sep  6 12:28:15 2006
***************
*** 191,197 ****
      }

      /* plan the query */
!     plan = planner(query, isCursor, cursorOptions, NULL);

      /*
       * Update snapshot command ID to ensure this query sees results of any
--- 191,197 ----
      }

      /* plan the query */
!     plan = planner(query, isCursor, cursorOptions, params);

      /*
       * Update snapshot command ID to ensure this query sees results of any
*** src/backend/commands/portalcmds.c.orig    Sun Sep  3 13:44:27 2006
--- src/backend/commands/portalcmds.c    Wed Sep  6 12:28:15 2006
***************
*** 95,101 ****
                errmsg("DECLARE CURSOR ... FOR UPDATE/SHARE is not supported"),
                   errdetail("Cursors must be READ ONLY.")));

!     plan = planner(query, true, stmt->options, NULL);

      /*
       * Create a portal and copy the query and plan into its memory context.
--- 95,101 ----
                errmsg("DECLARE CURSOR ... FOR UPDATE/SHARE is not supported"),
                   errdetail("Cursors must be READ ONLY.")));

!     plan = planner(query, true, stmt->options, params);

      /*
       * Create a portal and copy the query and plan into its memory context.
*** src/backend/commands/prepare.c.orig    Wed Aug 30 13:02:43 2006
--- src/backend/commands/prepare.c    Wed Sep  6 11:22:11 2006
***************
*** 261,266 ****
--- 261,267 ----
          ParamExternData *prm = ¶mLI->params[i];

          prm->ptype = lfirst_oid(la);
+         prm->pflags = 0;
          prm->value = ExecEvalExprSwitchContext(n,
                                                 GetPerTupleExprContext(estate),
                                                 &prm->isnull,
*** src/backend/executor/functions.c.orig    Sat Aug 12 16:05:55 2006
--- src/backend/executor/functions.c    Wed Sep  6 11:22:20 2006
***************
*** 442,447 ****
--- 442,448 ----

              prm->value = fcinfo->arg[i];
              prm->isnull = fcinfo->argnull[i];
+             prm->pflags = 0;
              prm->ptype = fcache->argtypes[i];
          }
      }
*** src/backend/executor/spi.c.orig    Sun Sep  3 13:44:27 2006
--- src/backend/executor/spi.c    Wed Sep  6 11:22:20 2006
***************
*** 893,898 ****
--- 893,899 ----
              ParamExternData *prm = ¶mLI->params[k];

              prm->ptype = spiplan->argtypes[k];
+             prm->pflags = 0;
              prm->isnull = (Nulls && Nulls[k] == 'n');
              if (prm->isnull)
              {
***************
*** 1357,1362 ****
--- 1358,1364 ----

                  prm->value = Values[k];
                  prm->isnull = (Nulls && Nulls[k] == 'n');
+                 prm->pflags = 0;
                  prm->ptype = plan->argtypes[k];
              }
          }
*** src/backend/optimizer/util/clauses.c.orig    Sat Aug 12 16:05:55 2006
--- src/backend/optimizer/util/clauses.c    Wed Sep  6 12:04:03 2006
***************
*** 1462,1468 ****
   *
   * Currently the extra steps that are taken in this mode are:
   * 1. Substitute values for Params, where a bound Param value has been made
!  *      available by the caller of planner().
   * 2. Fold stable, as well as immutable, functions to constants.
   *--------------------
   */
--- 1462,1470 ----
   *
   * Currently the extra steps that are taken in this mode are:
   * 1. Substitute values for Params, where a bound Param value has been made
!  *      available by the caller of planner(), even if the Param isn't marked
!  *      constant.  This effectively means that we plan using the first supplied
!  *      value of the Param.
   * 2. Fold stable, as well as immutable, functions to constants.
   *--------------------
   */
***************
*** 1487,1519 ****
      {
          Param       *param = (Param *) node;

!         /* OK to try to substitute value? */
!         if (context->estimate && param->paramkind == PARAM_EXTERN &&
!             PlannerBoundParamList != NULL)
          {
!             /* Look to see if we've been given a value for this Param */
!             if (param->paramid > 0 &&
!                 param->paramid <= PlannerBoundParamList->numParams)
!             {
!                 ParamExternData *prm = &PlannerBoundParamList->params[param->paramid - 1];

!                 if (OidIsValid(prm->ptype))
                  {
                      /*
!                      * Found it, so return a Const representing the param
!                      * value.  Note that we don't copy pass-by-ref datatypes,
!                      * so the Const will only be valid as long as the bound
!                      * parameter list exists.  This is okay for intended uses
!                      * of estimate_expression_value().
                       */
                      int16        typLen;
                      bool        typByVal;

                      Assert(prm->ptype == param->paramtype);
                      get_typlenbyval(param->paramtype, &typLen, &typByVal);
                      return (Node *) makeConst(param->paramtype,
                                                (int) typLen,
!                                               prm->value,
                                                prm->isnull,
                                                typByVal);
                  }
--- 1489,1526 ----
      {
          Param       *param = (Param *) node;

!         /* Look to see if we've been given a value for this Param */
!         if (param->paramkind == PARAM_EXTERN &&
!             PlannerBoundParamList != NULL &&
!             param->paramid > 0 &&
!             param->paramid <= PlannerBoundParamList->numParams)
          {
!             ParamExternData *prm = &PlannerBoundParamList->params[param->paramid - 1];

!             if (OidIsValid(prm->ptype))
!             {
!                 /* OK to substitute parameter value? */
!                 if (context->estimate || (prm->pflags & PARAM_FLAG_CONST))
                  {
                      /*
!                      * Return a Const representing the param value.  Must copy
!                      * pass-by-ref datatypes, since the Param might be in a
!                      * memory context shorter-lived than our output plan
!                      * should be.
                       */
                      int16        typLen;
                      bool        typByVal;
+                     Datum        pval;

                      Assert(prm->ptype == param->paramtype);
                      get_typlenbyval(param->paramtype, &typLen, &typByVal);
+                     if (prm->isnull || typByVal)
+                         pval = prm->value;
+                     else
+                         pval = datumCopy(prm->value, typByVal, typLen);
                      return (Node *) makeConst(param->paramtype,
                                                (int) typLen,
!                                               pval,
                                                prm->isnull,
                                                typByVal);
                  }
*** src/backend/tcop/postgres.c.orig    Sun Sep  3 13:44:33 2006
--- src/backend/tcop/postgres.c    Wed Sep  6 11:52:42 2006
***************
*** 1168,1174 ****
       * statement, we assume the statement isn't going to hang around long, so
       * getting rid of temp space quickly is probably not worth the costs of
       * copying parse/plan trees.  So in this case, we set up a special context
!      * for the unnamed statement, and do all the parsing/planning therein.
       */
      is_named = (stmt_name[0] != '\0');
      if (is_named)
--- 1168,1174 ----
       * statement, we assume the statement isn't going to hang around long, so
       * getting rid of temp space quickly is probably not worth the costs of
       * copying parse/plan trees.  So in this case, we set up a special context
!      * for the unnamed statement, and do all the parsing work therein.
       */
      is_named = (stmt_name[0] != '\0');
      if (is_named)
***************
*** 1367,1372 ****
--- 1367,1373 ----
      PreparedStatement *pstmt;
      Portal        portal;
      ParamListInfo params;
+     List       *plan_list;
      StringInfoData bind_values_str;

      pgstat_report_activity("<BIND>");
***************
*** 1474,1479 ****
--- 1475,1481 ----
          {
              Oid            ptype = lfirst_oid(l);
              int32        plength;
+             Datum        pval;
              bool        isNull;
              StringInfoData pbuf;
              char        csave;
***************
*** 1532,1542 ****
                  else
                      pstring = pg_client_to_server(pbuf.data, plength);

!                 params->params[paramno].value =
!                     OidInputFunctionCall(typinput,
!                                          pstring,
!                                          typioparam,
!                                          -1);

                  /* Save the parameter values */
                  appendStringInfo(&bind_values_str, "%s$%d = ",
--- 1534,1540 ----
                  else
                      pstring = pg_client_to_server(pbuf.data, plength);

!                 pval = OidInputFunctionCall(typinput, pstring, typioparam, -1);

                  /* Save the parameter values */
                  appendStringInfo(&bind_values_str, "%s$%d = ",
***************
*** 1576,1585 ****
                  else
                      bufptr = &pbuf;

!                 params->params[paramno].value = OidReceiveFunctionCall(typreceive,
!                                                                  bufptr,
!                                                                  typioparam,
!                                                                  -1);

                  /* Trouble if it didn't eat the whole buffer */
                  if (!isNull && pbuf.cursor != pbuf.len)
--- 1574,1580 ----
                  else
                      bufptr = &pbuf;

!                 pval = OidReceiveFunctionCall(typreceive, bufptr, typioparam, -1);

                  /* Trouble if it didn't eat the whole buffer */
                  if (!isNull && pbuf.cursor != pbuf.len)
***************
*** 1596,1608 ****
--- 1591,1612 ----
                          (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                           errmsg("unsupported format code: %d",
                                  pformat)));
+                 pval = 0;        /* keep compiler quiet */
              }

              /* Restore message buffer contents */
              if (!isNull)
                  pbuf.data[plength] = csave;

+             params->params[paramno].value = pval;
              params->params[paramno].isnull = isNull;
+             /*
+              * We mark the params as CONST.  This has no effect if we already
+              * did planning, but if we didn't, it licenses the planner to
+              * substitute the parameters directly into the one-shot plan we
+              * will generate below.
+              */
+             params->params[paramno].pflags = PARAM_FLAG_CONST;
              params->params[paramno].ptype = ptype;

              paramno++;
***************
*** 1641,1659 ****

      /*
       * If we didn't plan the query before, do it now.  This allows the planner
!      * to make use of the concrete parameter values we now have.
       *
!      * This happens only for unnamed statements, and so switching into the
!      * statement context for planning is correct (see notes in
!      * exec_parse_message).
       */
      if (pstmt->plan_list == NIL && pstmt->query_list != NIL)
      {
!         MemoryContext oldContext = MemoryContextSwitchTo(pstmt->context);

!         pstmt->plan_list = pg_plan_queries(pstmt->query_list, params, true);
          MemoryContextSwitchTo(oldContext);
      }

      /*
       * Define portal and start execution.
--- 1645,1673 ----

      /*
       * If we didn't plan the query before, do it now.  This allows the planner
!      * to make use of the concrete parameter values we now have.  Because we
!      * use PARAM_FLAG_CONST, the plan is good only for this set of param
!      * values, and so we generate the plan in the portal's own memory context
!      * where it will be thrown away after use.  As in exec_parse_message,
!      * we make no attempt to recover planner temporary memory until the end
!      * of the operation.
       *
!      * XXX because the planner has a bad habit of scribbling on its input,
!      * we have to make a copy of the parse trees, just in case someone binds
!      * and executes an unnamed statement multiple times.  FIXME someday
       */
      if (pstmt->plan_list == NIL && pstmt->query_list != NIL)
      {
!         MemoryContext oldContext;

!         oldContext = MemoryContextSwitchTo(PortalGetHeapMemory(portal));
!         plan_list = pg_plan_queries(copyObject(pstmt->query_list),
!                                     params,
!                                     true);
          MemoryContextSwitchTo(oldContext);
      }
+     else
+         plan_list = pstmt->plan_list;

      /*
       * Define portal and start execution.
***************
*** 1664,1670 ****
                        bind_values_str.len ? pstrdup(bind_values_str.data) : NULL,
                        pstmt->commandTag,
                        pstmt->query_list,
!                       pstmt->plan_list,
                        pstmt->context);

      PortalStart(portal, params, InvalidSnapshot);
--- 1678,1684 ----
                        bind_values_str.len ? pstrdup(bind_values_str.data) : NULL,
                        pstmt->commandTag,
                        pstmt->query_list,
!                       plan_list,
                        pstmt->context);

      PortalStart(portal, params, InvalidSnapshot);
*** src/include/nodes/params.h.orig    Fri Apr 21 21:26:01 2006
--- src/include/nodes/params.h    Wed Sep  6 11:12:38 2006
***************
*** 26,40 ****
--- 26,47 ----
   *      Although parameter numbers are normally consecutive, we allow
   *      ptype == InvalidOid to signal an unused array entry.
   *
+  *      PARAM_FLAG_CONST signals the planner that it may treat this parameter
+  *      as a constant (i.e., generate a plan that works only for this value
+  *      of the parameter).
+  *
   *      Although the data structure is really an array, not a list, we keep
   *      the old typedef name to avoid unnecessary code changes.
   * ----------------
   */

+ #define PARAM_FLAG_CONST    0x0001            /* parameter is constant */
+
  typedef struct ParamExternData
  {
      Datum        value;            /* parameter value */
      bool        isnull;            /* is it NULL? */
+     uint16        pflags;            /* flag bits, see above */
      Oid            ptype;            /* parameter's datatype, or 0 */
  } ParamExternData;

*** src/pl/plpgsql/src/pl_exec.c.orig    Sun Aug 27 19:54:46 2006
--- src/pl/plpgsql/src/pl_exec.c    Wed Sep  6 11:22:46 2006
***************
*** 3958,3963 ****
--- 3958,3964 ----
              ParamExternData *prm = ¶mLI->params[i];
              PLpgSQL_datum *datum = estate->datums[expr->params[i]];

+             prm->pflags = 0;
              exec_eval_datum(estate, datum, expr->plan_argtypes[i],
                              &prm->ptype,
                              &prm->value, &prm->isnull);

pgsql-patches by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: XML syntax patch
Next
From: Andrew Dunstan
Date:
Subject: Re: [HACKERS] Coding style for emacs