Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL

2005-09-11 Thread Oliver Jowett
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

2005-09-08 Thread Oliver Jowett
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

2005-09-08 Thread Oliver Jowett
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

2005-09-08 Thread Oliver Jowett
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

2005-09-08 Thread Oliver Jowett
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

2005-07-29 Thread Oliver Jowett
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

2005-07-04 Thread Oliver Jowett
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

2005-05-26 Thread Oliver Jowett

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

2005-05-04 Thread Oliver Jowett
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)

2005-05-02 Thread Oliver Jowett
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 ...

2005-01-04 Thread Oliver Jowett
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 ...

2005-01-04 Thread Oliver Jowett
(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 ...

2005-01-03 Thread Oliver Jowett
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 ...

2005-01-03 Thread Oliver Jowett
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 ...

2005-01-03 Thread Oliver Jowett
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

2004-10-09 Thread Oliver Jowett
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

2004-10-07 Thread Oliver Jowett
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

2004-09-24 Thread Oliver Jowett
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

2004-09-24 Thread Oliver Jowett
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

2004-09-24 Thread Oliver Jowett
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

2004-08-22 Thread Oliver Jowett
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

2004-08-17 Thread Oliver Jowett
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

2004-08-17 Thread Oliver Jowett
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

2004-08-12 Thread Oliver Jowett
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

2004-08-12 Thread Oliver Jowett
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

2004-08-12 Thread Oliver Jowett
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

2004-08-11 Thread Oliver Jowett
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

2004-07-26 Thread Oliver Jowett
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

2004-07-26 Thread Oliver Jowett
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

2004-06-18 Thread Oliver Jowett
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