On Fri, Jan 10, 2020 at 05:39:40PM +0100, Fabien COELHO wrote:
> So IMHO the current situation is fine, but the __variable name. So ISTM that
> the attached is the simplest and more reasonable option to fix this.
I'd rather hear more from others at this point. Peter's opinion, as
the main author
Michaël,
So I think that the current situation is a good thing at least for debug.
If you look at some of my messages on other threads, you would likely
notice that my mood of the day is to not design things which try to
outsmart a user's expectations :)
I'm not following you.
ISTM that
On Fri, Jan 10, 2020 at 08:52:17AM +0100, Fabien COELHO wrote:
> Compared to dealing with the level inside the call, the use of the level
> variable avoids a call-test-return cycle in this case, and the unlikely
> should help the compiler reorder instructions so that no actual branch is
> taken
Bonjour Michaël,
TBH, my recommendation would be to drop *all* of these likely()
and unlikely() calls. What evidence have you got that those are
meaningfully improving the quality of the generated code? And if
they're buried inside macros, they certainly aren't doing anything
useful in terms
On Thu, Jan 09, 2020 at 08:09:29PM -0500, Tom Lane wrote:
> TBH, my recommendation would be to drop *all* of these likely()
> and unlikely() calls. What evidence have you got that those are
> meaningfully improving the quality of the generated code? And if
> they're buried inside macros, they
Alvaro Herrera writes:
> On 2020-Jan-09, Fabien COELHO wrote:
>> -if (unlikely(__pg_log_level <= PG_LOG_DEBUG))
>> +if (pg_log_debug_level)
>> {
> Umm ... I find the original exceedingly ugly, but the new line is
> totally impenetrable.
So, I had not been paying any attention to
On Thu, Jan 09, 2020 at 09:27:42PM -0300, Alvaro Herrera wrote:
> On 2020-Jan-09, Fabien COELHO wrote:
>> -if (unlikely(__pg_log_level <= PG_LOG_DEBUG))
>> +if (pg_log_debug_level)
>> {
>
> Umm ... I find the original exceedingly ugly, but the new line is
> totally impenetrable.
On 2020-Jan-09, Fabien COELHO wrote:
> - if (unlikely(__pg_log_level <= PG_LOG_DEBUG))
> + if (pg_log_debug_level)
> {
Umm ... I find the original exceedingly ugly, but the new line is
totally impenetrable.
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL
On Thu, Jan 09, 2020 at 10:28:21AM +0100, Fabien COELHO wrote:
> Yep, I thought of it, but I was not very keen on having a malloc/free cycle
> just for one debug message. However under debug this is probably not an
> issue.
Consistency is more important here IMO, so applied.
> Your patch works
Bonjour Michaël,
I don't follow what you mean by that.
The first versions of the patch did not change syntax_error(), and the
version committed has switched to use PQExpBufferData there. I think
that we should just do the same for the debug logs executing the meta
commands. This way, we
On Wed, Jan 08, 2020 at 03:31:46PM +0100, Peter Eisentraut wrote:
> On 2020-01-08 15:12, Michael Paquier wrote:
>> while syntax_error() has been
>> changed in a more modular way.
>
> I don't follow what you mean by that.
The first versions of the patch did not change syntax_error(), and the
On 2020-01-08 15:12, Michael Paquier wrote:
On Wed, Jan 08, 2020 at 02:27:46PM +0100, Peter Eisentraut wrote:
Committed.
That was fast.
- if (debug)
+ if (unlikely(__pg_log_level <= PG_LOG_DEBUG))
{
I am surprised that you kept this one,
I'm not happy about it, but it seems OK for
On Wed, Jan 08, 2020 at 02:27:46PM +0100, Peter Eisentraut wrote:
> Committed.
That was fast.
- if (debug)
+ if (unlikely(__pg_log_level <= PG_LOG_DEBUG))
{
I am surprised that you kept this one, while syntax_error() has been
changed in a more modular way.
--
Michael
signature.asc
Patch v6 makes syntax error location code more compact and tests the case.
Committed.
Thanks!
--
Fabien.
On 2020-01-08 13:07, Fabien COELHO wrote:
Patch v5 attached tries to follow your above suggestions.
Patch v6 makes syntax error location code more compact and tests the case.
Committed.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support,
Patch v5 attached tries to follow your above suggestions.
Patch v6 makes syntax error location code more compact and tests the case.
--
Fabien.diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index a1e0663c8b..cfb5110608 100644
--- a/src/bin/pgbench/pgbench.c
+++
Bonjour Michaël,
I think that I would just remove the "debug" variable defined in
pgbench.c all together, and switch the messages for the duration and the
one in executeMetaCommand to use info-level logging..
Ok, done.
Thanks. The output then gets kind of inconsistent when using --debug:
On Mon, Jan 06, 2020 at 01:36:23PM +0100, Fabien COELHO wrote:
>> I think that I would just remove the "debug" variable defined in
>> pgbench.c all together, and switch the messages for the duration and the
>> one in executeMetaCommand to use info-level logging..
>
> Ok, done.
Thanks. The
Bonjour Michaël,
Without looking at the context I thought that argv[0] was the program name,
which is not the case here. I put it back everywhere, including the DEBUG
message.
The variable names in Command are confusing IMO...
Somehow, yes. Note that there is a logic, it will indeed be the
On Fri, Jan 03, 2020 at 01:01:18PM +0100, Fabien COELHO wrote:
> Without looking at the context I thought that argv[0] was the program name,
> which is not the case here. I put it back everywhere, including the DEBUG
> message.
The variable names in Command are confusing IMO...
> Ok. I
Hello Peter,
Compared to v1 I have also made a few changes to be more consistent when
using fatal/error/info.
The renaming of debug to debug_level seems unnecessary and unrelated.
Indeed. It was when I used "debug" as a shorthand for "pg_log_debug".
In runShellCommand(), you removed some
On 2020-01-01 22:55, Fabien COELHO wrote:
I'm trying to keep the column width under control, but if you like it
wider, here it is.
Compared to v1 I have also made a few changes to be more consistent when
using fatal/error/info.
The renaming of debug to debug_level seems unnecessary and
On Wed, Jan 01, 2020 at 10:19:52PM +0100, Fabien COELHO wrote:
> Bonjour Michaël, et excellente année 2020 !
Toi aussi! Bonne année.
>> Hmm. Wouldn't it make sense to output the log generated as
>> information from the test using pg_log_info() instead of using
>> fprintf(stderr) (the logs of
Hello Peter,
The patch seems pretty straightforward, but this
+/*
+ * Convenient shorcuts
+ */
+#define fatal pg_log_fatal
+#define error pg_log_error
+#define warning pg_log_warning
+#define info pg_log_info
+#define debug pg_log_debug
seems counterproductive. Let's just use the normal
Bonjour Michaël, et excellente année 2020 !
Hmm. Wouldn't it make sense to output the log generated as
information from the test using pg_log_info() instead of using
fprintf(stderr) (the logs of the initial data load, progress report)?
For the progress report, the reason I decided against
On December 31, 2019 8:10:13 PM GMT+09:00, Peter Eisentraut
wrote:
> seems counterproductive. Let's just use the normal function names.
+1.
--
Michael
On 2019-12-24 11:17, Fabien COELHO wrote:
As suggested in "cce64a51", this patch make pgbench use postgres logging
capabilities.
I tried to use fatal/error/warning/info/debug where appropriate.
Some printing to stderr remain for some pgbench specific output.
The patch seems pretty
On Tue, Dec 24, 2019 at 11:17:31AM +0100, Fabien COELHO wrote:
> Some printing to stderr remain for some pgbench specific output.
Hmm. Wouldn't it make sense to output the log generated as
information from the test using pg_log_info() instead of using
fprintf(stderr) (the logs of the initial
28 matches
Mail list logo
