Re: [COMMITTERS] pgsql: Adjust things so that the query_string of a cached plan and the - Mailing list pgsql-hackers

Hi,

Tom Lane wrote:
> Adjust things so that the query_string of a cached plan and the sourceText of
> a portal are never NULL, but reliably provide the source text of the query.
> It turns out that there was only one place that was really taking a short-cut,
> which was the 'EXECUTE' utility statement.  That doesn't seem like a
> sufficiently critical performance hotspot to justify not offering a guarantee
> of validity of the portal source text....

This commit added a variable 'query_string' to the function
ExecuteQuery() in src/backend/commands/prepare.c, but that function
already takes an argument named 'queryString'. What's the difference?
Which is which? Do we need both?

It looks like the second is the query string of the prepare statement,
where the string passed as an argument contains the EXECUTE command. I
propose renaming the variable (as in the attached patch) or at least
explaining it better in additional comments.

Sorry, if this is nitpicking. I just happened to stumbled over it and
thought I better tell you.

Regards

Markus

*** src/backend/commands/prepare.c    1f53747076d3cb8d83832179c2e8a0ee3d8f2d37
--- src/backend/commands/prepare.c    0b2ffacdca58b5a073fe6a57b3aa2c7d61d317a4
*************** ExecuteQuery(ExecuteStmt *stmt, const ch
*** 174,180 ****
      ParamListInfo paramLI = NULL;
      EState       *estate = NULL;
      Portal        portal;
!     char       *query_string;

      /* Look it up in the hash table */
      entry = FetchPreparedStatement(stmt->name, true);
--- 174,180 ----
      ParamListInfo paramLI = NULL;
      EState       *estate = NULL;
      Portal        portal;
!     char       *prepared_qs;

      /* Look it up in the hash table */
      entry = FetchPreparedStatement(stmt->name, true);
*************** ExecuteQuery(ExecuteStmt *stmt, const ch
*** 205,212 ****
      portal->visible = false;

      /* Copy the plan's saved query string into the portal's memory */
!     query_string = MemoryContextStrdup(PortalGetHeapMemory(portal),
!                                        entry->plansource->query_string);

      /*
       * For CREATE TABLE / AS EXECUTE, we must make a copy of the stored query
--- 205,212 ----
      portal->visible = false;

      /* Copy the plan's saved query string into the portal's memory */
!     prepared_qs = MemoryContextStrdup(PortalGetHeapMemory(portal),
!                                       entry->plansource->query_string);

      /*
       * For CREATE TABLE / AS EXECUTE, we must make a copy of the stored query
*************** ExecuteQuery(ExecuteStmt *stmt, const ch
*** 256,262 ****

      PortalDefineQuery(portal,
                        NULL,
!                       query_string,
                        entry->plansource->commandTag,
                        plan_list,
                        cplan);
--- 256,262 ----

      PortalDefineQuery(portal,
                        NULL,
!                       prepared_qs,
                        entry->plansource->commandTag,
                        plan_list,
                        cplan);