Re: [HACKERS] log_statement output for protocol - Mailing list pgsql-patches

From Bruce Momjian
Subject Re: [HACKERS] log_statement output for protocol
Date
Msg-id 200608292010.k7TKAC204184@momjian.us
Whole thread Raw
In response to Re: [HACKERS] log_statement output for protocol  ("Guillaume Smet" <guillaume.smet@gmail.com>)
Responses Re: [HACKERS] log_statement output for protocol  ("Guillaume Smet" <guillaume.smet@gmail.com>)
List pgsql-patches
Guillaume Smet wrote:
> On 8/29/06, Bruce Momjian <bruce@momjian.us> wrote:
> >         DETAIL:  prepare: SELECT $1;  bind: $1 = 'a''b'
>
> I attached a trivial patch to add a dash between the prepare part and
> the bind part. People usually don't finish their queries with a semi
> colon so it's more readable with a separator.
> DETAIL:  prepare: SELECT $1  bind: $1 = 'a''b'
> becomes
> DETAIL:  prepare: SELECT $1 - bind: $1 = 'a''b'

Good point.  I thought it was clear enough, but obviously not.  I had a
similar case with bind, and used a comma to separate them:

        LOG:  statement: prepare sel1, SELECT $1;
        LOG:  statement: bind sel1, $1 = 'a''b'

I am concerned a dash isn't clear enough, and a semicolon is confusing.
Using a comma the new output is:

    LOG:  duration: 0.023 ms  execute sel1
    DETAIL:  prepare: SELECT $1;,  bind: $1 = 'a''b'

or with no semicolon:

    LOG:  duration: 0.023 ms  execute sel1
    DETAIL:  prepare: SELECT $1,  bind: $1 = 'a''b'

Is that OK?  Patch attached and committed.  I also fixed the null bind
parameter bug.  It now displays $1 = NULL (no quotes used).  Other
suggestions?

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: src/backend/tcop/postgres.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/tcop/postgres.c,v
retrieving revision 1.501
diff -c -c -r1.501 postgres.c
*** src/backend/tcop/postgres.c    29 Aug 2006 02:32:41 -0000    1.501
--- src/backend/tcop/postgres.c    29 Aug 2006 19:54:08 -0000
***************
*** 1539,1555 ****
                                           -1);

                  /* Save the parameter values */
!                 appendStringInfo(&bind_values_str, "%s$%d = '",
                                   bind_values_str.len ? ", " : "",
                                   paramno + 1);
!                 for (p = pstring; *p; p++)
                  {
!                     if (*p == '\'')    /* double single quotes */
                          appendStringInfoChar(&bind_values_str, *p);
!                     appendStringInfoChar(&bind_values_str, *p);
                  }
!                 appendStringInfoChar(&bind_values_str, '\'');
!
                  /* Free result of encoding conversion, if any */
                  if (pstring && pstring != pbuf.data)
                      pfree(pstring);
--- 1539,1561 ----
                                           -1);

                  /* Save the parameter values */
!                 appendStringInfo(&bind_values_str, "%s$%d = ",
                                   bind_values_str.len ? ", " : "",
                                   paramno + 1);
!                 if (pstring)
                  {
!                     appendStringInfoChar(&bind_values_str, '\'');
!                     for (p = pstring; *p; p++)
!                     {
!                         if (*p == '\'')    /* double single quotes */
!                             appendStringInfoChar(&bind_values_str, *p);
                          appendStringInfoChar(&bind_values_str, *p);
!                     }
!                     appendStringInfoChar(&bind_values_str, '\'');
                  }
!                 else
!                     appendStringInfo(&bind_values_str, "NULL");
!
                  /* Free result of encoding conversion, if any */
                  if (pstring && pstring != pbuf.data)
                      pfree(pstring);
***************
*** 1782,1788 ****
                          *portal_name ? portal_name : ""),
                          errdetail("prepare: %s%s%s", sourceText,
                          /* optionally print bind parameters */
!                         bindText ? "  bind: " : "",
                          bindText ? bindText : "")));

      BeginCommand(portal->commandTag, dest);
--- 1788,1794 ----
                          *portal_name ? portal_name : ""),
                          errdetail("prepare: %s%s%s", sourceText,
                          /* optionally print bind parameters */
!                         bindText ? ",  bind: " : "",
                          bindText ? bindText : "")));

      BeginCommand(portal->commandTag, dest);
***************
*** 1896,1902 ****
                                  *portal_name ? portal_name : ""),
                                  errdetail("prepare: %s%s%s", sourceText,
                                  /* optionally print bind parameters */
!                                 bindText ? "  bind: " : "",
                                  bindText ? bindText : "")));
          }
      }
--- 1902,1908 ----
                                  *portal_name ? portal_name : ""),
                                  errdetail("prepare: %s%s%s", sourceText,
                                  /* optionally print bind parameters */
!                                 bindText ? ",  bind: " : "",
                                  bindText ? bindText : "")));
          }
      }

pgsql-patches by date:

Previous
From: Chris Browne
Date:
Subject: Changes to epgc test
Next
From: Bruce Momjian
Date:
Subject: Re: [HACKERS] Interval aggregate regression failure