Re: [PATCHES] 8.0.0beta3 duration logging patch
Ed L. wrote: > The attached patch forces queryless duration log statements to be turned off > in step with the log_statement directive. Without this patch, and with > log_statement = 'mod' and log_duration = true, there is no way to silence > SELECT query duration logging when quieting the logging of SELECT queries. > > Note this patch changes the semantics of log_duration from logging the > duration of "every completed statement" to "every completed statement that > satisfies log_statement directive". I argue this semantic change is > justified given 1) the docs themselves recommend turning log_statement > sufficiently up to be able to make this mapping, and 2) I can see it being > quite common that folks only want to log queries (and durations) that > change the database, while I fail to see the usefulness of queryless > durations (and I'm trying to scratch my own itch with a small effort). > It's possible someone else feels strongly about their queryless durations > for reasons I cannot imagine. If so, then another more conservative > approach may be in order. > > Note also this patch is independent of queries and durations logged due to > the log_min_duration_statement directive. If, for example, log_statement = > 'all', log_min_duration_statement = 1 (ms), and a SELECT query takes longer > than 1ms, it's duration will be logged twice, with the 2nd log entry > including the statement with the duration. Let me tell you how log_duration used to work. You would turn on log_pid, log_statement, and log_duration, and you would use pid to link together the query and the duration. That was kind of hard so we added log_min_duration_statement that when set to zero prints all statements and their durations, but it does it _after_ the query has run. Now that log_statement is not a boolean but rather has four possible values, it isn't as clear that you turn on log_statement and log_duration at the same time and have them synchronized. Your issue brings up that the boolean API doesn't really work well, and in fact highlights the fact that printing the duration as an independent capability really made no sense at all. Perhaps your approach is the proper solution --- to link them together. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
[PATCHES] 8.0.0beta3 duration logging patch
The attached patch forces queryless duration log statements to be turned off
in step with the log_statement directive. Without this patch, and with
log_statement = 'mod' and log_duration = true, there is no way to silence
SELECT query duration logging when quieting the logging of SELECT queries.
Note this patch changes the semantics of log_duration from logging the
duration of "every completed statement" to "every completed statement that
satisfies log_statement directive". I argue this semantic change is
justified given 1) the docs themselves recommend turning log_statement
sufficiently up to be able to make this mapping, and 2) I can see it being
quite common that folks only want to log queries (and durations) that
change the database, while I fail to see the usefulness of queryless
durations (and I'm trying to scratch my own itch with a small effort).
It's possible someone else feels strongly about their queryless durations
for reasons I cannot imagine. If so, then another more conservative
approach may be in order.
Note also this patch is independent of queries and durations logged due to
the log_min_duration_statement directive. If, for example, log_statement =
'all', log_min_duration_statement = 1 (ms), and a SELECT query takes longer
than 1ms, it's duration will be logged twice, with the 2nd log entry
including the statement with the duration.
Ed
Index: doc/src/sgml/runtime.sgml
===
RCS file: /projects/cvsroot/pgsql-server/doc/src/sgml/runtime.sgml,v
retrieving revision 1.284
diff -C1 -r1.284 runtime.sgml
*** doc/src/sgml/runtime.sgml 26 Sep 2004 22:51:49 - 1.284
--- doc/src/sgml/runtime.sgml 28 Sep 2004 02:19:16 -
***
*** 2305,2313
! Causes the duration of every completed statement to be logged.
! To use this option, it is recommended that you also enable
! log_statement and if not using syslog
! log the PID using log_line_prefix so that you
! can link the statement to the duration using the process
! ID. The default is off. Only superusers can turn off this
! option if it is enabled by the administrator.
--- 2305,2314
! Causes the duration of every completed statement which satisfies
! log_statement directive to be logged.
! When using this option, if you are not using syslog,
! it is recommended that you log the PID or session ID using
! log_line_prefix or log the session ID so that you can
! link the statement to the duration using the process ID or session
! ID. The default is off. Only superusers can turn off this option
! if it is enabled by the administrator.
Index: src/backend/tcop/postgres.c
===
RCS file: /projects/cvsroot/pgsql-server/src/backend/tcop/postgres.c,v
retrieving revision 1.433
diff -C1 -r1.433 postgres.c
*** src/backend/tcop/postgres.c 26 Sep 2004 00:26:25 - 1.433
--- src/backend/tcop/postgres.c 28 Sep 2004 02:19:19 -
***
*** 83,84
--- 83,87
+ /* flag noting if the statement satisfies log_statement directive */
+ bool loggable_statement;
+
/* GUC variable for maximum stack depth (measured in kilobytes) */
***
*** 465,469
--- 468,476
+ loggable_statement = false;
if (log_statement == LOGSTMT_ALL)
+ {
ereport(LOG,
(errmsg("statement: %s", query_string)));
+ loggable_statement = true;
+ }
***
*** 503,504
--- 510,512
(errmsg("statement: %s", query_string)));
+ loggable_statement = true;
break;
***
*** 514,515
--- 522,524
(errmsg("statement: %s", query_string)));
+ loggable_statement = true;
break;
***
*** 1005,1007
! if (save_log_duration)
ereport(LOG,
--- 1014,1023
! /*
! * If log_duration = true, don't log duration unless statement
! * statement also satifies log_statement directive. Otherwise,
! * the duration statements are devoid of context without their
! * query having been logged. Note the statement still may be
! * below due to log_min_duration_statement directive.
! */
! if (loggable_statement && save_log_duration)
ereport(LOG,
---(end of broadcast)---
TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Win32 Version numbering patch
"Dave Page" <[EMAIL PROTECTED]> writes: > I was hoping this would be in for beta 3, but alas - can someone > *please* commit the win32 version patch at: > http://candle.pha.pa.us/mhonarc/patches/msg0.html Personally I was holding off in hopes of seeing a cleaner solution. A patch that requires every executable-building Makefile to have a platform-specific wart isn't going to be very maintainable. You already missed pg_config, plus whichever of the contrib modules build executables... regards, tom lane ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] [pgsql-hackers-win32] VC++ psql build broken
I have fixed this issue with Dave's help. --- Dave Page wrote: > > > > -Original Message- > > From: Bruce Momjian [mailto:[EMAIL PROTECTED] > > Sent: 12 September 2004 14:59 > > To: Dave Page > > Cc: PgSQL Win32 developers; PostgreSQL Patches > > Subject: Re: [pgsql-hackers-win32] VC++ psql build broken > > > > > > How does your Win32 system rename prototype differ from what > > is in port.h? > > > > extern int pgrename(const char *from, const char *to); > > _CRTIMP int __cdecl rename(const char *, const char *); > > > > extern int pgunlink(const char *path); > > _CRTIMP int __cdecl _unlink(const char *); > > > Good question on wether we need to keep this working but it > > would be nice to keep it I guess for client-only installs > > that want to build from source using non-Mingw. > > Nice, but not essential. I certainly don't think it's worth spending > excessive amounts of energy on though. > > Regards, Dave > -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] [pgsql-hackers-win32] VC++ psql build broken
I have applied the attached patch. I changed psqlscan to only require flex when it needs to rebuild the flex output: psqlscan.c: psqlscan.l $(FLEX) -Cfe -opsqlscan.c psqlscan.l Because we normally ship the flex output in the tarball I didn't bother to document that flex is required for CVS builds. We already require flex for CVS builds so this isn't anything special. --- Dave Page wrote: > > > > -Original Message- > > From: Bruce Momjian [mailto:[EMAIL PROTECTED] > > Sent: 10 September 2004 17:41 > > To: [EMAIL PROTECTED] > > Cc: Dave Page; PgSQL Win32 developers > > Subject: Re: [pgsql-hackers-win32] VC++ psql build broken > > > > > > Now that I think of it, flex will not work because there is > > no *.mak rule to run it. You would have to run it manually, > > or write a *.mak rule for it. If someone does, please do > > bcc.make too. > > Patch attached. It works find under VC++, but the Borland mod is an > untested copy/paste. > > Unfortunately, psql still doesn't build giving the errors below. I > should point out that I have no interest in psql but am happy to keep > testing the build. I do wonder if there is any need to maintain these > makefiles at all now it builds under Mingw - it's not like psql is a > library like libpq that vc++ or bcc needs to link to (which I do need). > > Regards, Dave. > > cd ..\..\bin\psql > nmake /f win32.mak > > Microsoft (R) Program Maintenance Utility Version 6.00.8168.0 > Copyright (C) Microsoft Corp 1988-1998. All rights reserved. > > flex.exe -Cfe -opsqlscan.c psqlscan.l > echo #define PGBINDIR "" >"..\..\port\pg_config_paths.h" > echo #define PGSHAREDIR "" >>"..\..\port\pg_config_paths.h" > echo #define SYSCONFDIR "" >>"..\..\port\pg_config_paths.h" > echo #define INCLUDEDIR "" >>"..\..\port\pg_config_paths.h" > echo #define PKGINCLUDEDIR "" >>"..\..\port\pg_config_paths.h" > echo #define INCLUDEDIRSERVER "" > >>"..\..\port\pg_config_paths.h" > echo #define LIBDIR "" >>"..\..\port\pg_config_paths.h" > echo #define PKGLIBDIR "" >>"..\..\port\pg_config_paths.h" > echo #define LOCALEDIR "" >>"..\..\port\pg_config_paths.h" > cl.exe @C:\DOCUME~1\dpage\LOCALS~1\Temp\nma03280. > sprompt.c > cl.exe @C:\DOCUME~1\dpage\LOCALS~1\Temp\nmb03280. > getopt.c > cl.exe @C:\DOCUME~1\dpage\LOCALS~1\Temp\nmc03280. > getopt_long.c > cl.exe @C:\DOCUME~1\dpage\LOCALS~1\Temp\nmd03280. > path.c > cl.exe @C:\DOCUME~1\dpage\LOCALS~1\Temp\nme03280. > command.c > C:\PROGRA~1\MICROS~3\VC98\INCLUDE\io.h(176) : error C2375: 'pgrename' : > redefini > tion; different linkage > ..\..\include\port.h(167) : see declaration of 'pgrename' > C:\PROGRA~1\MICROS~3\VC98\INCLUDE\io.h(244) : error C2375: 'pgunlink' : > redefini > tion; different linkage > ..\..\include\port.h(168) : see declaration of 'pgunlink' > common.c > C:\PROGRA~1\MICROS~3\VC98\INCLUDE\io.h(176) : error C2375: 'pgrename' : > redefini > tion; different linkage > ..\..\include\port.h(167) : see declaration of 'pgrename' > C:\PROGRA~1\MICROS~3\VC98\INCLUDE\io.h(244) : error C2375: 'pgunlink' : > redefini > tion; different linkage > ..\..\include\port.h(168) : see declaration of 'pgunlink' > common.c(536) : warning C4018: '<' : signed/unsigned mismatch > help.c > input.c > stringutils.c > mainloop.c > mainloop.c(253) : warning C4018: '==' : signed/unsigned mismatch > copy.c > C:\PROGRA~1\MICROS~3\VC98\INCLUDE\io.h(176) : error C2375: 'pgrename' : > redefini > tion; different linkage > ..\..\include\port.h(167) : see declaration of 'pgrename' > C:\PROGRA~1\MICROS~3\VC98\INCLUDE\io.h(244) : error C2375: 'pgunlink' : > redefini > tion; different linkage > ..\..\include\port.h(168) : see declaration of 'pgunlink' > startup.c > C:\PROGRA~1\MICROS~3\VC98\INCLUDE\io.h(176) : error C2375: 'pgrename' : > redefini > tion; different linkage > ..\..\include\port.h(167) : see declaration of 'pgrename' > C:\PROGRA~1\MICROS~3\VC98\INCLUDE\io.h(244) : error C2375: 'pgunlink' : > redefini > tion; different linkage > ..\..\include\port.h(168) : see declaration of 'pgunlink' > prompt.c > C:\PROGRA~1\MICROS~3\VC98\INCLUDE\io.h(176) : error C2375: 'pgrename' : > redefini > tion; different linkage > ..\..\include\port.h(167) : see declaration of 'pgrename' > C:\PROGRA~1\MICROS~3\VC98\INCLUDE\io.h(244) : error C2375: 'pgunlink' : > redefini > tion; different linkage > ..\..\include\port.h(168) : see declaration of 'pgunlink' > variables.c > large_obj.c > print.c > print.c(1238) : warning C4090: 'function' : different 'const' qualifiers > print.c(1238) : warning C4022: 'free' : pointer mismatch for actual > parameter 1 > print.c(1239) : warning C4090: 'function' : different 'const' qualifiers > print.c(1239) : war
