Re: log bind parameter values on error

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Alexey Bashtanov <bashtanov(at)imap(dot)cc>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: log bind parameter values on error
Date: 2019-12-09 19:49:08
Message-ID: 20191209194908.GA17784@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2019-Dec-09, Tom Lane wrote:

> Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
> > So rather than mess with stringinfo.c at all I could just create
> > stringinfo_server.c and put this function there, compiled only for
> > backend ...
>
> Good point: if we make a separate source file then we don't have
> to solve any of the code-movement issues till somebody wants this
> functionality in frontend. But we should expect that that day might
> come eventually, so I don't much like "stringinfo_server.c" as the
> file name. It'll look pretty silly once we start compiling it for
> frontend. Perhaps "appendquoted.c" or some such?

Okay, so here are two patches. I had already used the name
stringinfo_mb.c, so that's what's v19. (I'm fine with renaming it to
appendquoted.c but we might gain other such functions in the future.
Other opinions?)

The other patch (v18) puts the new function together with moving wchar
to pgcommon.

The API is different in each case: if we want it available in frontend,
we need to pass the encoding as a parameter rather than use
GetDatabaseEncoding().

This is pg_waldump with the first patch (src/utils/mb/stringinfo_mb.c):

$ nm --print-size /pgsql/install/master/bin/pg_waldump | grep -i stringinfo
000000000000a8f0 0000000000000060 T appendBinaryStringInfo
000000000000a980 0000000000000051 T appendBinaryStringInfoNT
000000000000a770 00000000000000d7 T appendStringInfo
000000000000a850 0000000000000050 T appendStringInfoChar
000000000000a8a0 000000000000004d T appendStringInfoSpaces
000000000000a950 0000000000000027 T appendStringInfoString
000000000000a630 0000000000000083 T appendStringInfoVA
000000000000a6c0 00000000000000af T enlargeStringInfo
000000000000a5e0 000000000000002b T initStringInfo
000000000000a5a0 0000000000000038 T makeStringInfo
000000000000a610 0000000000000015 T resetStringInfo

$ ls -l /pgsql/install/master/bin/pg_waldump
-rwxr-xr-x 1 alvherre alvherre 647576 dic 9 16:23 /pgsql/install/master/bin/pg_waldump*

This is with v18:

$ nm --print-size /pgsql/install/master/bin/pg_waldump | grep -i stringinfo
000000000000c8f0 0000000000000060 T appendBinaryStringInfo
000000000000c980 0000000000000051 T appendBinaryStringInfoNT
000000000000c770 00000000000000d7 T appendStringInfo
000000000000c850 0000000000000050 T appendStringInfoChar
000000000000c8a0 000000000000004d T appendStringInfoSpaces
000000000000c950 0000000000000027 T appendStringInfoString
000000000000c9e0 00000000000001b1 T appendStringInfoStringQuoted
000000000000c630 0000000000000083 T appendStringInfoVA
000000000000c6c0 00000000000000af T enlargeStringInfo
000000000000c5e0 000000000000002b T initStringInfo
000000000000c5a0 0000000000000038 T makeStringInfo
000000000000c610 0000000000000015 T resetStringInfo

$ ls -l /pgsql/install/master/bin/pg_waldump
-rwxr-xr-x 1 alvherre alvherre 704808 dic 9 16:29 /pgsql/install/master/bin/pg_waldump*

While the function itself is tiny (though it's the largest of all
stringinfo functions, hmm), it has led to a rather huge bloating of the
binary. That's because the new binary contains a number of functions
from wchar.c, like you said, such as

$ nm --print-size /pgsql/install/master/bin/pg_waldump | grep dsplen
000000000000d820 0000000000000026 t pg_ascii_dsplen
000000000000e200 0000000000000005 t pg_big5_dsplen
000000000000eb30 0000000000000030 T pg_encoding_dsplen
000000000000dac0 000000000000002e t pg_euccn_dsplen
000000000000d960 0000000000000036 t pg_eucjp_dsplen
000000000000d9b0 000000000000002e t pg_euckr_dsplen
000000000000dc10 000000000000002e t pg_euctw_dsplen
000000000000e230 0000000000000005 t pg_gb18030_dsplen
000000000000e210 0000000000000005 t pg_gbk_dsplen
000000000000dd10 0000000000000005 t pg_johab_dsplen
000000000000e170 0000000000000026 t pg_latin1_dsplen
000000000000e0f0 0000000000000034 t pg_mule_dsplen
000000000000e1c0 0000000000000036 t pg_sjis_dsplen
000000000000e220 0000000000000005 t pg_uhc_dsplen
000000000000e870 000000000000013e t pg_utf_dsplen

and some others.

All in all, it seems like v19 is the way to go.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment Content-Type Size
v18-0001-Add-appendStringInfoStringQuoted.patch text/x-diff 12.1 KB
v19-0001-Add-appendStringInfoStringQuoted.patch text/x-diff 8.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2019-12-09 19:51:21 Re: proposal: minscale, rtrim, btrim functions for numeric
Previous Message Tom Lane 2019-12-09 19:34:29 Re: hyrax versus isolationtester.c's hard-wired timeouts