Hello
On 2019-Dec-12, Andres Freund wrote:
> I see that you pushed this patch.
Yeah, a version of it -- significantly different from what Alexey
submitted last.
> I'm unfortunately unhappy with the
> approach taken. As previously said, handling a lot of this from exec_*
> is a mistake in my op
Hi,
I see that you pushed this patch. I'm unfortunately unhappy with the
approach taken. As previously said, handling a lot of this from exec_*
is a mistake in my opinion. Pretty much the same problem exists for
parametrized query execution from other contexts, e.g. for queries
executed inside pl
Alexey: I would appreciate it if you give this patch a spin. Let me
know if it does what you wanted it to do.
On 2019-Dec-10, Alvaro Herrera wrote:
> Here's a curious thing that happens with this patch. If you have
> log_duration set so that parameters are logged during the bind phase,
> and th
Here's a curious thing that happens with this patch. If you have
log_duration set so that parameters are logged during the bind phase,
and then an error occurs during the execution phase but you don't have
log_parameters_on_error set true, the second error will log the
parameters nonetheless ... b
On 2019-Dec-10, Alvaro Herrera wrote:
> On 2019-Dec-10, Alvaro Herrera wrote:
>
> > On 2019-Dec-09, Tom Lane wrote:
> >
> > > Some quick review of v19:
> >
> > Here's v20 with all these comments hopefully addressed.
>
> Grr, I had forgotten to git add the stringinfo.h -> pg_wchar.h changes,
>
On 2019-Dec-07, Tom Lane wrote:
> 0002:
Here's a version of this part with fixes for these comments. It applies
on top of the stringinfo_mb.c patch sent elsewhere in the thread.
(If we were to add a "log_parameters_on_error_max_length" GUC to decide
the length to log, we would get rid of the re
On 2019-Dec-10, Alvaro Herrera wrote:
> On 2019-Dec-09, Tom Lane wrote:
>
> > Some quick review of v19:
>
> Here's v20 with all these comments hopefully addressed.
Grr, I had forgotten to git add the stringinfo.h -> pg_wchar.h changes,
so the prototype isn't anywhere in v20. However, after loo
On 2019-Dec-09, Tom Lane wrote:
> Some quick review of v19:
Here's v20 with all these comments hopefully addressed.
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 62277fcd3f63ae68495300c98f77c7e4b4713614
Greg Stark writes:
> On Mon, 9 Dec 2019 at 15:17, Tom Lane wrote:
>> Meh ... people will inevitably complain that they needed to see the
>> whole value, and we'll end up having to add another configuration
>> variable. Let's not go there just yet.
> I haven't been following this whole thread bu
On Mon, 9 Dec 2019 at 15:17, Tom Lane wrote:
>
> Meh ... people will inevitably complain that they needed to see the
> whole value, and we'll end up having to add another configuration
> variable. Let's not go there just yet.
I haven't been following this whole thread but my initial reaction is
Alvaro Herrera writes:
> The stuff about truncating to N chars was of my own devising. If we
> don't want truncation in log_parameters_on_error either, we could do
> away with the stringinfo changes altogether. (I stand by my opinion
> that we should truncate, but if there are contrary votes I'm
Alvaro Herrera writes:
> 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().
Hm, yeah, so that's another reason we aren't going to be sharing this
code exactly with frontend. Given that, dr
On 2019-Dec-09, Tom Lane wrote:
> Alvaro Herrera writes:
> > Also:
> > * v18 and v19 now alwys do a "strlen(s)", i.e. they scan the whole input
> > string -- pointless when maxlen is given. We could avoid that for
> > very large input strings by doing strnlen(maxlen + MAX_MULTIBYTE_CHAR_LEN)
Hi,
On 2019-12-09 17:05:31 -0300, Alvaro Herrera wrote:
> * Thoughts from Andres, who was busily messing about with stringinfo.c
> on his patch series for programmatic out/read node functions?
I don't immediately see a need for mb functionality there. But to me it
seems pretty clear that we're
Alvaro Herrera writes:
> Also:
> * v18 and v19 now alwys do a "strlen(s)", i.e. they scan the whole input
> string -- pointless when maxlen is given. We could avoid that for
> very large input strings by doing strnlen(maxlen + MAX_MULTIBYTE_CHAR_LEN)
> so that we capture our input string pl
Alvaro Herrera writes:
> * Thoughts on changing the current usage of the ...Quoted() function in
> postgres.c and pl_exec.c so that they print N characters (64) instead
> of the current default of printing everything? I'm up for changing,
> but have got no +1s on that. I think bloating log
Alvaro Herrera writes:
> On 2019-Dec-09, Tom Lane wrote:
>> 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 m
Also:
* Thoughts from Andres, who was busily messing about with stringinfo.c
on his patch series for programmatic out/read node functions?
* Thoughts on changing the current usage of the ...Quoted() function in
postgres.c and pl_exec.c so that they print N characters (64) instead
of the cur
On 2019-Dec-09, Tom Lane wrote:
> Alvaro Herrera 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
Alvaro Herrera 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 want
On 2019-Dec-09, Tom Lane wrote:
> I wrote:
> > Alvaro Herrera writes:
> >> 1. change enough of the build system so that pg_encoding_mbcliplen is
> >> available. (Offhand I see no reason why we couldn't move the
> >> function from mbutils.c to wchar.c, but I haven't tried.)
>
> > I'd be in favor
I wrote:
> Alvaro Herrera writes:
>> 1. change enough of the build system so that pg_encoding_mbcliplen is
>> available. (Offhand I see no reason why we couldn't move the
>> function from mbutils.c to wchar.c, but I haven't tried.)
> I'd be in favor of this if it doesn't turn out to require nast
Alvaro Herrera writes:
> On 2019-Dec-07, Tom Lane wrote:
>> It is a very bad idea that this is truncating text without regard to
>> multibyte character boundaries.
> I see four possible ways forward, with nuances. In order of preference:
> 1. change enough of the build system so that pg_encodin
On 2019-Dec-09, Alvaro Herrera wrote:
> I see four possible ways forward, with nuances. In order of preference:
>
> 1. change enough of the build system so that pg_encoding_mbcliplen is
>available. (Offhand I see no reason why we couldn't move the
>function from mbutils.c to wchar.c, bu
On 2019-Dec-07, Tom Lane wrote:
> 0001:
> It is a very bad idea that this is truncating text without regard to
> multibyte character boundaries.
Hmm. So it turns out that we recently made stringinfo.c exported to
frontends in libpgcommon. Clipping strings at a given character size
means we nee
Alvaro Herrera writes:
> I added some tests to the pgbench suite in the attached. I also finally
> put it the business to limit the length of parameters reported.
> I'd probably drop testlibpq5.c, since it seems a bit pointless now ...
I took a look through the v17 patches.
0001:
The two heade
On 2019-Dec-06, Pavel Stehule wrote:
> pá 6. 12. 2019 v 18:57 odesílatel Alvaro Herrera
> napsal:
> > (It seems a bit weird that if I assign NULL to :two pgbench stores the
> > empty string, instead of the NULL that I get as in the above which is
> > what happens when the variable is not defined
I added some tests to the pgbench suite in the attached. I also finally
put it the business to limit the length of parameters reported.
I'd probably drop testlibpq5.c, since it seems a bit pointless now ...
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24
pá 6. 12. 2019 v 18:57 odesílatel Alvaro Herrera
napsal:
> On 2019-Dec-05, Tom Lane wrote:
>
> > Possibly a workable compromise is to emit the info as an error
> > context line, appending it to whatever context exists today.
> > The result might look like, say,
> >
> > ERROR: whatever
> > CONTEX
On 2019-Dec-05, Tom Lane wrote:
> Possibly a workable compromise is to emit the info as an error
> context line, appending it to whatever context exists today.
> The result might look like, say,
>
> ERROR: whatever
> CONTEXT: SQL function "foo"
> extended query with parameters x, y, ...
This is
Alvaro Herrera writes:
> If anybody would like to review it once more, now's the time. I'm
> aiming at getting it pushed tomorrow (including the length-limited
> appendStringInfoStringQuoted stuff).
Um, surely this bit is unacceptable?
+ /*
+* XXX this clobbers any other DETAIL li
On 2019-Dec-05, Alvaro Herrera wrote:
> > Makes me wonder though, if we ought to invent something similar to the
> > errdata fields we have for schema, table, etc...
>
> Hmm, perhaps we should do that. It avoids the problem altogether.
... then again, I'm not up for writing everything that woul
Hello
On 2019-Dec-05, Andres Freund wrote:
> On 2019-12-05 14:19:45 -0300, Alvaro Herrera wrote:
> > What this does is set an error callback, which adds the parameter values
> > in the DETAIL line. This is at odds with all existing error callbacks:
> > they only add stuff to the CONTEXT string.
Hi,
On 2019-12-05 14:19:45 -0300, Alvaro Herrera wrote:
> What this does is set an error callback, which adds the parameter values
> in the DETAIL line. This is at odds with all existing error callbacks:
> they only add stuff to the CONTEXT string. The implication is that
> if an error site has
I made a number of changes on the proposed patch and ended up with the
attached. (I have not included the business to limit number of
characters, yet).
What this does is set an error callback, which adds the parameter values
in the DETAIL line. This is at odds with all existing error callbacks:
On 2019-Dec-04, Alvaro Herrera wrote:
>
> > (Maybe do strnlen(maxlen), then count strnlen(1) starting at that point
> > -- so if that returns >=1, print the "..."?)
>
> So I found that I can make the code more reasonable with this simple
> coding,
With strndup.
--
Álvaro Herrera
> (Maybe do strnlen(maxlen), then count strnlen(1) starting at that point
> -- so if that returns >=1, print the "..."?)
So I found that I can make the code more reasonable with this simple
coding,
if (maxlen > 0)
{
s = pnstrdup(s, maxlen);
ellips
On 2019-Dec-03, Alvaro Herrera wrote:
> Now, I have to say that this doesn't make me terribly happy, because I
> would like the additional ability to limit the printed values to N
> bytes. This means the new function would have to have an additional
> argument to indicate the maximum length (pass
On 2019-Sep-20, Andres Freund wrote:
> > > > + appendStringInfoCharMacro(¶m_str, '\'');
> > > > + for (p = pstring; *p; p++)
> > > > + {
> > > > + if (*p == '\'') /* double single quotes */
> > > > + appe
Patch file name is not great, changing it.
On 30/11/2019 12:35, Alexey Bashtanov wrote:
I've implemented it using error contexts, please could you have a look
at the patch attached?
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 4ec13f3311..b3a0d27861 100644
--- a/doc/s
Hi,
I'm sorry for replying so late.
I don't think those really are contradictions. You can continue to have
an errdetail_params(), and but call it from the error context callback
set up in the portal code
...
Even leaving that aside, I'm *STRONGLY* against entangling elog.c with
query execut
On Thu, Nov 07, 2019 at 03:41:04PM -0800, Andres Freund wrote:
> The way you do it you need to do it in numerous places, and I'm pretty
> sure you're missing some already. E.g. this will not work to log
> parameters for parametrized statements generated on the server side,
> e.g. for foreign key qu
Hi,
On 2019-11-05 12:07:50 +, Alexey Bashtanov wrote:
> > What I'm suggesting is that PortalStart() would build a string
> > representation out of the parameter list (using
> > ParamExternData.textValue if set, calling the output function
> > otherwise), and stash that in the portal.
> >
> >
On 2019-Nov-07, Alvaro Herrera wrote:
> So, if some parameters are large (they can be up to 1 GB-1, remember)
> then we can bloat the log file severely. I think we need to place an
> upper limit on the strings that we're going to log -- as inspiration,
> callers of ExecBuildValueDescription uses
So, if some parameters are large (they can be up to 1 GB-1, remember)
then we can bloat the log file severely. I think we need to place an
upper limit on the strings that we're going to log -- as inspiration,
callers of ExecBuildValueDescription uses 64 chars per value maximum.
Something like that
Hi Anders and Alvaro,
I must have missed this conversation branch when sending in v011.
Sorry about that.
> I think it might be worthwhile to cross-reference
> log_min_error_statement, as log_parameters_on_error will only take
effect when the
> statement is logged due to log_min_error_statemen
Hello Alvaro,
I didn't like abusing testlibpq3.c for your new stuff, so I moved it off
to a new file testlibpq5.c. I cleaned up a few other cosmetics things
about this -- v10 attached.
Thanks for doing this.
I eventually noticed that this patch fails
to initialize each param's textValue to N
Hi,
On 2019-09-20 16:56:57 -0300, Alvaro Herrera wrote:
> > > +/*
> > > + * The top-level portal that the client is immediately working with:
> > > + * creating, binding, executing, or all at one using simple protocol
> > > + */
> > > +Portal current_top_portal = NULL;
> > > +
> >
> > This strike
Hi, thanks for looking.
On 2019-Sep-20, Andres Freund wrote:
> On 2019-09-18 17:58:53 -0300, Alvaro Herrera wrote:
> > + > xreflabel="log_parameters_on_error">
> > + log_parameters_on_error
> > (boolean)
> > +
> > + log_parameters_on_error configuration
> > parameter
> >
Hi,
On 2019-09-18 17:58:53 -0300, Alvaro Herrera wrote:
> --- a/doc/src/sgml/config.sgml
> +++ b/doc/src/sgml/config.sgml
> @@ -6414,6 +6414,23 @@ log_line_prefix = '%m [%p] %q%u@%d/%a '
>
>
>
> + xreflabel="log_parameters_on_error">
> + log_parameters_on_error (boolean)
Nice patch, thanks.
I didn't like abusing testlibpq3.c for your new stuff, so I moved it off
to a new file testlibpq5.c. I cleaned up a few other cosmetics things
about this -- v10 attached. I eventually noticed that this patch fails
to initialize each param's textValue to NULL, which probably e
Please find the rebased patch attached.
Tested like the following.
Provided you're in the postgres checkout and you've run make in
src/test/examples/ and connected to db=postgres:
CREATE SCHEMA testlibpq3;
SET search_path = testlibpq3;
CREATE TABLE test1_(i int4, t text, b bytea);
INSERT INTO
On 05/04/2019 12:23, Peter Eisentraut wrote:
On 2019-03-29 15:55, Alexey Bashtanov wrote:
ERROR: value "62812" is out of range for type smallint
STATEMENT: SELECT abalance FROM pgbench_accounts WHERE aid = $1;
(In this case the error message contains the parameter value, so it's
not a very pr
On 2019-03-29 15:55, Alexey Bashtanov wrote:
>> ERROR: value "62812" is out of range for type smallint
>> STATEMENT: SELECT abalance FROM pgbench_accounts WHERE aid = $1;
>>
>> (In this case the error message contains the parameter value, so it's
>> not a very practical case, but it should work,
Hello and sorry for weeks of silence.
Hello Anders and Peter,
Thanks for your messages.
Please see the new patch version attached.
In my testing, I couldn't get this patch to do anything. Could you
please share your testing steps?
Sure. Provided you're in the postgres checkout and you've ru
Hi Alexey,
On 3/14/19 12:38 PM, Peter Eisentraut wrote:
Meanwhile, I have committed a patch that refactors the ParamListInfo
initialization a bit, so you don't have to initialize hasTextValues in a
bunch of places unrelated to your core code. So please rebase your
patch on that.
It's been tw
On 2019-02-15 15:02, Alexey Bashtanov wrote:
> Hello Anders and Peter,
>
> Thanks for your messages.
> Please see the new patch version attached.
In my testing, I couldn't get this patch to do anything. Could you
please share your testing steps?
I did
postgres -D data --log-parameters-on-error
Hello Anders and Peter,
Thanks for your messages.
Please see the new patch version attached.
> Have you analyzed how invasive it'd be to delay that till we actually
> can safely start a [sub]transaction to do that logging? Probably too
> expensive, but it seems like something that ought to be an
Hi,
tiny scroll-through review.
On 2019-01-28 00:15:51 +, Alexey Bashtanov wrote:
> diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
> index b6f5822..997e6e8 100644
> --- a/doc/src/sgml/config.sgml
> +++ b/doc/src/sgml/config.sgml
> @@ -6274,6 +6274,23 @@ log_line_prefix = '%m
Hi,
On 2018-12-14 23:04:26 +, Alexey Bashtanov wrote:
> Unfortunately, when enabled, the feature comes with some memory and CPU
> overhead,
> as we cannot convert certain values to text when in aborted transaction.
Have you analyzed how invasive it'd be to delay that till we actually
can safe
This piece in your patch probably doesn't belong:
+ elog(WARNING, "params->hasTextValues=%d,
IsAbortedTransactionBlockState()=%d",
+ params->hasTextValues &&
IsAbortedTransactionBlockState());
--
Peter Eisentraut http://
On Mon, Jan 28, 2019 at 12:15:51AM +, Alexey Bashtanov wrote:
> I'm sorry for the delay, feel free to move it to next commitfest if
> needed.
Done.
--
Michael
signature.asc
Description: PGP signature
Hi Peter,
With your patch, with log_statement=all and log_parameters=on, you get
the same, but with log_statement=all and log_parameters=off you get
LOG: execute : SELECT abalance FROM pgbench_accounts WHERE aid
= $1;
DETAIL: parameters: $1 = UNKNOWN TYPE
Thanks for spotting this, I've fixed
There appears to be a problem with how this affects current logging
behavior.
I'm using
pgbench -M extended -S -T 10 bench
to test the extended protocol.
Unpatched, with log_statement=all, you get something like
LOG: execute : SELECT abalance FROM pgbench_accounts WHERE aid
= $1;
DETAIL:
please see attached
sorry, some unintended changes sneaked in, please see the corrected patch
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index f64402a..924a76c 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6270,6 +6270,23 @@ log_line_prefix = '%m
Hello Peter,
Unlike SQL, parameters may spend much more memory, so I'd have them
in portal memory context to make sure the memory is released earlier
rather than later.
Having them in the portal structure is different from having it in the
portal memory context. Although there is perhaps value
On 02/01/2019 23:53, Alexey Bashtanov wrote:
>> In fact, maybe don't use the Portal structure at all and just store the
>> saved textualized values inside postgres.c in a static variable.
>
> Unlike SQL, parameters may spend much more memory, so I'd have them
> in portal memory context to make sur
That sounds like a plausible approach. Have you done any performance
measurements?
No I haven't yet
In your patch, I would organize the changes to the portal API a bit
differently. Don't change the signature of CreatePortal().
okay
Get rid of PortalSetCurrentTop() and PortalClearCurrent
On 15/12/2018 00:04, Alexey Bashtanov wrote:
> I'd like to propose a patch to log bind parameter values not only when
> logging duration,
> but also on error (timeout in particular) or in whatever situation the
> statement normally gets logged.
> This mostly could be useful when the request origi
69 matches
Mail list logo
