On 08/27/2013 05:06 AM, David Rowley wrote:
> Heikki wrote that it might be useful to allow formatting in the
> log_line_prefix here
> http://www.postgresql.org/message-id/5187cadb.50...@vmware.com
>
> I thought I'd take a bash at implementing space padding part on
> log_line_prefix options.
>
> It's been a while since I worked with the PostgreSQL source, so please
> forgive me if my coding style is a bit off or the patch is not quite
> in the correct format.
>
> It's a little rough around the edges at the moment and likely the
> documentation needs more work, but I'm at the stage of wanting to know
> if this is even wanted or needed by people.
>
> If you apply this patch you can do things like have %10u %-10d in your
> log_line_prefix which will include spaces to give the option the
> number you specify as the minimum width. Left and right aligning is
> supported.
>
> Let this be the thread to collect consensus to whether this needed and
> useful.
I was just working on this last week! I didn't get a chance to post it
because I was still polishing it. I first took the same approach as you
but decided it reduced the clarity of the code so I re-did it a
different way. I'll attach it here along with yours to see which way
others like it.
Did you test performance? You should add your patch to the next commitfest.
--
Vik
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***************
*** 4046,4052 **** local0.* /var/log/postgresql
--- 4046,4062 ----
</tbody>
</tgroup>
</informaltable>
+ </para>
+
+ <para>
+ Between the <literal>%</> and the escape letter can be added a
+ minimum field width. If the value is shorter than the specified
+ width, it is left-padded with spaces. If the value is larger then
+ it takes the space it needs; it is not truncated. A negative width
+ will cause the value to be right-padded.
+ </para>
+ <para>
The <literal>%c</> escape prints a quasi-unique session identifier,
consisting of two 4-byte hexadecimal numbers (without leading zeros)
separated by a dot. The numbers are the process start time and the
*** a/src/backend/utils/error/elog.c
--- b/src/backend/utils/error/elog.c
***************
*** 2131,2139 **** log_line_prefix(StringInfo buf, ErrorData *edata)
--- 2131,2145 ----
/* has counter been reset in current process? */
static int log_my_pid = 0;
+ StringInfoData workbuf; /* we put the fields in here for later formatting */
+ int field_width;
+ bool left_justify;
+
int format_len;
int i;
+ initStringInfo(&workbuf);
+
/*
* This is one of the few places where we'd rather not inherit a static
* variable's value from the postmaster. But since we will, reset it when
***************
*** 2163,2171 **** log_line_prefix(StringInfo buf, ErrorData *edata)
}
/* go to char after '%' */
i++;
! if (i >= format_len)
break; /* format error - ignore it */
/* process the option */
switch (Log_line_prefix[i])
{
--- 2169,2198 ----
}
/* go to char after '%' */
i++;
!
! /* process the field width modifier, if present */
! left_justify = i < format_len && Log_line_prefix[i] == '-';
! if (left_justify)
! i++;
!
! field_width = 0;
! while (i < format_len && Log_line_prefix[i] >= '0' && Log_line_prefix[i] <= '9')
! {
! field_width = 10 * field_width + (Log_line_prefix[i] - '0');
! if (field_width < 0)
! break; /* integer out of range - consider it a format error */
! i++;
! }
!
! if (i >= format_len || field_width < 0 || (left_justify && field_width == 0))
break; /* format error - ignore it */
+ if (left_justify)
+ field_width = -field_width;
+
+ /* get the working buffer ready */
+ resetStringInfo(&workbuf);
+
/* process the option */
switch (Log_line_prefix[i])
{
***************
*** 2176,2182 **** log_line_prefix(StringInfo buf, ErrorData *edata)
if (appname == NULL || *appname == '\0')
appname = _("[unknown]");
! appendStringInfoString(buf, appname);
}
break;
case 'u':
--- 2203,2209 ----
if (appname == NULL || *appname == '\0')
appname = _("[unknown]");
! appendStringInfoString(&workbuf, appname);
}
break;
case 'u':
***************
*** 2186,2192 **** log_line_prefix(StringInfo buf, ErrorData *edata)
if (username == NULL || *username == '\0')
username = _("[unknown]");
! appendStringInfoString(buf, username);
}
break;
case 'd':
--- 2213,2219 ----
if (username == NULL || *username == '\0')
username = _("[unknown]");
! appendStringInfoString(&workbuf, username);
}
break;
case 'd':
***************
*** 2196,2216 **** log_line_prefix(StringInfo buf, ErrorData *edata)
if (dbname == NULL || *dbname == '\0')
dbname = _("[unknown]");
! appendStringInfoString(buf, dbname);
}
break;
case 'c':
! appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
break;
case 'p':
! appendStringInfo(buf, "%d", MyProcPid);
break;
case 'l':
! appendStringInfo(buf, "%ld", log_line_number);
break;
case 'm':
setup_formatted_log_time();
! appendStringInfoString(buf, formatted_log_time);
break;
case 't':
{
--- 2223,2243 ----
if (dbname == NULL || *dbname == '\0')
dbname = _("[unknown]");
! appendStringInfoString(&workbuf, dbname);
}
break;
case 'c':
! appendStringInfo(&workbuf, "%lx.%x", (long) (MyStartTime), MyProcPid);
break;
case 'p':
! appendStringInfo(&workbuf, "%d", MyProcPid);
break;
case 'l':
! appendStringInfo(&workbuf, "%ld", log_line_number);
break;
case 'm':
setup_formatted_log_time();
! appendStringInfoString(&workbuf, formatted_log_time);
break;
case 't':
{
***************
*** 2220,2232 **** log_line_prefix(StringInfo buf, ErrorData *edata)
pg_strftime(strfbuf, sizeof(strfbuf),
"%Y-%m-%d %H:%M:%S %Z",
pg_localtime(&stamp_time, log_timezone));
! appendStringInfoString(buf, strfbuf);
}
break;
case 's':
if (formatted_start_time[0] == '\0')
setup_formatted_start_time();
! appendStringInfoString(buf, formatted_start_time);
break;
case 'i':
if (MyProcPort)
--- 2247,2259 ----
pg_strftime(strfbuf, sizeof(strfbuf),
"%Y-%m-%d %H:%M:%S %Z",
pg_localtime(&stamp_time, log_timezone));
! appendStringInfoString(&workbuf, strfbuf);
}
break;
case 's':
if (formatted_start_time[0] == '\0')
setup_formatted_start_time();
! appendStringInfoString(&workbuf, formatted_start_time);
break;
case 'i':
if (MyProcPort)
***************
*** 2235,2256 **** log_line_prefix(StringInfo buf, ErrorData *edata)
int displen;
psdisp = get_ps_display(&displen);
! appendBinaryStringInfo(buf, psdisp, displen);
}
break;
case 'r':
if (MyProcPort && MyProcPort->remote_host)
{
! appendStringInfoString(buf, MyProcPort->remote_host);
if (MyProcPort->remote_port &&
MyProcPort->remote_port[0] != '\0')
! appendStringInfo(buf, "(%s)",
MyProcPort->remote_port);
}
break;
case 'h':
if (MyProcPort && MyProcPort->remote_host)
! appendStringInfoString(buf, MyProcPort->remote_host);
break;
case 'q':
/* in postmaster and friends, stop if %q is seen */
--- 2262,2283 ----
int displen;
psdisp = get_ps_display(&displen);
! appendBinaryStringInfo(&workbuf, psdisp, displen);
}
break;
case 'r':
if (MyProcPort && MyProcPort->remote_host)
{
! appendStringInfoString(&workbuf, MyProcPort->remote_host);
if (MyProcPort->remote_port &&
MyProcPort->remote_port[0] != '\0')
! appendStringInfo(&workbuf, "(%s)",
MyProcPort->remote_port);
}
break;
case 'h':
if (MyProcPort && MyProcPort->remote_host)
! appendStringInfoString(&workbuf, MyProcPort->remote_host);
break;
case 'q':
/* in postmaster and friends, stop if %q is seen */
***************
*** 2261,2283 **** log_line_prefix(StringInfo buf, ErrorData *edata)
case 'v':
/* keep VXID format in sync with lockfuncs.c */
if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
! appendStringInfo(buf, "%d/%u",
MyProc->backendId, MyProc->lxid);
break;
case 'x':
! appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
break;
case 'e':
! appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
break;
case '%':
! appendStringInfoChar(buf, '%');
break;
default:
/* format error - ignore it */
break;
}
}
}
/*
--- 2288,2317 ----
case 'v':
/* keep VXID format in sync with lockfuncs.c */
if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
! appendStringInfo(&workbuf, "%d/%u",
MyProc->backendId, MyProc->lxid);
break;
case 'x':
! appendStringInfo(&workbuf, "%u", GetTopTransactionIdIfAny());
break;
case 'e':
! appendStringInfoString(&workbuf, unpack_sql_state(edata->sqlerrcode));
break;
case '%':
! appendStringInfoChar(&workbuf, '%');
break;
default:
/* format error - ignore it */
break;
}
+
+ if (field_width == 0)
+ appendStringInfoString(buf, workbuf.data); /* fast path */
+ else
+ appendStringInfo(buf, "%*s", field_width, workbuf.data);
}
+
+ pfree(workbuf.data);
}
/*
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
