Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL
Tom Lane wrote: So the postmaster-log message may be the best we can do ... but I don't think we should drop the connection. Here's a patch to do that; it appears to work as intended on my Linux system. -O Index: src/backend/libpq/pqcomm.c === RCS file: /projects/cvsroot/pgsql/src/backend/libpq/pqcomm.c,v retrieving revision 1.178 diff -c -r1.178 pqcomm.c *** src/backend/libpq/pqcomm.c 30 Jul 2005 20:28:20 - 1.178 --- src/backend/libpq/pqcomm.c 12 Sep 2005 00:58:15 - *** *** 595,612 return STATUS_ERROR; } ! /* Set default keepalive parameters. This should also catch !* misconfigurations (non-zero values when socket options aren't !* supported) */ ! if (pq_setkeepalivesidle(tcp_keepalives_idle, port) != STATUS_OK) ! return STATUS_ERROR; ! ! if (pq_setkeepalivesinterval(tcp_keepalives_interval, port) != STATUS_OK) ! return STATUS_ERROR; ! ! if (pq_setkeepalivescount(tcp_keepalives_count, port) != STATUS_OK) ! return STATUS_ERROR; } return STATUS_OK; --- 595,607 return STATUS_ERROR; } ! /* Set default keepalive parameters. We ignore misconfigurations !* that cause errors -- they will be logged, but won't kill the !* connection. */ ! pq_setkeepalivesidle(tcp_keepalives_idle, port); ! pq_setkeepalivesinterval(tcp_keepalives_interval, port); ! pq_setkeepalivescount(tcp_keepalives_count, port); } return STATUS_OK; ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL
Merlin Moncure wrote: Here's a patch that adds four new GUCs: tcp_keepalives (defaults to on, controls SO_KEEPALIVE) tcp_keepalives_idle (controls TCP_KEEPIDLE) tcp_keepalives_interval (controls TCP_KEEPINTVL) tcp_keepalives_count (controls TCP_KEEPCNT) I just tested this on my windows XP machine running rc1. A default configuration reports zeros for the tcp values in a 'show all'. That usually means that there is no OS-level support for the TCP_* values. Does Windows actually provide those #defines? More significantly, if you change a tcp parameter from the default, the server rejects connections without a relevant error message :(. Could you clarify what you mean by rejects? Does it accept them and then close the connection, or does it fail to even accept the TCP connection? If the connection gets accepted, I'd expect *something* in the postmaster logs -- can you check? I did some research and the only way to control these parameters is to adjust the system registry plus a reboot. (somebody correct me here). If that is the case IMO it makes the most sense to have the server fail to start if the default parameters are changed. The problem is that the GUC only makes sense when you have an actual TCP connection present, so it is only set while establishing a new connection. If setting the value fails (e.g. the OS rejects the value), then it's just like any other GUC setup failure during backend startup, which by the sounds of it makes the whole connection fail. I don't know if we should ignore failures to set a GUC and continue anyway .. that sounds dangerous. I'd expect unix-domain-socket connections to continue to work fine in the face of a misconfiguration currently. We could check the non-zero configuration on an OS that has no support case at postmaster boot time, I suppose, but that doesn't help with the case where there is support but the OS rejects the particular values you're trying to set. Even better would be a stronger test to make sure o/s supports this feature. Well, the code assumes that if the TCP_* constants are present then they can be used. It seems a bit stupid if Windows defines them but doesn't support them at all. -O ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL
Tom Lane wrote: Oliver Jowett [EMAIL PROTECTED] writes: Merlin Moncure wrote: Even better would be a stronger test to make sure o/s supports this feature. Well, the code assumes that if the TCP_* constants are present then they can be used. It seems a bit stupid if Windows defines them but doesn't support them at all. In short, if you were assuming that then you'd better fix the code. Sorry, to clarify: If the TCP_* constants are provided, *and* you configure the backend to try to use non-default values, then the code will try the appropriate setsockopt(). If that fails, then the connection gets dropped. If you don't change the defaults, it doesn't use setsockopt() at all. The assumption I'm making is that if the TCP_* values are present at compile time, then you can make a setsockopt() call and get a sane error code back if there's no support -- rather than a segfault, or having the OS spontaneously do weird things to the connection, or anything like that. Is that a reasonable thing to assume? ... There doesn't seem any clean way to test if a particular set of values specified at runtime is going to work or not -- the only way is to try. I suppose we could set up a dummy TCP connection on postmaster start and try that, but.. We could potentially do better in the no TCP_* support case, detecting it on postmaster startup (move the check for NULL port down into the pqcomm code, and complain about non-zero values even in that case if there's no support); but that doesn't help the other cases. If I specify out-of-range values on my Linux box, I get this: [EMAIL PROTECTED] ~ $ pg/8.1-beta1/bin/psql -h localhost template1 psql: server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. and in the logs: LOG: setsockopt(TCP_KEEPIDLE) failed: Invalid argument I'd expect to see something similar in the TCP_* present but no real support case. -O ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL
Tom Lane wrote: Oliver Jowett [EMAIL PROTECTED] writes: The assumption I'm making is that if the TCP_* values are present at compile time, then you can make a setsockopt() call and get a sane error code back if there's no support -- rather than a segfault, or having the OS spontaneously do weird things to the connection, or anything like that. Is that a reasonable thing to assume? Well, on a sane OS it's reasonable. I dunno about Windows ;-) One question to ask is whether we should treat the setsockopt failure as fatal or not. It seems to me that aborting the connection could reasonably be called an overreaction to a bad parameter setting; couldn't we just set the GUC variable to zero and keep going? There's no real reason why not; currently the code looks like this: /* Set default keepalive parameters. This should also catch * misconfigurations (non-zero values when socket options aren't * supported) */ if (pq_setkeepalivesidle(tcp_keepalives_idle, port) != STATUS_OK) return STATUS_ERROR; if (pq_setkeepalivesinterval(tcp_keepalives_interval, port) != STATUS_OK) return STATUS_ERROR; if (pq_setkeepalivescount(tcp_keepalives_count, port) != STATUS_OK) return STATUS_ERROR; We could just log (already done inside pq_*, IIRC) and continue, instead of erroring out. It's just the way it is because I personally prefer misconfigurations to break loudly, so you have to fix them ;-) -O ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL
Tom Lane wrote: I'm not sure if we can issue a notice that will be seen on the client side at this point in the startup cycle. I seem to recall the protocol document advising against sending NOTICEs during the authentication cycle. As currently written the setsockopt() calls are done very early, well before even ProcessStartupPacket() has run -- so we can't really send anything at all to the client because we don't even know which protocol to use. Delaying the setsockopt() introduces the danger that you could lose the network at just the wrong time and have a backend sitting around for an extended period waiting on a startup packet (but not holding locks, admittedly). -O ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL
Bruce Momjian wrote: Is this the functionality we agreed we wanted? I think it covers Tom's comments on the first version: - a GUC to turn off SO_KEEPALIVE isn't particularly useful - don't do redundant get/setsockopt calls on backend startup -O ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL
Bruce Momjian wrote: Is this patch being worked on? Here's an updated version. It compiles and appears to work as expected under Linux (supports TCP_KEEPIDLE etc) and Solaris 9 (no support). Main changes: - removed the tcp_keepalives GUC, SO_KEEPALIVE is now always on (as in current CVS) - {get,set}sockopt calls are only done when absolutely necessary (no extra syscalls during backend startup in a default configuration). I still haven't had a chance to glue in support for the TCP_KEEPALIVE (Solaris-style) option, but that should be fairly painless to add later. -O ? postgresql-8.1devel.tar.gz Index: doc/src/sgml/runtime.sgml === RCS file: /projects/cvsroot/pgsql/doc/src/sgml/runtime.sgml,v retrieving revision 1.335 diff -u -c -r1.335 runtime.sgml *** doc/src/sgml/runtime.sgml 2 Jul 2005 19:16:36 - 1.335 --- doc/src/sgml/runtime.sgml 4 Jul 2005 10:41:33 - *** *** 894,899 --- 894,946 /listitem /varlistentry + varlistentry id=guc-tcp-keepalives-idle xreflabel=tcp_keepalives_idle + termvarnametcp_keepalives_idle/varname (typeinteger/type)/term + indexterm +primaryvarnametcp_keepalives_idle/ configuration parameter/primary + /indexterm + listitem +para + On systems that support the TCP_KEEPIDLE socket option, specifies the + number of seconds between sending keepalives on an otherwise idle + connection. A value of 0 uses the system default. If TCP_KEEPIDLE is + not supported, this parameter must be 0. This option is ignored for + connections made via a Unix-domain socket. +/para + /listitem + /varlistentry + + varlistentry id=guc-tcp-keepalives-interval xreflabel=tcp_keepalives_interval + termvarnametcp_keepalives_interval/varname (typeinteger/type)/term + indexterm +primaryvarnametcp_keepalives_interval/ configuration parameter/primary + /indexterm + listitem +para + On systems that support the TCP_KEEPINTVL socket option, specifies how + long, in seconds, to wait for a response to a keepalive before + retransmitting. A value of 0 uses the system default. If TCP_KEEPINTVL + is not supported, this parameter must be 0. This option is ignored + for connections made via a Unix-domain socket. +/para + /listitem + /varlistentry + + varlistentry id=guc-tcp-keepalives-count xreflabel=tcp_keepalives_count + termvarnametcp_keepalives_count/varname (typeinteger/type)/term + indexterm +primaryvarnametcp_keepalives_count/ configuration parameter/primary + /indexterm + listitem +para + On systems that support the TCP_KEEPCNT socket option, specifies how + many keepalives may be lost before the connection is considered dead. + A value of 0 uses the system default. If TCP_KEEPINTVL is not + supported, this parameter must be 0. +/para + /listitem + /varlistentry + /variablelist /sect3 sect3 id=runtime-config-connection-security Index: src/backend/libpq/pqcomm.c === RCS file: /projects/cvsroot/pgsql/src/backend/libpq/pqcomm.c,v retrieving revision 1.176 diff -u -c -r1.176 pqcomm.c *** src/backend/libpq/pqcomm.c 22 Feb 2005 04:35:57 - 1.176 --- src/backend/libpq/pqcomm.c 4 Jul 2005 10:41:33 - *** *** 87,93 #include libpq/libpq.h #include miscadmin.h #include storage/ipc.h ! /* * Configuration options --- 87,93 #include libpq/libpq.h #include miscadmin.h #include storage/ipc.h ! #include utils/guc.h /* * Configuration options *** *** 594,599 --- 594,612 elog(LOG, setsockopt(SO_KEEPALIVE) failed: %m); return STATUS_ERROR; } + + /* Set default keepalive parameters. This should also catch +* misconfigurations (non-zero values when socket options aren't +* supported) +*/ + if (pq_setkeepalivesidle(tcp_keepalives_idle, port) != STATUS_OK) + return STATUS_ERROR; + + if (pq_setkeepalivesinterval(tcp_keepalives_interval, port) != STATUS_OK) + return STATUS_ERROR; + + if (pq_setkeepalivescount(tcp_keepalives_count, port) != STATUS_OK) + return STATUS_ERROR; } return STATUS_OK; *** *** 1158,1160 --- 1171,1369 /* in non-error case, copy.c will have emitted the terminator line */ DoingCopyOut = false; } + + int + pq_getkeepalivesidle(Port *port) + { + #ifdef TCP_KEEPIDLE + if (IS_AF_UNIX(port-laddr.addr.ss_family))
Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL
Tom Lane wrote: Oliver Jowett [EMAIL PROTECTED] writes: Here's a patch that adds four new GUCs: tcp_keepalives (defaults to on, controls SO_KEEPALIVE) tcp_keepalives_idle (controls TCP_KEEPIDLE) tcp_keepalives_interval (controls TCP_KEEPINTVL) tcp_keepalives_count (controls TCP_KEEPCNT) Do you think the system defaults are really going to be port-specific? I don't understand what you mean. TCP_* override the system defaults on a per-connection basis, if that's what you mean. I'm slightly annoyed by the number of syscalls this adds to every connection startup ... getting rid of redundant getsockopt calls would help. Getting rid of redundant setsockopt calls (ie, a call to establish the value that we already know is in force) would help more. I originally did this but went in favor of simpler code. Are the extra syscalls an issue? I didn't think they were that expensive.. Alternatively, we could lose the frammish that PostgreSQL can tell you what the system defaults are: 0 in the GUC just means do nothing, not find out what the current setting is on the off chance that the user might want to know. I didn't do this as it meant that using SET tcp_whatever = 0 does *not* reset the setting to what you'd get with a value of 0 in postgresql.conf, which seemed confusing to me. This could all get simpler if we didn't allow per-connection changing of that GUC (i.e. set it once at startup and forget about it), which probably isn't unreasonable. I wonder if those socket options get inherited from the listening socket? Will check. -O ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL
Oliver Jowett wrote: Here's a patch that adds four new GUCs: I haven't had a chance to check it builds on other systems or to test that this handles actual network failures nicely yet. The patch builds OK on Solaris 9, which doesn't have the required socket options. Solaris (and some other OSes, apparently) supports TCP_KEEPALIVE instead of TCP_KEEPIDLE/TCP_KEEPCNT/TCP_KEEPINTVL. I will look at adding support for that next. -O ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
[PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL (was Re: [HACKERS] Feature freeze date for 8.1)
Tom Lane wrote: Oliver Jowett [EMAIL PROTECTED] writes: Tom Lane wrote: I'm not convinced that Postgres ought to provide a way to second-guess the TCP stack ... Would you be ok with a patch that allowed configuration of the TCP_KEEPCNT / TCP_KEEPIDLE / TCP_KEEPINTVL socket options on backend sockets? [ shrug... ] As long as it doesn't fail to build on platforms that don't offer those options, I couldn't complain too hard. But do we really need all that? Here's a patch that adds four new GUCs: tcp_keepalives (defaults to on, controls SO_KEEPALIVE) tcp_keepalives_idle (controls TCP_KEEPIDLE) tcp_keepalives_interval (controls TCP_KEEPINTVL) tcp_keepalives_count (controls TCP_KEEPCNT) They're ignored for AF_UNIX connections and when running standalone. tcp_keepalives_* treat 0 as use system default. If the underlying OS doesn't provide the TCP_KEEP* socket options, the GUCs are present but reject any value other than 0. SHOW reflects the currently-in-use value or 0 if not applicable or not known. i.e. if you set it to 0 and you have the socket options available, SHOW will show the result of getsockopt() which is non-zero. A default install on my Linux system produces: template1=# show all; name | setting +-- [...] tcp_keepalives | on tcp_keepalives_count | 9 tcp_keepalives_idle| 7200 tcp_keepalives_interval| 75 [...] I haven't had a chance to check it builds on other systems or to test that this handles actual network failures nicely yet. -O Index: doc/src/sgml/runtime.sgml === RCS file: /projects/cvsroot/pgsql/doc/src/sgml/runtime.sgml,v retrieving revision 1.315 diff -c -r1.315 runtime.sgml *** doc/src/sgml/runtime.sgml 23 Apr 2005 03:27:40 - 1.315 --- doc/src/sgml/runtime.sgml 3 May 2005 01:44:02 - *** *** 889,894 --- 889,961 /listitem /varlistentry + varlistentry id=guc-tcp-keepalives xreflabel=tcp_keepalives + termvarnametcp_keepalives/varname (typeboolean/type)/term + indexterm +primaryvarnametcp_keepalives/ configuration parameter/primary + /indexterm + listitem +para + Controls the use of TCP keepalives on client connections. When enabled, + idle connections will be periodically probed to check that the client + is still present. If sufficient probes are lost, the connection will + be broken. This option is ignored for connections made via a + Unix-domain socket. +/para + /listitem + /varlistentry + + varlistentry id=guc-tcp-keepalives-idle xreflabel=tcp_keepalives_idle + termvarnametcp_keepalives_idle/varname (typeinteger/type)/term + indexterm +primaryvarnametcp_keepalives_idle/ configuration parameter/primary + /indexterm + listitem +para + On systems that support the TCP_KEEPIDLE socket option, specifies the + number of seconds between sending keepalives on an otherwise idle + connection. A value of 0 uses the system default. If TCP_KEEPIDLE is + not supported, this parameter must be 0. This option is ignored for + connections made via a Unix-domain socket, and will have no effect + unless the varnametcp_keepalives/varname option is enabled. +/para + /listitem + /varlistentry + + varlistentry id=guc-tcp-keepalives-interval xreflabel=tcp_keepalives_interval + termvarnametcp_keepalives_interval/varname (typeinteger/type)/term + indexterm +primaryvarnametcp_keepalives_interval/ configuration parameter/primary + /indexterm + listitem +para + On systems that support the TCP_KEEPINTVL socket option, specifies how + long, in seconds, to wait for a response to a keepalive before + retransmitting. A value of 0 uses the system default. If TCP_KEEPINTVL + is not supported, this parameter must be 0. This option is ignored + for connections made via a Unix-domain socket, and will have no effect + unless the varnametcp_keepalives/varname option is enabled. +/para + /listitem + /varlistentry + + varlistentry id=guc-tcp-keepalives-count xreflabel=tcp_keepalives_count + termvarnametcp_keepalives_count/varname (typeinteger/type)/term + indexterm +primaryvarnametcp_keepalives_count/ configuration parameter/primary + /indexterm + listitem +para + On systems that support the TCP_KEEPCNT socket option, specifies how + many keepalives may be lost before the connection is considered dead. + A value of 0 uses the system default. If TCP_KEEPINTVL is not + supported
Re: [PATCHES] Implementing RESET CONNECTION ...
Hans-Jürgen Schönig wrote: If the JDBC driver prefers different behaviour (maybe for prepared statements) we should discuss further options for RESET. Now there is: RESET CONNECTION (cleaning entire connection), RESET ALL (cleaning GUCS only) and RESET some_guc. Maybe we want RESET LISTENER, RESET PREPARED, RESET CURSORS. Personally I think this is not a good idea. It doesn't help, either, if user code can still issue RESET CONNECTION. (the scenario is user code, not the driver itself, originating the RESET..) -O ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] Implementing RESET CONNECTION ...
(cc'ing -hackers) Karel Zak wrote: I think command status is common and nice feedback for client. I think it's more simple change something in JDBC than change protocol that is shared between more tools. There is a bit of a queue of changes that would be nice to have but require a protocol version change. If we're going to change the protocol for any of those we might as well handle RESET CONNECTION cleanly too. We need some common way how detect on client what's happen on server -- a way that doesn't mean change protocol always when we add some feature/command to backend. The command status is possible use for this. Command status only works if commands are directly executed. If you can execute the command indirectly, e.g. via a PL, then you'll miss the notification. Making RESET a top-level-only command isn't unreasonable, but using command status won't work as a general approach for notifying clients. We have a mechanism for GUC changes that uses a separate message (ParameterStatus). Perhaps that should be generalized to report different sorts of connection-related changes. -O ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Implementing RESET CONNECTION ...
Resending to the correctly-spelled list alias. Why does postgresql.org not generate bounces for unknown addresses, anyway? Original Message Subject: Re: [PATCHES] Implementing RESET CONNECTION ... Date: Tue, 04 Jan 2005 13:58:44 +1300 From: Oliver Jowett [EMAIL PROTECTED] To: Hans-Jürgen Schönig [EMAIL PROTECTED] CC: [EMAIL PROTECTED], pgman@candle.pha.pa.us, [EMAIL PROTECTED], [EMAIL PROTECTED] References: [EMAIL PROTECTED] Hans-Jürgen Schönig wrote: We have implemented a patch which can be used by connection pools for instance. RESECT CONNECTION cleans up a backend so that it can be reused. Perhaps this should be done at the protocol level (as a new message type), not as a SQL command, since it is dealing primarily with per-connection protocol state. As Kris has mentioned elsewhere in the thread, things like the JDBC driver really don't want the connection state unexpectedly being changed under them by user queries (and go to some lengths to detect and complain about it if they do, e.g. by monitoring ParameterStatus messages). If you do it at the protocol level, the semantics can be a bit more obvious: a reset packet resets the backend and protocol state to as if just connected state. Structure the reset packet like the initial startup packet; don't allow changing the user or database. So you get the same state as doing a disconnect/reconnect but presumably it is cheaper as you don't need to do authentication/fork/initialization. If this does go in at the SQL command level, can we have a protocol-level notification that the connection has been reset please? The JDBC driver needs to notice the reset to continue to operate correctly after a reset. I can't see an obvious way to crowbar this into the current protocol unless we overload something like ParameterStatus which seems ugly. -O ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] Implementing RESET CONNECTION ...
Tom Lane wrote: Kris Jurka [EMAIL PROTECTED] writes: Also I don't like the idea of cleaning up prepared statements. Hmm. This seems a bit eye-of-the-beholder-ish, ie you could make a legitimate argument either way. Maybe the RESET CONNECTION command should have an option whether to zap prepared statements or not? That doesn't really help the JDBC driver case. The problem is that there are prepared statements that have been set up by the driver invisibly to the user. Zapping them will make the driver break, and it's too easy for the user code to do a full RESET CONNECTION and accidently zap them. Yes, you can break the JDBC driver currently by doing explicit DEALLOCATEs of its prepared statements -- but you have to do that quite deliberately so it's less of a problem. Having notification of either prepared statement deallocation or connection reset (a la ParameterStatus for GUC changes) would help the driver to recover from this case. -O ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] Implementing RESET CONNECTION ...
Tom Lane wrote: Fair point, but you could make the same argument against *any* side effect of RESET CONNECTION. You're just complaining about PREPARE because you can see immediately where that breaks JDBC. Anything that any driver does to set up per-connection state the way it wants will be equally vulnerable. Yes, exactly. Perhaps RESET CONNECTION should be a protocol-level operation instead of a SQL command? That would prevent user-level code from causing it without the driver knowing. I just suggested as much in another email (our emails crossed). -O ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] cast pid_t to int when using *printf
Bruce Momjian wrote: I see in include/sys/types.h on Solaris 9: #if defined(_LP64) || defined(_I32LPx) typedef uint_t nlink_t; /* file link type */ typedef int pid_t; /* process id type */ #else typedef ulong_t nlink_t;/* (historical version) */ typedef longpid_t; /* (historical version) */ #endif I am confused why you are seeing long for pid_t? What is your Solaris system information? If Solaris needs adjustments, there are a lot of place we print getpid(). This is also what is on the Solaris system I was using. gcc -E showed that the #else branch was being taken. The #if branch is taken when compiling in 64-bit mode (gcc -m64). We're fine so long as everything casts to either int or long. I only saw warnings from a couple of places that did not do a cast at all. -O ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] Support LDFLAGS_SL on most ports
Peter Eisentraut wrote: Two comments: First, support for specifying it on configure requires you to call AC_ARG_VAR (require as in it would prevent people from shooting themselves in the foot). Please read up on it and add the appropriate call. Ok, so that makes sure that the var is hooked into the cache/autoreconfig/etc machinery. Fixed. Second, the AC_MSG_NOTICE calls are there for some variables because configure may modify them. But LDFLAGS_SL is just taken as given, so there is no need to print a notice about it. Personally I find it annoying when configure eats arguments without any feedback at all -- did it actually use the parameter, or did I make a typo? I've removed the AC_MSG_NOTICE anyway. Updated patch attached. -O Index: configure.in === RCS file: /projects/cvsroot/pgsql-server/configure.in,v retrieving revision 1.380 diff -u -c -r1.380 configure.in *** configure.in6 Oct 2004 09:35:19 - 1.380 --- configure.in7 Oct 2004 22:40:42 - *** *** 532,537 --- 532,538 AC_MSG_NOTICE([using CPPFLAGS=$CPPFLAGS]) AC_MSG_NOTICE([using LDFLAGS=$LDFLAGS]) + AC_ARG_VAR(LDFLAGS_SL) AC_PROG_AWK PGAC_PATH_FLEX Index: src/Makefile.global.in === RCS file: /projects/cvsroot/pgsql-server/src/Makefile.global.in,v retrieving revision 1.200 diff -u -c -r1.200 Makefile.global.in *** src/Makefile.global.in 6 Oct 2004 15:14:13 - 1.200 --- src/Makefile.global.in 7 Oct 2004 22:40:42 - *** *** 200,205 --- 200,206 with_gnu_ld = @with_gnu_ld@ ld_R_works = @ld_R_works@ LDFLAGS = @LDFLAGS@ + LDFLAGS_SL = @LDFLAGS_SL@ LDREL = -r LDOUT = -o RANLIB = @RANLIB@ Index: src/Makefile.shlib === RCS file: /projects/cvsroot/pgsql-server/src/Makefile.shlib,v retrieving revision 1.78 diff -u -c -r1.78 Makefile.shlib *** src/Makefile.shlib 2 Sep 2004 23:06:43 - 1.78 --- src/Makefile.shlib 7 Oct 2004 22:40:42 - *** *** 276,282 # Normal case $(shlib): $(OBJS) ! $(LINK.shared) $(OBJS) $(SHLIB_LINK) -o $@ # If we're using major and minor versions, then make a symlink to major-version-only. ifneq ($(shlib), $(shlib_major)) rm -f $(shlib_major) --- 276,282 # Normal case $(shlib): $(OBJS) ! $(LINK.shared) $(LDFLAGS_SL) $(OBJS) $(SHLIB_LINK) -o $@ # If we're using major and minor versions, then make a symlink to major-version-only. ifneq ($(shlib), $(shlib_major)) rm -f $(shlib_major) ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
[PATCHES] Support LDFLAGS_SL on most ports
This patch includes LDFLAGS_SL in SHLIB_LINK on most ports (previously it was only used on AIX and BeOS), and adds support for specifying it in configure. This lets you do something like: ./configure LDFLAGS=-static-libgcc LDFLAGS_SL=-static-libgcc to produce binaries that do not depend on libgcc_s.so at all. I haven't touched the win32/cygwin section of Makefile.shlib as I'm not sure exactly where it'd be useful to add LDFLAGS_SL. -O Index: src/Makefile.global.in === RCS file: /projects/cvsroot/pgsql-server/src/Makefile.global.in,v retrieving revision 1.195 diff -u -c -r1.195 Makefile.global.in *** src/Makefile.global.in 18 Sep 2004 13:28:54 - 1.195 --- src/Makefile.global.in 24 Sep 2004 06:23:11 - *** *** 199,204 --- 199,205 with_gnu_ld = @with_gnu_ld@ ld_R_works = @ld_R_works@ LDFLAGS = @LDFLAGS@ + LDFLAGS_SL = @LDFLAGS_SL@ LDREL = -r LDOUT = -o RANLIB = @RANLIB@ Index: src/Makefile.shlib === RCS file: /projects/cvsroot/pgsql-server/src/Makefile.shlib,v retrieving revision 1.78 diff -u -c -r1.78 Makefile.shlib *** src/Makefile.shlib 2 Sep 2004 23:06:43 - 1.78 --- src/Makefile.shlib 24 Sep 2004 06:23:12 - *** *** 276,282 # Normal case $(shlib): $(OBJS) ! $(LINK.shared) $(OBJS) $(SHLIB_LINK) -o $@ # If we're using major and minor versions, then make a symlink to major-version-only. ifneq ($(shlib), $(shlib_major)) rm -f $(shlib_major) --- 276,282 # Normal case $(shlib): $(OBJS) ! $(LINK.shared) $(LDFLAGS_SL) $(OBJS) $(SHLIB_LINK) -o $@ # If we're using major and minor versions, then make a symlink to major-version-only. ifneq ($(shlib), $(shlib_major)) rm -f $(shlib_major) Index: configure.in === RCS file: /projects/cvsroot/pgsql-server/configure.in,v retrieving revision 1.377 diff -u -c -r1.377 configure.in *** configure.in17 Sep 2004 22:31:59 - 1.377 --- configure.in24 Sep 2004 06:23:12 - *** *** 531,536 --- 531,538 AC_MSG_NOTICE([using CPPFLAGS=$CPPFLAGS]) AC_MSG_NOTICE([using LDFLAGS=$LDFLAGS]) + AC_MSG_NOTICE([using LDFLAGS_SL=$LDFLAGS_SL]) + AC_SUBST(LDFLAGS_SL) AC_PROG_AWK ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
[PATCHES] cast pid_t to int when using *printf
gcc (3.2.3 on Solaris 9) warns about a couple of places where a pid_t is formatted with %d by a printf-family function. This patch explicitly casts to int to suppress the warning. -O Index: src/backend/postmaster/pgstat.c === RCS file: /projects/cvsroot/pgsql-server/src/backend/postmaster/pgstat.c,v retrieving revision 1.80 diff -u -c -r1.80 pgstat.c *** src/backend/postmaster/pgstat.c 29 Aug 2004 05:06:46 - 1.80 --- src/backend/postmaster/pgstat.c 24 Sep 2004 06:46:27 - *** *** 1505,1511 snprintf(pgStat_fname, MAXPGPATH, PGSTAT_STAT_FILENAME, DataDir); /* tmpfname need only be set correctly in this process */ snprintf(pgStat_tmpfname, MAXPGPATH, PGSTAT_STAT_TMPFILE, !DataDir, getpid()); /* * Arrange to write the initial status file right away --- 1505,1511 snprintf(pgStat_fname, MAXPGPATH, PGSTAT_STAT_FILENAME, DataDir); /* tmpfname need only be set correctly in this process */ snprintf(pgStat_tmpfname, MAXPGPATH, PGSTAT_STAT_TMPFILE, !DataDir, (int)getpid()); /* * Arrange to write the initial status file right away Index: src/backend/postmaster/postmaster.c === RCS file: /projects/cvsroot/pgsql-server/src/backend/postmaster/postmaster.c,v retrieving revision 1.425 diff -u -c -r1.425 postmaster.c *** src/backend/postmaster/postmaster.c 9 Sep 2004 00:59:33 - 1.425 --- src/backend/postmaster/postmaster.c 24 Sep 2004 06:46:27 - *** *** 2835,2841 */ ereport(DEBUG3, (errmsg_internal(%s child[%d]: starting with (, !progname, getpid(; for (i = 0; i ac; ++i) ereport(DEBUG3, (errmsg_internal(\t%s, av[i]))); --- 2835,2841 */ ereport(DEBUG3, (errmsg_internal(%s child[%d]: starting with (, !progname, (int)getpid(; for (i = 0; i ac; ++i) ereport(DEBUG3, (errmsg_internal(\t%s, av[i]))); ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] cast pid_t to int when using *printf
Peter Eisentraut wrote: Am Freitag, 24. September 2004 09:34 schrieb Oliver Jowett: Neil Conway wrote: On Fri, 2004-09-24 at 16:51, Oliver Jowett wrote: gcc (3.2.3 on Solaris 9) warns about a couple of places where a pid_t is formatted with %d by a printf-family function. For curiosity's sake, what formatting escape does gcc prefer? I don't think there is an escape for pid_t, you always have to cast it. I think what he was asking is this: Since pid_t has to be a signed integer type, but gcc does not accept %d for it, then it could be that pid_t is wider than an int, so casting it to int would potentially lose information. pid_t on the Solaris/sparc system is a long (but both int and long are 32 bits). Some experimentation shows that gcc is happy with a %ld format specifier. But compiling the same code on a Linux/x86 system makes gcc complain when applying %ld to pid_t, so we will need a cast there either way. I notice that elog.c casts MyProcPid to long and uses %ld. Amusingly, MyProcPid is explicitly an int.. -O ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] SGML cleanup
Alvaro Herrera wrote: para !The SQL2003 standard specifies that the keyword !literalSAVEPOINT/ is mandatory. productnamePostgreSQL/ and !productnameOracle/ allow the literalSAVEPOINT/literal !keyword to be omitted. SQL2003 allows only literalWORK/, not Why are we mentioning Oracle behavior at all? One of the influences on the SAVEPOINT/ROLLBACK syntax was avoiding gratuitous incompatibility with Oracle; doesn't it make sense to document this? -O ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] libpq problem
Bruce Momjian wrote: OK, I like your idea of chaining on to any existing SIGPIPE handler rather than just do it if none is installed. I also see your fix for the uninitialized thread-specific variable. I added some comments to the patch, renamed the pipeheader variable so it was pg_* to avoid namespace conflicts, and updated the documentation. Patch attached and applied. At a glance, this looks like it will break applications that pass SA_SIGINFO in sa_flags for their SIGPIPE handlers. This changes the expected signal handler signature to a three-arg form; the extra two args provide context about where the signal occurred. The libpq handler, however, doesn't pass those args when chaining to the next handler. The Sun JVM under linux is one example of an app that does1 this. I've seen a similar problem to this before with a version of libnss_ldap that did not correctly restore the entire sigaction state when restoring the SIGPIPE handler before returning from libnss_ldap .. the next SIGPIPE that arrives would crash the JVM. See http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4630104 for more details (requires registration) -O ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] Add GUC_REPORT to server_encoding, integer_datetimes
Bruce Momjian wrote: Is this 8.0 material? Tom's already applied it.. -O ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] [HACKERS] SAVEPOINT syntax again
Tom Lane wrote: Oliver Jowett [EMAIL PROTECTED] writes: release savepoint statement ::= RELEASE SAVEPOINT savepoint specifier Oracle does not have RELEASE SAVEPOINT. DB2 has RELEASE [ TO ] SAVEPOINT savepoint specifier I'd vote for RELEASE [ SAVEPOINT ] name (for brevity, and for consistency with ROLLBACK). I feel no urge to copy DB2. Here's an updated patch that supports the syntax you suggest. I kept the error messages, doc examples and regression tests using RELEASE SAVEPOINT in full to follow the standard. -O ? GNUmakefile ? config.log ? config.status ? src/Makefile.global ? src/include/pg_config.h ? src/include/stamp-h Index: doc/src/sgml/ref/release.sgml === RCS file: /projects/cvsroot/pgsql-server/doc/src/sgml/ref/release.sgml,v retrieving revision 1.1 diff -u -c -r1.1 release.sgml *** doc/src/sgml/ref/release.sgml 1 Aug 2004 17:32:13 - 1.1 --- doc/src/sgml/ref/release.sgml 12 Aug 2004 11:15:33 - *** *** 5,21 refentry id=SQL-RELEASE refmeta ! refentrytitle id=SQL-RELEASE-TITLERELEASE/refentrytitle refmiscinfoSQL - Language Statements/refmiscinfo /refmeta refnamediv ! refnameRELEASE/refname refpurposedestroy a previously defined savepoint/refpurpose /refnamediv indexterm zone=sql-release ! primaryRELEASE/primary /indexterm indexterm zone=sql-release --- 5,21 refentry id=SQL-RELEASE refmeta ! refentrytitle id=SQL-RELEASE-TITLERELEASE SAVEPOINT/refentrytitle refmiscinfoSQL - Language Statements/refmiscinfo /refmeta refnamediv ! refnameRELEASE SAVEPOINT/refname refpurposedestroy a previously defined savepoint/refpurpose /refnamediv indexterm zone=sql-release ! primaryRELEASE SAVEPOINT/primary /indexterm indexterm zone=sql-release *** *** 25,31 refsynopsisdiv synopsis ! RELEASE replaceablesavepoint_name/replaceable /synopsis /refsynopsisdiv --- 25,31 refsynopsisdiv synopsis ! RELEASE [ SAVEPOINT ] replaceablesavepoint_name/replaceable /synopsis /refsynopsisdiv *** *** 33,39 titleDescription/title para !commandRELEASE/command destroys a savepoint previously defined in the current transaction. /para --- 33,39 titleDescription/title para !commandRELEASE SAVEPOINT/command destroys a savepoint previously defined in the current transaction. /para *** *** 48,54 /para para !commandRELEASE/command also destroys all savepoints that were established after the named savepoint was established. /para /refsect1 --- 48,54 /para para !commandRELEASE SAVEPOINT/command also destroys all savepoints that were established after the named savepoint was established. /para /refsect1 *** *** 97,103 INSERT INTO table VALUES (3); SAVEPOINT my_savepoint; INSERT INTO table VALUES (4); ! RELEASE my_savepoint; COMMIT; /programlisting The above transaction will insert both 3 and 4. --- 97,103 INSERT INTO table VALUES (3); SAVEPOINT my_savepoint; INSERT INTO table VALUES (4); ! RELEASE SAVEPOINT my_savepoint; COMMIT; /programlisting The above transaction will insert both 3 and 4. *** *** 108,114 titleCompatibility/title para !RELEASE is fully conforming to the SQL standard. /para /refsect1 --- 108,116 titleCompatibility/title para !The SQL2003 standard specifies that the keyword SAVEPOINT is mandatory. !productnamePostgreSQL/productname allows the SAVEPOINT keyword to be !omitted. Otherwise, this command is fully conforming. /para /refsect1 Index: doc/src/sgml/ref/rollback_to.sgml === RCS file: /projects/cvsroot/pgsql-server/doc/src/sgml/ref/rollback_to.sgml,v retrieving revision 1.1 diff -u -c -r1.1 rollback_to.sgml *** doc/src/sgml/ref/rollback_to.sgml 1 Aug 2004 17:32:13 - 1.1 --- doc/src/sgml/ref/rollback_to.sgml 12 Aug 2004 11:15:33 - *** *** 5,21 refentry id=SQL-ROLLBACK-TO refmeta ! refentrytitle id=SQL-ROLLBACK-TO-TITLEROLLBACK TO/refentrytitle refmiscinfoSQL - Language Statements/refmiscinfo /refmeta refnamediv ! refnameROLLBACK TO/refname refpurposeroll back to a savepoint/refpurpose /refnamediv indexterm zone=sql-rollback-to ! primaryROLLBACK TO/primary /indexterm indexterm zone=sql-rollback-to --- 5,21 refentry id=SQL-ROLLBACK-TO refmeta ! refentrytitle id=SQL-ROLLBACK-TO-TITLEROLLBACK TO SAVEPOINT/refentrytitle refmiscinfoSQL - Language Statements/refmiscinfo /refmeta refnamediv ! refnameROLLBACK TO SAVEPOINT/refname refpurposeroll back to a savepoint/refpurpose
[PATCHES] Add GUC_REPORT to server_encoding, integer_datetimes
This adds GUC_REPORT to server_encoding and integer_datetimes so they are reported in the V3 protocol startup packet. Also some related doc updates. Rationale: 1) server_encoding is useful to allow clients to detect bad server/client encoding pairs. The one that bites JDBC regularly is client_encoding = UNICODE with server_encoding = SQL_ASCII. I'd prefer for the server to reject that combination entirely, but failing that clients can implement such a policy themselves by inspecting server_encoding when establishing a connection. 2) integer_datetimes affects the binary representation of date/time types. A client that wants to use binary parameters or resultsets that involve those types needs to know the value of this setting before dealing with the binary data. In both cases, it's good to have it in the startup packet to avoid an extra round-trip on connection establishment. Any chance that this can go into 8.0? -O ? GNUmakefile ? config.log ? config.status ? src/Makefile.global ? src/include/pg_config.h ? src/include/stamp-h Index: doc/src/sgml/libpq.sgml === RCS file: /projects/cvsroot/pgsql-server/doc/src/sgml/libpq.sgml,v retrieving revision 1.158 diff -u -c -r1.158 libpq.sgml *** doc/src/sgml/libpq.sgml 11 Aug 2004 18:06:00 - 1.158 --- doc/src/sgml/libpq.sgml 13 Aug 2004 03:29:45 - *** *** 854,864 para Parameters reported as of the current release include ! literalserver_version/ (cannot change after startup); ! literalclient_encoding/, literalis_superuser/, ! literalsession_authorization/literal, and ! literalDateStyle/. /para para --- 854,866 para Parameters reported as of the current release include ! xref linkend=guc-server-version, ! xref linkend=guc-server-encoding, ! xref linkend=guc-client-encoding, literalis_superuser/, ! literalsession_authorization/, ! xref linkend=guc-datestyle, and ! xref linkend=guc-integer-datetimes. /para para Index: doc/src/sgml/protocol.sgml === RCS file: /projects/cvsroot/pgsql-server/doc/src/sgml/protocol.sgml,v retrieving revision 1.52 diff -u -c -r1.52 protocol.sgml *** doc/src/sgml/protocol.sgml 11 Jun 2004 01:08:33 - 1.52 --- doc/src/sgml/protocol.sgml 13 Aug 2004 03:29:46 - *** *** 1046,1057 para At present there is a hard-wired set of parameters for which ParameterStatus will be generated: they are ! literalserver_version/ (a pseudo-parameter that cannot change after ! startup); ! literalclient_encoding/, literalis_superuser/, ! literalsession_authorization/literal, and ! literalDateStyle/. This set might change in the future, or even become configurable. Accordingly, a frontend should simply ignore ParameterStatus for parameters that it does not understand or care about. --- 1046,1058 para At present there is a hard-wired set of parameters for which ParameterStatus will be generated: they are ! xref linkend=guc-server-version, ! xref linkend=guc-server-encoding, ! xref linkend=guc-client-encoding, literalis_superuser/, ! literalsession_authorization/, ! xref linkend=guc-datestyle, and ! xref linkend=guc-integer-datetimes. This set might change in the future, or even become configurable. Accordingly, a frontend should simply ignore ParameterStatus for parameters that it does not understand or care about. Index: doc/src/sgml/runtime.sgml === RCS file: /projects/cvsroot/pgsql-server/doc/src/sgml/runtime.sgml,v retrieving revision 1.276 diff -u -c -r1.276 runtime.sgml *** doc/src/sgml/runtime.sgml 12 Aug 2004 19:03:17 - 1.276 --- doc/src/sgml/runtime.sgml 13 Aug 2004 03:29:46 - *** *** 2674,2681 variablelist ! varlistentry id=guc-datestyle xreflabel=datestyle ! termvarnamedatestyle/varname (typestring/type)/term indextermprimarydate style// listitem para --- 2674,2681 variablelist ! varlistentry id=guc-datestyle xreflabel=DateStyle ! termvarnameDateStyle/varname (typestring/type)/term indextermprimarydate style// listitem para *** *** 2747,2752 --- 2747,2763 /listitem /varlistentry + varlistentry id=guc-server-encoding xreflabel=server_encoding + termvarnameserver_encoding/varname (typestring/type)/term + indextermprimarycharacter set// + listitem +para + This parameter shows the database encoding (character set). + It is determined when the database is created, and is read-only. +/para + /listitem + /varlistentry + varlistentry id=guc-client-encoding xreflabel=client_encoding
Re: [PATCHES] Add GUC_REPORT to server_encoding, integer_datetimes
Oliver Jowett wrote: This adds GUC_REPORT to server_encoding and integer_datetimes so they are reported in the V3 protocol startup packet. s/startup packet/startup process/ -O ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] SAVEPOINT syntax again
Making the assumption that we want standards-conforming syntax here, I went ahead and did the necessary changes: Oliver Jowett wrote: Comments: 1) We have a different syntax to the SQL200n draft (and Oracle by the looks of it) for ROLLBACK. The draft says: rollback statement ::= ROLLBACK [ WORK ] [ AND [ NO ] CHAIN ] [ savepoint clause ] savepoint clause ::= TO SAVEPOINT savepoint specifier Oracle has ROLLBACK TO [ SAVEPOINT ] savepoint specifier DB2 has ROLLBACK TO SAVEPOINT savepoint specifier 2) We have a different syntax for RELEASE too. The SQL200n draft says: release savepoint statement ::= RELEASE SAVEPOINT savepoint specifier Oracle does not have RELEASE SAVEPOINT. DB2 has RELEASE [ TO ] SAVEPOINT savepoint specifier The attached patch implements an approximate union of the above syntaxes: ROLLBACK [ WORK | TRANSACTION ] TO [ SAVEPOINT ] savepoint specifier RELEASE [ TO ] SAVEPOINT savepoint specifier Note that this means that RELEASE foo is no longer valid. It seems solely a postgresql-ism -- anyone particularly attached to that syntax? Also in the patch are documentation and regression test updates to reflect the new syntax. I have changed the examples in the docs and the regression tests to prefer the standard-conforming variants. Error messages now refer to ROLLBACK TO SAVEPOINT and RELEASE SAVEPOINT rather than ROLLBACK TO and RELEASE. -O ? GNUmakefile ? config.log ? config.status ? src/Makefile.global ? src/include/pg_config.h ? src/include/stamp-h Index: doc/src/sgml/ref/release.sgml === RCS file: /projects/cvsroot/pgsql-server/doc/src/sgml/ref/release.sgml,v retrieving revision 1.1 diff -u -c -r1.1 release.sgml *** doc/src/sgml/ref/release.sgml 1 Aug 2004 17:32:13 - 1.1 --- doc/src/sgml/ref/release.sgml 11 Aug 2004 23:38:37 - *** *** 5,21 refentry id=SQL-RELEASE refmeta ! refentrytitle id=SQL-RELEASE-TITLERELEASE/refentrytitle refmiscinfoSQL - Language Statements/refmiscinfo /refmeta refnamediv ! refnameRELEASE/refname refpurposedestroy a previously defined savepoint/refpurpose /refnamediv indexterm zone=sql-release ! primaryRELEASE/primary /indexterm indexterm zone=sql-release --- 5,21 refentry id=SQL-RELEASE refmeta ! refentrytitle id=SQL-RELEASE-TITLERELEASE SAVEPOINT/refentrytitle refmiscinfoSQL - Language Statements/refmiscinfo /refmeta refnamediv ! refnameRELEASE SAVEPOINT/refname refpurposedestroy a previously defined savepoint/refpurpose /refnamediv indexterm zone=sql-release ! primaryRELEASE SAVEPOINT/primary /indexterm indexterm zone=sql-release *** *** 25,31 refsynopsisdiv synopsis ! RELEASE replaceablesavepoint_name/replaceable /synopsis /refsynopsisdiv --- 25,31 refsynopsisdiv synopsis ! RELEASE [ TO ] SAVEPOINT replaceablesavepoint_name/replaceable /synopsis /refsynopsisdiv *** *** 33,39 titleDescription/title para !commandRELEASE/command destroys a savepoint previously defined in the current transaction. /para --- 33,39 titleDescription/title para !commandRELEASE SAVEPOINT/command destroys a savepoint previously defined in the current transaction. /para *** *** 48,54 /para para !commandRELEASE/command also destroys all savepoints that were established after the named savepoint was established. /para /refsect1 --- 48,54 /para para !commandRELEASE SAVEPOINT/command also destroys all savepoints that were established after the named savepoint was established. /para /refsect1 *** *** 97,103 INSERT INTO table VALUES (3); SAVEPOINT my_savepoint; INSERT INTO table VALUES (4); ! RELEASE my_savepoint; COMMIT; /programlisting The above transaction will insert both 3 and 4. --- 97,103 INSERT INTO table VALUES (3); SAVEPOINT my_savepoint; INSERT INTO table VALUES (4); ! RELEASE SAVEPOINT my_savepoint; COMMIT; /programlisting The above transaction will insert both 3 and 4. *** *** 108,114 titleCompatibility/title para !RELEASE is fully conforming to the SQL standard. /para /refsect1 --- 108,117 titleCompatibility/title para !The SQL2003 standard specifies only a RELEASE SAVEPOINT form. !productnamePostgreSQL/productname and productnameDB2/productname !also allow RELEASE TO SAVEPOINT. Otherwise, this command is !fully conforming. /para /refsect1 Index: doc/src/sgml/ref/rollback_to.sgml === RCS file: /projects/cvsroot/pgsql-server/doc/src/sgml/ref/rollback_to.sgml,v retrieving revision 1.1 diff -u -c -r1.1 rollback_to.sgml *** doc
[PATCHES] fix for parameterized queries in DECLARE CURSOR statements
Here's a patch that allows parameterized queries to be used in a DECLARE CURSOR statement. Previously, the DECLARE would succeed but any FETCHes would fail as the parameter values supplied to DECLARE were not propagated to the portal it created. This patch adds that propagation. See http://archives.postgresql.org/pgsql-hackers/2004-07/msg01047.php for discussion. I've tested the V3 protocol and SQL function paths by hand. The SPI path hasn't been specifically tested but the regression tests appear happy. -O ? src/Makefile.global ? src/include/pg_config.h ? src/include/stamp-h Index: src/backend/commands/portalcmds.c === RCS file: /projects/cvsroot/pgsql-server/src/backend/commands/portalcmds.c,v retrieving revision 1.29 diff -u -c -r1.29 portalcmds.c *** src/backend/commands/portalcmds.c 17 Jul 2004 03:28:47 - 1.29 --- src/backend/commands/portalcmds.c 26 Jul 2004 11:37:24 - *** *** 36,42 *Execute SQL DECLARE CURSOR command. */ void ! PerformCursorOpen(DeclareCursorStmt *stmt) { List *rewritten; Query *query; --- 36,42 *Execute SQL DECLARE CURSOR command. */ void ! PerformCursorOpen(DeclareCursorStmt *stmt, ParamListInfo params) { List *rewritten; Query *query; *** *** 104,111 list_make1(plan), PortalGetHeapMemory(portal)); - MemoryContextSwitchTo(oldContext); - /* * Set up options for portal. * --- 104,109 *** *** 123,131 } /* !* Start execution --- never any params for a cursor. */ ! PortalStart(portal, NULL); Assert(portal-strategy == PORTAL_ONE_SELECT); --- 121,133 } /* !* Start execution. The original DECLARE may have had parameters !* (if V3 protocol Parse/Bind messages were used); we must !* pass those parameters into the new portal, making sure to copy them !* into the new portal's memory context as the originals are owned by !* our caller. */ ! PortalStart(portal, copyParamList(params)); Assert(portal-strategy == PORTAL_ONE_SELECT); *** *** 133,138 --- 135,142 * We're done; the query won't actually be run until * PerformPortalFetch is called. */ + + MemoryContextSwitchTo(oldContext); } /* Index: src/backend/commands/prepare.c === RCS file: /projects/cvsroot/pgsql-server/src/backend/commands/prepare.c,v retrieving revision 1.28 diff -u -c -r1.28 prepare.c *** src/backend/commands/prepare.c 11 Jun 2004 01:08:38 - 1.28 --- src/backend/commands/prepare.c 26 Jul 2004 11:37:24 - *** *** 211,217 int nargs = list_length(argtypes); ParamListInfo paramLI; List *exprstates; ! ListCell *l; int i = 0; /* Parser should have caught this error, but check for safety */ --- 211,217 int nargs = list_length(argtypes); ParamListInfo paramLI; List *exprstates; ! ListCell *le, *la; int i = 0; /* Parser should have caught this error, but check for safety */ *** *** 223,231 paramLI = (ParamListInfo) palloc0((nargs + 1) * sizeof(ParamListInfoData)); ! foreach(l, exprstates) { ! ExprState *n = lfirst(l); boolisNull; paramLI[i].value = ExecEvalExprSwitchContext(n, --- 223,231 paramLI = (ParamListInfo) palloc0((nargs + 1) * sizeof(ParamListInfoData)); ! forboth(le, exprstates, la, argtypes) { ! ExprState *n = lfirst(le); boolisNull; paramLI[i].value = ExecEvalExprSwitchContext(n, *** *** 235,240 --- 235,241 paramLI[i].kind = PARAM_NUM; paramLI[i].id = i + 1; paramLI[i].isnull = isNull; + paramLI[i].valuetype = lfirst_oid(la); i++; } Index: src/backend/commands/schemacmds.c === RCS file: /projects/cvsroot/pgsql-server/src/backend/commands/schemacmds.c,v retrieving revision 1.20 diff -u -c -r1.20 schemacmds.c *** src/backend/commands/schemacmds.c 25 Jun 2004 21:55:53 - 1.20 --- src/backend/commands/schemacmds.c 26 Jul 2004 11:37:24 - *** *** 168,174 /* schemas should contain only utility stmts */
Re: [PATCHES] [HACKERS] Function to kill backend
Tom Lane wrote: So what you'd basically need is a separate signal to trigger that sort of exit, which would be easy ... if we had any spare signal numbers. What about multiplexing it onto an existing signal? e.g. set a shared-mem flag saying exit after cancel then send SIGINT? I guess this is getting away from the original patch though.. -O ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] Nested transactions
Barry Lind wrote: The other thing that I have been meaning to say in this thread is that I don't like using COMMIT to mean subtransaction commit (vs. introducing a new command for it) because of the following situation. Lets say that I have a java method that takes a jdbc connection and this code starts a transaction and does some work then issues a commit to ensure the changes have been committed to the database, and then does some other work outside the database based on the fact that the commit was sucessfull and it therefore knows the data is saved to disk (i.e. send out an email notification, or any number of other non-database tasks). Now lets suppose that someone calls this method with a database connection that already has a transaction started, so that this method really is beginning and working with a sub-transaction. Now when it commits it doesn't know if the changes will ever get to disk since its commit could be rolled back later. So this code gets broken. Note that there is no standard way in JDBC to enter a subtransaction unless you issue the BEGIN explicitly or invoke the savepoint API. There are lots of ways to confuse the driver's transaction handling already if you issue arbitary transaction control instructions (althugh I'm working on making the driver recover better -- the v3 transaction status indicator in ReadyForQuery helps). And, well, how do you expect the code to ever not be broken? If commit() really does commit the whole transaction, the caller code that expects to still be in a transaction is going to be unhappy (if it doesn't expect to still be in a transaction, why is it opening a subtransaction at all?). The callee should be using JTA's registerSynchronization() to get callbacks after transaction commit, or using 2PC (yes I know it's not supported yet), depending on the guarantees it needs. Earlier Alvaro was looking at ways to provide the transaction nesting level via the client protocol; I suggested doing it as a parameter (so you get ParameterStatus on nesting change) but I'm not sure what happened with it after that. Assuming something does get done here, the driver can track where it is in subtransactions quite easily, and so if you want Connection.commit() to really mean commit this transaction and all subtransactions even in the face of the user messing around with BEGIN themselves, we can do that even if multiple COMMITs are needed -- we just look at the current nesting level to work out how many to issue. This behaviour (of commit()/rollback()) actually makes sense as things like connection pools will expect Connection.commit() or Connection.rollback to produce a reasonably vanilla connection state, and transaction monitors are likely to want those methods to affect the entire transaction too. I like the functionality of nested transactions, I just think that there needs to be different commands other than BEGIN/COMMIT to work with them. So that there is no possiblity for misunderstanding what COMMIT really means. BEGIN NESTED WORK / COMMIT NESTED WORK / END NESTED WORK or something? And make plain BEGIN inside a transaction a warning (as it currently is) and plain COMMIT/END inside a subtransaction an error? (or should they affect all subtransactions?) I can see this having some value for dealing with existing applications that issue redundant BEGIN/COMMIT/ROLLBACK statements (and get warnings, but ignore them). -O ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org