[HACKERS] walreceiver is uninterruptible on win32

2010-03-10 Thread Fujii Masao
Hi,

http://archives.postgresql.org/pgsql-hackers/2010-01/msg01672.php
On win32, the blocking libpq functions like PQconnectdb() and
PQexec() are uninterruptible since they use the vanilla select()
instead of our signal emulation layer compatible select().
Nevertheless, currently walreceiver uses them to establish a
connection, send a handshake message and wait for the reply.
So walreceiver also becomes uninterruptible for a while. This
is the must-fix problem for 9.0.

I replaced the blocking libpq functions currently used with
asynchronous ones, and used the emulated version of select()
to wait, to make walreceiver interruptible. Here is the patch.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
*** a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
--- b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
***
*** 18,23 
--- 18,24 
  
  #include unistd.h
  #include sys/time.h
+ #include time.h
  
  #include libpq-fe.h
  #include access/xlog.h
***
*** 53,59  static bool libpqrcv_receive(int timeout, unsigned char *type,
  static void libpqrcv_disconnect(void);
  
  /* Prototypes for private functions */
! static bool libpq_select(int timeout_ms);
  
  /*
   * Module load callback
--- 54,61 
  static void libpqrcv_disconnect(void);
  
  /* Prototypes for private functions */
! static int libpq_select(bool forRead, bool forWrite, int timeout_ms);
! static PGresult *libpqrcv_PQexec(const char *query);
  
  /*
   * Module load callback
***
*** 83,103  libpqrcv_connect(char *conninfo, XLogRecPtr startpoint)
  	TimeLineID	standby_tli;
  	PGresult   *res;
  	char		cmd[64];
  
  	/* Connect */
  	snprintf(conninfo_repl, sizeof(conninfo_repl), %s replication=true, conninfo);
  
! 	streamConn = PQconnectdb(conninfo_repl);
! 	if (PQstatus(streamConn) != CONNECTION_OK)
  		ereport(ERROR,
  (errmsg(could not connect to the primary server : %s,
  		PQerrorMessage(streamConn;
  
  	/*
  	 * Get the system identifier and timeline ID as a DataRow message from the
  	 * primary server.
  	 */
! 	res = PQexec(streamConn, IDENTIFY_SYSTEM);
  	if (PQresultStatus(res) != PGRES_TUPLES_OK)
  	{
  		PQclear(res);
--- 85,198 
  	TimeLineID	standby_tli;
  	PGresult   *res;
  	char		cmd[64];
+ 	PQconninfoOption   *options;
+ 	time_t		finish_time = ((time_t) -1);
+ 
+ 	/*
+ 	 * Extract timeout from the connection string
+ 	 */
+ 	options = PQconninfoParse(conninfo, NULL);
+ 	if (options)
+ 	{
+ 		PQconninfoOption *option;
+ 		for (option = options; option-keyword != NULL; option++)
+ 		{
+ 			if (strcmp(option-keyword, connect_timeout) == 0)
+ 			{
+ if (option-val != NULL  option-val[0] != '\0')
+ {
+ 	int		timeout = atoi(option-val);
+ 
+ 	if (timeout  0)
+ 	{
+ 		/*
+ 		 * Rounding could cause connection to fail;
+ 		 * need at least 2 secs
+ 		 */
+ 		if (timeout  2)
+ 			timeout = 2;
+ 		/* calculate the finish time based on start + timeout */
+ 		finish_time = time(NULL) + timeout;
+ 	}
+ }
+ 			}
+ 		}
+ 		PQconninfoFree(options);
+ 	}
  
  	/* Connect */
  	snprintf(conninfo_repl, sizeof(conninfo_repl), %s replication=true, conninfo);
  
! 	streamConn = PQconnectStart(conninfo_repl);
! 	if (PQstatus(streamConn) == CONNECTION_BAD)
  		ereport(ERROR,
  (errmsg(could not connect to the primary server : %s,
  		PQerrorMessage(streamConn;
  
  	/*
+ 	 * Wait for connection to be established
+ 	 */
+ 	for (;;)
+ 	{
+ 		PostgresPollingStatusType	status;
+ 		bool		established = false;
+ 		bool		forRead = false;
+ 		bool		forWrite = false;
+ 		int		timeout_ms;
+ 		int		ret;
+ 
+ 		status = PQconnectPoll(streamConn);
+ 		switch (status)
+ 		{
+ 			case PGRES_POLLING_READING:
+ forRead = true;
+ break;
+ 			case PGRES_POLLING_WRITING:
+ forWrite = true;
+ break;
+ 			case PGRES_POLLING_OK:
+ established = true;
+ break;
+ 			case PGRES_POLLING_FAILED:
+ 			default:
+ ereport(ERROR,
+ 		(errmsg(could not connect to the primary server : %s,
+ PQerrorMessage(streamConn;
+ 		}
+ 
+ 		if (established)
+ 			break;
+ 
+ 	retry:
+ 		/* Compute appropriate timeout interval */
+ 		if (finish_time == ((time_t) -1))
+ 			timeout_ms = -1;
+ 		else
+ 		{
+ 			time_t		now = time(NULL);
+ 
+ 			if (finish_time  now)
+ timeout_ms = (finish_time - now) * 1000;
+ 			else
+ timeout_ms = 0;
+ 		}
+ 
+ 		/*
+ 		 * Wait until we can read or write the connection socket
+ 		 */
+ 		ret = libpq_select(forRead, forWrite, timeout_ms);
+ 		if (ret == 0)	/* timeout */
+ 			ereport(ERROR,
+ 	(errmsg(could not connect to the primary server : timeout expired)));
+ 		if (ret  0) /* interrupted */
+ 			goto retry;
+ 	}
+ 
+ 	/*
  	 * Get the system identifier and timeline ID as a DataRow message from the
  	 * primary server.
  	 */
! 	res = libpqrcv_PQexec(IDENTIFY_SYSTEM);
  	if (PQresultStatus(res) 

Re: [HACKERS] Dyamic updates of NEW with pl/pgsql

2010-03-10 Thread hubert depesz lubaczewski
On Tue, Mar 09, 2010 at 06:59:31PM +0100, Pavel Stehule wrote:
 2010/3/9 strk s...@keybit.net:
  How can a pl/pgsql trigger change the
  values of dynamic fields in NEW record ?
 
  By dynamic I mean that the field name
  is a variable in the trigger context.
 
  I've been told it's easy to do with pl/perl but
  I'd like to delive a pl/pgsql solution to have
  less dependencies.
 
 It isn't possible yet

well, it's possible. it's just not nice.

http://www.depesz.com/index.php/2010/03/10/dynamic-updates-of-fields-in-new-in-plpgsql/

depesz

-- 
Linkedin: http://www.linkedin.com/in/depesz  /  blog: http://www.depesz.com/
jid/gtalk: dep...@depesz.com / aim:depeszhdl / skype:depesz_hdl / gg:6749007

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Re: Hot Standby query cancellation and Streaming Replication integration

2010-03-10 Thread Greg Stark
On Wed, Mar 10, 2010 at 6:29 AM, Josh Berkus j...@agliodbs.com wrote:
 Then I increased vacuum_defer_cleanup_age to 10, which represents
 about 5 minutes of transactions on the test system.  This eliminated all
 query cancels for the reporting query, which takes an average of 10s.

 Next is a database bloat test, but I'll need to do that on a system with
 more free space than my laptop.

Note that this will be heavily dependent on the use case. If you have
one of those counter records that keeps being updated and gets cleaned
up by HOT whenever the page fills up then you need to allow HOT to
clean it up before it overflows the page or else it'll bloat the table
and require a real vacuum. I think that means that a
vacuum_defer_cleanup of up to about 100 or so (it depends on the width
of your counter record) might be reasonable as a general suggestion
but anything higher will depend on understanding the specific system.

Another use case that might suprise people who are accustomed to the
current behaviour is massive updates. This is the main really pessimal
use case left in Postgres -- ideally they wouldn't bloat the table at
all but currently they double the size of the table. People may be
accustomed to the idea that they can then run vacuum and that will
limit the bloat to 50%, assuming they have no (other) long-lived
transactions. With vacuum_defer_cleanup that will no longer be true.
It will be as if you always have a query lasting n transactions in
your system at all times.

-- 
greg

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Dyamic updates of NEW with pl/pgsql

2010-03-10 Thread Andrew Dunstan



hubert depesz lubaczewski wrote:

On Tue, Mar 09, 2010 at 06:59:31PM +0100, Pavel Stehule wrote:
  

2010/3/9 strk s...@keybit.net:


How can a pl/pgsql trigger change the
values of dynamic fields in NEW record ?

By dynamic I mean that the field name
is a variable in the trigger context.

I've been told it's easy to do with pl/perl but
I'd like to delive a pl/pgsql solution to have
less dependencies.
  

It isn't possible yet



well, it's possible. it's just not nice.

http://www.depesz.com/index.php/2010/03/10/dynamic-updates-of-fields-in-new-in-plpgsql/
  


Using an hstore in 9.0 it's not too bad, Try something like:

   CREATE OR REPLACE FUNCTION dyntrig()
RETURNS trigger
LANGUAGE plpgsql
   AS $function$

   declare
   hst hstore;
   begin
   hst := hstore(NEW);
   hst := hst || ('foo' = 'bar');
   NEW := populate_record(NEW,hst);
   return NEW;
   end;

   $function$;


But this question probably belongs on -general rather than -hackers.

cheers

andrew



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] [patch] build issues on Win32

2010-03-10 Thread Dag-Erling Smørgrav
I've run across a couple of stumbling blocks when building on Win32
(specifically, XP + MinGW):

 1. PostgreSQL's private versions of inet_aton etc. can conflict with
similar functions in other libraries (in my case, PostgreSQL's
inet_aton conflicts with libavformat's).

 2. On Win32, PostgreSQL uses the SSPI authentication interface.  This
interface is implemented in secur32.dll, which needs to be added to
the linker command line.  This is apparently only an issue when
building a static libpq.

 3. src/template/win32 sets LDFLAGS unconditionally, overriding anything
the user may have specified in the environment or on the command
line (such as -L/path/to/my/lib/dir).

The attached patch addresses these issues by:

 1. #defining the function names as appropriate

 2. adding -lsecur32 to LIBS in src/Makefile.global.in when PORTNAME is
win32.

 3. adding ${LDFLAGS} at the front of the LDFLAGS redefinition in
src/template/win32.

The patch was developed and tested on 8.3.9, because that's what my
customer uses.  I have verified that it applies cleanly (albeit with
offsets) to 8.4.2.

BTW, IWBNI there were a quick and easy way to build and install only
libpq.  I use this sequence of commands (after configure):

$ make -C src/port all
$ make -C src/backend utils/fmgroids.h
$ make -C src/backend ../../src/include/utils/fmgroids.h
$ make -C src/include all install
$ make -C src/interfaces/libpq all install
$ make -C src/bin/pg_config all install

DES
-- 
Dag-Erling Smørgrav - d...@des.no

--- src/include/port.h.orig	2009-11-14 16:39:41.0 +0100
+++ src/include/port.h	2010-03-10 13:17:27.0 +0100
@@ -337,6 +337,7 @@
  * When necessary, these routines are provided by files in src/port/.
  */
 #ifndef HAVE_CRYPT
+#define crypt pq_crypt
 extern char *crypt(const char *key, const char *setting);
 #endif
 
@@ -351,44 +352,60 @@
 #endif
 
 #ifndef HAVE_GETOPT
+#define getopt pq_getopt
 extern int	getopt(int nargc, char *const * nargv, const char *ostr);
 #endif
 
 #ifndef HAVE_ISINF
+#define isinf pq_isinf
 extern int	isinf(double x);
 #endif
 
 #ifndef HAVE_RINT
+#define rint pq_rint
 extern double rint(double x);
 #endif
 
 #ifndef HAVE_INET_ATON
 #include netinet/in.h
 #include arpa/inet.h
+#define inet_aton pq_inet_aton
 extern int	inet_aton(const char *cp, struct in_addr * addr);
 #endif
 
 #ifndef HAVE_STRDUP
+#define strdup pq_strdup
 extern char *strdup(const char *str);
 #endif
 
+#ifndef HAVE_STRLCAT
+#define strlcat pq_strlcat
+#endif
+
 #if !HAVE_DECL_STRLCAT
 extern size_t strlcat(char *dst, const char *src, size_t siz);
 #endif
 
+#ifndef HAVE_STRLCPY
+#define strlcpy pq_strlcpy
+#endif
+
 #if !HAVE_DECL_STRLCPY
 extern size_t strlcpy(char *dst, const char *src, size_t siz);
 #endif
 
 #if !defined(HAVE_RANDOM)  !defined(__BORLANDC__)
+#define random pq_random
 extern long random(void);
 #endif
 
 #ifndef HAVE_UNSETENV
+#define unsetenv pq_unsetenv
 extern void unsetenv(const char *name);
 #endif
 
 #ifndef HAVE_SRANDOM
+#define srandom pq_srandom
 extern void srandom(unsigned int seed);
 #endif
 
--- src/Makefile.global.in.orig	2007-11-13 01:13:19.0 +0100
+++ src/Makefile.global.in	2010-03-10 13:42:04.0 +0100
@@ -435,9 +435,10 @@
 endif
 
 # to make ws2_32.lib the last library, and always link with shfolder,
-# so SHGetFolderName isn't picked up from shell32.dll
+# so SHGetFolderName isn't picked up from shell32.dll.  Also link
+# with secur32 for SSPI.
 ifeq ($(PORTNAME),win32)
-LIBS += -lws2_32 -lshfolder
+LIBS += -lws2_32 -lshfolder -lsecur32
 endif
 
 # Not really standard libc functions, used by the backend.
--- src/template/win32.orig	2004-11-08 06:23:26.0 +0100
+++ src/template/win32	2010-03-10 13:40:59.0 +0100
@@ -1,4 +1,4 @@
 # This is required to link pg_dump because it finds pg_toupper() in
 # libpq and pgport
-LDFLAGS=-Wl,--allow-multiple-definition
+LDFLAGS=${LDFLAGS} -Wl,--allow-multiple-definition
 

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [patch] build issues on Win32

2010-03-10 Thread Tom Lane
=?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= d...@des.no writes:
 I've run across a couple of stumbling blocks when building on Win32
 (specifically, XP + MinGW):

  1. PostgreSQL's private versions of inet_aton etc. can conflict with
 similar functions in other libraries (in my case, PostgreSQL's
 inet_aton conflicts with libavformat's).

So what?  We don't link with those libraries.  The proposed #defines
seem like a completely bad idea, especially since as-presented they
would affect every platform not only yours.  We don't need the
maintenance/debugging headaches of routines that don't have the names
they appear to have.

  ifeq ($(PORTNAME),win32)
 -LIBS += -lws2_32 -lshfolder
 +LIBS += -lws2_32 -lshfolder -lsecur32
  endif

Surely this bit would need to be conditional on whether libsecur32
is available?

 -LDFLAGS=-Wl,--allow-multiple-definition
 +LDFLAGS=${LDFLAGS} -Wl,--allow-multiple-definition

That bit seems sane.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] renameatt() can rename attribute of index, sequence, ...

2010-03-10 Thread Robert Haas
On Wed, Mar 3, 2010 at 7:16 PM, Robert Haas robertmh...@gmail.com wrote:
 2010/3/3 KaiGai Kohei kai...@ak.jp.nec.com:
 (2010/03/03 22:42), Robert Haas wrote:
 2010/3/3 KaiGai Koheikai...@ak.jp.nec.com:
 (2010/03/03 14:26), Robert Haas wrote:
 2010/3/2 KaiGai Koheikai...@ak.jp.nec.com:
 Is it an expected behavior?

    postgres=    CREATE SEQUENCE s;
    CREATE SEQUENCE
    postgres=    ALTER TABLE s RENAME sequence_name TO abcd;
    ALTER TABLE

    postgres=    CREATE TABLE t (a int primary key, b text);
    NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index 
 t_pkey for table t
    CREATE TABLE
    postgres=    ALTER TABLE t_pkey RENAME a TO xyz;
    ALTER TABLE

 The documentation says:
    http://developer.postgresql.org/pgdocs/postgres/sql-altertable.html

      :
    RENAME
      The RENAME forms change the name of a table (or an index, sequence, 
 or view) or
      the name of an individual column in a table. There is no effect on 
 the stored data.

 It seems to me the renameatt() should check relkind of the specified 
 relation, and
 raise an error if relkind != RELKIND_RELATION.

 Are we talking about renameatt() or RenameRelation()?  Letting
 RenameRelation() rename whatever seems fairly harmless; renameatt(),
 on the other hand, should probably refuse to allow this:

 CREATE SEQUENCE foo;
 ALTER TABLE foo RENAME COLUMN is_cycled TO bob;

 ...because that's just weird.  Tables, indexes, and views make sense,
 but the attributes of a sequence should be nailed down I think;
 they're basically system properties.

 I'm talking about renameatt(), not RenameRelation().

 OK.  Your original example was misleading because you had renameatt()
 in the subject line but the actual SQL commands were renaming a whole
 relation (which is a reasonable thing to do).

 If our perspective is these are a type of system properties, we should
 be able to reference these attributes with same name, so it is not harmless
 to allow renaming these attributes.

 I also agree that it makes sense to allow renaming attributes of tables
 and views. But I don't know whether it makes sense to allow it on indexs,
 like sequence and toast relations.

 I would think not.

 OK, the attached patch forbid renameatt() on relations expect for tables
 and views.

 OK, I will review it.

I don't think we should apply this as-is.  After some reflection, I
don't believe we should reject attribute renames on indices or
composite types.  The former is maybe useless but seems harmless, and
the latter seems affirmatively useful.

Also, I think that the comment about this would normally be done in
utility.c is no longer true and should be removed while we're
cleaning house.

...Robert

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [patch] build issues on Win32

2010-03-10 Thread Dag-Erling Smørgrav
Tom Lane t...@sss.pgh.pa.us writes:
 Dag-Erling Smørgrav d...@des.no writes:
   1. PostgreSQL's private versions of inet_aton etc. can conflict with
  similar functions in other libraries (in my case, PostgreSQL's
  inet_aton conflicts with libavformat's).
 So what?  We don't link with those libraries.

Your users might need to link with both.  I'm working on an application
that generates animations (specifically, animated weather forecasts)
based on data retrieved from a PostgreSQL database.

 The proposed #defines seem like a completely bad idea, especially
 since as-presented they would affect every platform not only yours.

Yes.  That's the idea.  This is a common idiom - the canonical way, if
you will, of solving this problem, which is not exclusive to PostgreSQL.
There are even cases in which you have no other choice, e.g. when the
function you want to use is available but does not work properly or does
not implement a particular feature that you need.

 We don't need the maintenance/debugging headaches of routines that
 don't have the names they appear to have.

Honestly, I don't see the problem.

   ifeq ($(PORTNAME),win32)
  -LIBS += -lws2_32 -lshfolder
  +LIBS += -lws2_32 -lshfolder -lsecur32
   endif
 Surely this bit would need to be conditional on whether libsecur32
 is available?

It's just as much part of Win32 as ws2_32 (winsock).

DES
-- 
Dag-Erling Smørgrav - d...@des.no

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [patch] build issues on Win32

2010-03-10 Thread Magnus Hagander
2010/3/10 Dag-Erling Smørgrav d...@des.no:
 Tom Lane t...@sss.pgh.pa.us writes:
 Dag-Erling Smørgrav d...@des.no writes:
   1. PostgreSQL's private versions of inet_aton etc. can conflict with
      similar functions in other libraries (in my case, PostgreSQL's
      inet_aton conflicts with libavformat's).
 So what?  We don't link with those libraries.

 Your users might need to link with both.  I'm working on an application
 that generates animations (specifically, animated weather forecasts)
 based on data retrieved from a PostgreSQL database.

This shows up only with static links of libpq, correct? Or are you
building a backend function?


 The proposed #defines seem like a completely bad idea, especially
 since as-presented they would affect every platform not only yours.

 Yes.  That's the idea.  This is a common idiom - the canonical way, if
 you will, of solving this problem, which is not exclusive to PostgreSQL.
 There are even cases in which you have no other choice, e.g. when the
 function you want to use is available but does not work properly or does
 not implement a particular feature that you need.

 We don't need the maintenance/debugging headaches of routines that
 don't have the names they appear to have.

 Honestly, I don't see the problem.

   ifeq ($(PORTNAME),win32)
  -LIBS += -lws2_32 -lshfolder
  +LIBS += -lws2_32 -lshfolder -lsecur32
   endif
 Surely this bit would need to be conditional on whether libsecur32
 is available?

 It's just as much part of Win32 as ws2_32 (winsock).

Yeah, it's a standard win32 library so that's not a problem.

But the fix seems wrong. If you are using a static libpq, the library
should be added whenever you link that library into your client
application. Not for every single EXE and DLL that postgres produces.
The makefiles already add it to the places in PostgreSQL where it's
needed - namely postgres.exe and libpq.dll.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [patch] build issues on Win32

2010-03-10 Thread Tom Lane
=?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= d...@des.no writes:
 Tom Lane t...@sss.pgh.pa.us writes:
 Dag-Erling Smørgrav d...@des.no writes:
 1. PostgreSQL's private versions of inet_aton etc. can conflict with
 similar functions in other libraries (in my case, PostgreSQL's
 inet_aton conflicts with libavformat's).

 So what?  We don't link with those libraries.

 Your users might need to link with both.

We don't support linking the backend into other applications.  If you're
complaining about libpq, the right thing for that is to use a platform
that can suppress non-exported symbols from a shared library.  Maybe
what we need to do is teach the mingw build path how to respect the
exports list for libpq?

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [patch] build issues on Win32

2010-03-10 Thread Dag-Erling Smørgrav
Magnus Hagander mag...@hagander.net writes:
  Dag-Erling Smørgrav d...@des.no writes:
  Your users might need to link with both.  I'm working on an
  application that generates animations (specifically, animated
  weather forecasts) based on data retrieved from a PostgreSQL
  database.
 This shows up only with static links of libpq, correct?

Yes.

 But the fix seems wrong. If you are using a static libpq, the library
 should be added whenever you link that library into your client
 application. Not for every single EXE and DLL that postgres produces.

Without this patch, pg_ctl fails to build...  and I don't see the logic
of including ws2_32 in LIBS but not secur32.  remember that LIBS
directly affects what pg_config reports, so if you don't add secur32 to
LIBS, consumers (applications) don't know that they need it.

DES
-- 
Dag-Erling Smørgrav - d...@des.no

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [patch] build issues on Win32

2010-03-10 Thread Dag-Erling Smørgrav
Tom Lane t...@sss.pgh.pa.us writes:
 We don't support linking the backend into other applications.

libpq uses this as well.

 If you're complaining about libpq, the right thing for that is to use
 a platform that can suppress non-exported symbols from a shared
 library.  Maybe what we need to do is teach the mingw build path how
 to respect the exports list for libpq?

If that works, I'm all for it.  I have no idea how to do it, though.

DES
-- 
Dag-Erling Smørgrav - d...@des.no

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [patch] build issues on Win32

2010-03-10 Thread Tom Lane
=?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= d...@des.no writes:
 Magnus Hagander mag...@hagander.net writes:
 But the fix seems wrong. If you are using a static libpq, the library
 should be added whenever you link that library into your client
 application. Not for every single EXE and DLL that postgres produces.

 Without this patch, pg_ctl fails to build...

It builds for everybody else (and we do have multiple mingw machines in
the buildfarm, so it's not like this doesn't get tested).  I think there
is some other factor involved here, and you need to identify what that
is.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [patch] build issues on Win32

2010-03-10 Thread Dag-Erling Smørgrav
Tom Lane t...@sss.pgh.pa.us writes:
 Dag-Erling Smørgrav d...@des.no writes:
  Without this patch, pg_ctl fails to build...
 It builds for everybody else (and we do have multiple mingw machines in
 the buildfarm, so it's not like this doesn't get tested).  I think there
 is some other factor involved here, and you need to identify what that
 is.

--disable-shared, as previously mentioned.

DES
-- 
Dag-Erling Smørgrav - d...@des.no

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [patch] build issues on Win32

2010-03-10 Thread Tom Lane
=?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= d...@des.no writes:
 Tom Lane t...@sss.pgh.pa.us writes:
 It builds for everybody else (and we do have multiple mingw machines in
 the buildfarm, so it's not like this doesn't get tested).  I think there
 is some other factor involved here, and you need to identify what that
 is.

 --disable-shared, as previously mentioned.

Oh.  Well, we don't really support that, and there is a proposal on the
table to remove it altogether from the configure script.  I don't think
we're going to contort our source code in order to make a marginal
improvement in the ability to coexist with random other code that is
also polluting the link-time namespace.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [patch] build issues on Win32

2010-03-10 Thread David Fetter
On Wed, Mar 10, 2010 at 01:28:55PM -0500, Tom Lane wrote:
 =?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= d...@des.no writes:
  Tom Lane t...@sss.pgh.pa.us writes:
  It builds for everybody else (and we do have multiple mingw
  machines in the buildfarm, so it's not like this doesn't get
  tested).  I think there is some other factor involved here, and
  you need to identify what that is.
 
  --disable-shared, as previously mentioned.
 
 Oh.  Well, we don't really support that, and there is a proposal on
 the table to remove it altogether from the configure script.  I
 don't think we're going to contort our source code in order to make
 a marginal improvement in the ability to coexist with random other
 code that is also polluting the link-time namespace.

+1 for de-supporting this option.

Cheers,
David.
-- 
David Fetter da...@fetter.org http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Access violation from palloc, Visual Studio 2005, C-language function

2010-03-10 Thread Tom Lane
Kevin Flanagan kevi...@linkprior.com writes:
 Hard to tell without seeing the actual code and a stack trace, but I'd
 bet that you haven't fully resolved the build process problems you
 mentioned earlier.  

 I've attached a zip of the (tiny) project, and a text file with the contents
 of the module containing the C-language functions. The only difference from
 sample code is that (as pointed out by Takahiro Itagaki in his post here of
 8th March) the function implementations need decorating with
 __declspec(dllexport).

Mph.  I don't actually believe that, nor do I believe the #define
BUILDING_DLL you put in, because neither of those are needed in any of
our contrib modules.  What I suspect at this point is that the reference
to CurrentMemoryContext in the palloc() macro is being bollixed by
having the wrong value for BUILDING_DLL.  However, not having a Windows
build environment to experiment with, I'll have to defer to somebody
with more experience in that.

(I wonder BTW if we should rename BUILDING_DLL, because it seems a bit
misnamed.  AIUI it's supposed to be set while building the core backend,
not while building loadable modules.)

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Re: Hot Standby query cancellation and Streaming Replication integration

2010-03-10 Thread Josh Berkus
On 3/10/10 3:38 AM, Greg Stark wrote:
 I think that means that a
 vacuum_defer_cleanup of up to about 100 or so (it depends on the width
 of your counter record) might be reasonable as a general suggestion
 but anything higher will depend on understanding the specific system.

100 wouldn't be useful at all.  It would increase bloat without doing
anything about query cancel except on a very lightly used system.

 With vacuum_defer_cleanup that will no longer be true.
 It will be as if you always have a query lasting n transactions in
 your system at all times.

Yep, but until we get XID-publish-to-master working in 9.1, I think it's
probably the best we can do.  At least it's no *worse* than having a
long-running query on the master at all times.

--Josh Berkus


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Access violation from palloc, Visual Studio 2005, C-language function

2010-03-10 Thread Kevin Flanagan
Aha. I'd read that the build process for the contrib modules involved
generating a .DEF file for the necessary exports. I had the impression that
defining BUILDING_DLL was an alternative, addressing (part) of the issue
(that is, PG_FUNCTION_INFO_V1 declares functions as 'extern PGDLLIMPORT',
and if you define BUILDING_DLL, then PGDLLIMPORT is defined as ' __declspec
(dllexport)'). But you're quite right, if I take out the BUILDING_DLL
definition, and put the __declspec (dllexport) stuff in piecemeal, the
access violation goes away. Thank goodness.

Thanks, that really helped me out.

Kevin.



-Original Message-
From: pgsql-hackers-ow...@postgresql.org
[mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tom Lane
Sent: 10 March 2010 18:51
To: Kevin Flanagan
Cc: 'PostgreSQL-development'
Subject: Re: [HACKERS] Access violation from palloc, Visual Studio 2005,
C-language function 

Kevin Flanagan kevi...@linkprior.com writes:
 Hard to tell without seeing the actual code and a stack trace, but I'd
 bet that you haven't fully resolved the build process problems you
 mentioned earlier.  

 I've attached a zip of the (tiny) project, and a text file with the
contents
 of the module containing the C-language functions. The only difference
from
 sample code is that (as pointed out by Takahiro Itagaki in his post here
of
 8th March) the function implementations need decorating with
 __declspec(dllexport).

Mph.  I don't actually believe that, nor do I believe the #define
BUILDING_DLL you put in, because neither of those are needed in any of
our contrib modules.  What I suspect at this point is that the reference
to CurrentMemoryContext in the palloc() macro is being bollixed by
having the wrong value for BUILDING_DLL.  However, not having a Windows
build environment to experiment with, I'll have to defer to somebody
with more experience in that.

(I wonder BTW if we should rename BUILDING_DLL, because it seems a bit
misnamed.  AIUI it's supposed to be set while building the core backend,
not while building loadable modules.)

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] gothic_moth, codlin_moth failures on REL8_2_STABLE

2010-03-10 Thread Tom Lane
Since the buildfarm is mostly green these days, I took some time to look
into the few remaining consistent failures.  One is that gothic_moth and
codlin_moth fail on contrib/tsearch2 in the 8.2 branch, with a
regression diff like this:

*** 2453,2459 
   body
   bSea/b view wow ubfoo/b bar/u iqq/i
   a href=http://www.google.com/foo.bar.html; target=_blankYES nbsp;/a
!   ff-bg
   script
  document.write(15);
   /script
--- 2453,2459 
   body
   bSea/b view wow ubfoo/b bar/u iqq/i
   a href=http://www.google.com/foo.bar.html; target=_blankYES nbsp;/a
!  ff-bgff-bg
   script
  document.write(15);
   /script

These animals are not testing any branches older than 8.2.  The same
test appears in newer branches and passes, but the code involved got
migrated to core and probably changed around a bit.

I traced through this test on my own machine and determined that the
way it's supposed to work is like this: the tsearch parser breaks the
string into a series of tokens that include these:

ff-bg   compound word
ff  compound word element
-   punctuation
bg  compound word element

The highlight function is then supposed to set skip = 1 on the compound
word, causing it to be skipped when genhl() reconstructs the text.
The failure looks to me like the compound word is not getting skipped.
Both the setting and the testing of the flag seem to be absolutely
straightforward portable code; although the skip struct field is a
bitfield, which is a C feature we don't use very heavily.

My conclusion is that this is probably a compiler bug.  Both buildfarm
animals appear to be using Sun Studio, although on different
architectures which weakens the compiler-bug theory a bit.  Even though
we are not seeing the failure in later PG branches, it would probably be
worth looking into more closely, because if it's biting 8.2 it could
start biting newer code as well.  But without access to a machine
showing the problem it is difficult to do much.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] renameatt() can rename attribute of index, sequence, ...

2010-03-10 Thread KaiGai Kohei
(2010/03/11 0:54), Robert Haas wrote:
 On Wed, Mar 3, 2010 at 7:16 PM, Robert Haasrobertmh...@gmail.com  wrote:
 2010/3/3 KaiGai Koheikai...@ak.jp.nec.com:
 (2010/03/03 22:42), Robert Haas wrote:
 2010/3/3 KaiGai Koheikai...@ak.jp.nec.com:
 (2010/03/03 14:26), Robert Haas wrote:
 2010/3/2 KaiGai Koheikai...@ak.jp.nec.com:
 Is it an expected behavior?

 postgres=  CREATE SEQUENCE s;
 CREATE SEQUENCE
 postgres=  ALTER TABLE s RENAME sequence_name TO abcd;
 ALTER TABLE

 postgres=  CREATE TABLE t (a int primary key, b text);
 NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index 
 t_pkey for table t
 CREATE TABLE
 postgres=  ALTER TABLE t_pkey RENAME a TO xyz;
 ALTER TABLE

 The documentation says:
 http://developer.postgresql.org/pgdocs/postgres/sql-altertable.html

   :
 RENAME
   The RENAME forms change the name of a table (or an index, 
 sequence, or view) or
   the name of an individual column in a table. There is no effect 
 on the stored data.

 It seems to me the renameatt() should check relkind of the specified 
 relation, and
 raise an error if relkind != RELKIND_RELATION.

 Are we talking about renameatt() or RenameRelation()?  Letting
 RenameRelation() rename whatever seems fairly harmless; renameatt(),
 on the other hand, should probably refuse to allow this:

 CREATE SEQUENCE foo;
 ALTER TABLE foo RENAME COLUMN is_cycled TO bob;

 ...because that's just weird.  Tables, indexes, and views make sense,
 but the attributes of a sequence should be nailed down I think;
 they're basically system properties.

 I'm talking about renameatt(), not RenameRelation().

 OK.  Your original example was misleading because you had renameatt()
 in the subject line but the actual SQL commands were renaming a whole
 relation (which is a reasonable thing to do).

 If our perspective is these are a type of system properties, we should
 be able to reference these attributes with same name, so it is not 
 harmless
 to allow renaming these attributes.

 I also agree that it makes sense to allow renaming attributes of tables
 and views. But I don't know whether it makes sense to allow it on indexs,
 like sequence and toast relations.

 I would think not.

 OK, the attached patch forbid renameatt() on relations expect for tables
 and views.

 OK, I will review it.
 
 I don't think we should apply this as-is.  After some reflection, I
 don't believe we should reject attribute renames on indices or
 composite types.  The former is maybe useless but seems harmless, and
 the latter seems affirmatively useful.

Indeed, it is useful to allow renaming attribute of composite types.

However, it is also useless to allow to rename attribute of sequences,
but harmless, like renames on indexes. It seems to me it is fair enough
to allow renaming attributes of tables, views and composite types...

 Also, I think that the comment about this would normally be done in
 utility.c is no longer true and should be removed while we're
 cleaning house.

Yes, when we rename attribute of the relations, it originally checks
ownership of the relation twice in ExecRenameStmt() and renameatt().
It should be reworked in the next.

Thanks,
-- 
KaiGai Kohei kai...@ak.jp.nec.com

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Re: Hot Standby query cancellation and Streaming Replication integration

2010-03-10 Thread Fujii Masao
On Wed, Mar 10, 2010 at 3:29 PM, Josh Berkus j...@agliodbs.com wrote:
 I've been playing with vacuum_defer_cleanup_age in reference to the
 query cancel problem.  It really seems to me that this is the way
 forward in terms of dealing with query cancel for normal operation
 rather than wal_standby_delay, or maybe in combination with it.

Why isn't vacuum_defer_cleanup_age listed on postgresql.conf.sample?
Though I also tried to test the effect of it, I was unable to find it
in the conf file.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Warning about invalid .pgpass passwords

2010-03-10 Thread Bruce Momjian
Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  The attached patch reports the fact that .pgpass was used if the libpq
  connection fails:
 
 The test is in a very inappropriate place --- it will be missed by
 several fully-supported code paths.

Agreed.  I moved it to the error return label (error_return) in
PQconnectPoll(), and placed the code in a new function.

  I am not sure if I like the parentheses or not.
 
 I don't like 'em.  If you don't have enough confidence in the message to

OK, parentheses removed.

 be replacing the normal error string, you probably shouldn't be doing
 this at all.  We'll look silly if we attach such a comment to a message
 that indicates a network failure, for example; and cases where the
 comment is actively misleading would be even worse.
 
 I'm inclined to think that maybe we should make the server return a
 distinct SQLSTATE for bad password, and have libpq check for that
 rather than just assuming that the failure must be bad password.
 The main argument against this is probably that it would tell a bad
 guy that he's got a valid username but not a valid password.  Not
 sure if that's a serious issue or not --- I seem to recall that we
 are exposing validity of user names already (or was that database
 names?).  It would also mean that the new message only triggers when
 talking to a 9.0+ server, but since we've gotten along without it
 till now, that aspect doesn't bother me at all.

Modifying the backend to issue this hint seems like overkill, unless we
have some other use for it.

 A compromise would be to check for
 ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION, which in combination
 with the knowledge that we got asked for a password would give
 fairly high confidence though not certainty that the problem is a bad
 password.

I originally considered using the SQLSTATE but found few uses of it in
the frontend code.  However, I agree it is the proper solution so I now
coded it that way, including a loop to convert from the 6-bit sqlstate
to base-10.  (FYI, the same C file hardcodes ERRCODE_APPNAME_UNKNOWN as
a string for historical reasons, but I didn't do that.)

Updated patch attached.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  PG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do
Index: src/interfaces/libpq/fe-connect.c
===
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.389
diff -c -c -r1.389 fe-connect.c
*** src/interfaces/libpq/fe-connect.c	3 Mar 2010 20:31:09 -	1.389
--- src/interfaces/libpq/fe-connect.c	11 Mar 2010 00:53:00 -
***
*** 71,76 
--- 71,78 
  
  #include libpq/ip.h
  #include mb/pg_wchar.h
+ #include utils/elog.h
+ #include utils/errcodes.h
  
  #ifndef FD_CLOEXEC
  #define FD_CLOEXEC 1
***
*** 284,289 
--- 286,292 
  static char *pwdfMatchesString(char *buf, char *token);
  static char *PasswordFromFile(char *hostname, char *port, char *dbname,
   char *username);
+ static void dot_pg_pass_warning(PGconn *conn);
  static void default_threadlock(int acquire);
  
  
***
*** 652,657 
--- 655,662 
  		conn-dbName, conn-pguser);
  		if (conn-pgpass == NULL)
  			conn-pgpass = strdup(DefaultPassword);
+ 		else
+ 			conn-dot_pgpass_used = true;
  	}
  
  	/*
***
*** 2133,2138 
--- 2138,2145 
  
  error_return:
  
+ 	dot_pg_pass_warning(conn);
+ 	
  	/*
  	 * We used to close the socket at this point, but that makes it awkward
  	 * for those above us if they wish to remove this socket from their own
***
*** 2191,2196 
--- 2198,2204 
  	conn-verbosity = PQERRORS_DEFAULT;
  	conn-sock = -1;
  	conn-password_needed = false;
+ 	conn-dot_pgpass_used = false;
  #ifdef USE_SSL
  	conn-allow_ssl_try = true;
  	conn-wait_ssl_try = false;
***
*** 4426,4431 
--- 4434,4471 
  #undef LINELEN
  }
  
+ 
+ /*
+  *	If the connection failed, we should mention if
+  *	we got the password from .pgpass in case that
+  *	password is wrong.
+  */
+ static void
+ dot_pg_pass_warning(PGconn *conn)
+ {
+ 	int invalid_authorization_specification_integer = 0;
+ 	int i;
+ 
+ 	if (!conn-dot_pgpass_used || !conn-password_needed)
+ 		return;	 /* quick exit */
+ 	
+ 	/* Convert 6-bit packed sqlstate to a base-10 integer */
+ 	for (i = 0; i  5; i++)
+ 	{
+ 		invalid_authorization_specification_integer *= 10;
+ 		invalid_authorization_specification_integer +=
+ 			(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION  (i * 6))  0x3F;
+ 	}
+ 
+ 	/* If it was 'invalid authorization', add .pgpass mention */
+ 	if (conn-result 
+ 		atoi(PQresultErrorField(conn-result, PG_DIAG_SQLSTATE)) ==
+ 			invalid_authorization_specification_integer)
+ 		appendPQExpBufferStr(conn-errorMessage,
+ 			libpq_gettext(password retrieved from .pgpass\n));
+ }
+ 
+ 	
  /*

Re: [HACKERS] Warning about invalid .pgpass passwords

2010-03-10 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 Tom Lane wrote:
 I'm inclined to think that maybe we should make the server return a
 distinct SQLSTATE for bad password, and have libpq check for that
 rather than just assuming that the failure must be bad password.

 Modifying the backend to issue this hint seems like overkill, unless we
 have some other use for it.

I wouldn't suggest it if I thought it were only helpful for this
particular message.  It seems to me that we've spent a lot of time
kluging around the lack of certainty about whether a connection failure
is a password issue.  Admittedly a lot of that was between libpq and its
client, but the state of affairs on the wire isn't great either.

I'm not convinced we have to do it that way, but now is definitely
the time to think about it before we implement yet another
sort-of-good-enough kluge.  Which is what this is.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Warning about invalid .pgpass passwords

2010-03-10 Thread Bruce Momjian
Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  Tom Lane wrote:
  I'm inclined to think that maybe we should make the server return a
  distinct SQLSTATE for bad password, and have libpq check for that
  rather than just assuming that the failure must be bad password.
 
  Modifying the backend to issue this hint seems like overkill, unless we
  have some other use for it.
 
 I wouldn't suggest it if I thought it were only helpful for this
 particular message.  It seems to me that we've spent a lot of time
 kluging around the lack of certainty about whether a connection failure
 is a password issue.  Admittedly a lot of that was between libpq and its
 client, but the state of affairs on the wire isn't great either.

Yes, I have seen that myself in psql.

 I'm not convinced we have to do it that way, but now is definitely
 the time to think about it before we implement yet another
 sort-of-good-enough kluge.  Which is what this is.

True.  Should we just hold this all for 9.1 or should I code it and
let's look at the size of the patch?

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  PG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Re: Hot Standby query cancellation and Streaming Replication integration

2010-03-10 Thread Bruce Momjian
Fujii Masao wrote:
 On Wed, Mar 10, 2010 at 3:29 PM, Josh Berkus j...@agliodbs.com wrote:
  I've been playing with vacuum_defer_cleanup_age in reference to the
  query cancel problem. ?It really seems to me that this is the way
  forward in terms of dealing with query cancel for normal operation
  rather than wal_standby_delay, or maybe in combination with it.
 
 Why isn't vacuum_defer_cleanup_age listed on postgresql.conf.sample?
 Though I also tried to test the effect of it, I was unable to find it
 in the conf file.

I asked about that last week and for some reason Simon didn't want it
added and Greg Smith was going to get this corrected.  Greg?

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  PG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] operator exclusion constraints

2010-03-10 Thread Tom Lane
Awhile back I wrote:
 * I'm not too satisfied with the behavior of psql's \d:

 regression=# create table foo (f1 int primary key using index tablespace ts1,
 regression(# f2 int, EXCLUDE USING btree (f2 WITH =) using index tablespace 
 ts1,
 regression(# f3 int, EXCLUDE USING btree (f3 WITH =) DEFERRABLE INITIALLY 
 DEFERRED);
 NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index foo_pkey for 
 table foo
 NOTICE:  CREATE TABLE / EXCLUDE will create implicit index foo_f2_exclusion 
 for table foo
 NOTICE:  CREATE TABLE / EXCLUDE will create implicit index foo_f3_exclusion 
 for table foo
 CREATE TABLE
 regression=# \d foo
   Table public.foo
  Column |  Type   | Modifiers 
 +-+---
  f1 | integer | not null
  f2 | integer | 
  f3 | integer | 
 Indexes:
 foo_pkey PRIMARY KEY, btree (f1), tablespace ts1
 foo_f2_exclusion btree (f2), tablespace ts1
 foo_f3_exclusion btree (f3) DEFERRABLE INITIALLY DEFERRED
 Exclusion constraints:
 foo_f2_exclusion EXCLUDE USING btree (f2 WITH =)
 foo_f3_exclusion EXCLUDE USING btree (f3 WITH =) DEFERRABLE INITIALLY 
 DEFERRED

 regression=# 

 This might have been defensible back when the idea was to keep constraints
 decoupled from indexes, but now it just looks bizarre.  We should either
 get rid of the Exclusion constraints: display and attach the info to
 the index entries, or hide indexes that are attached to exclusion
 constraints.  I lean to the former on the grounds of the precedent for
 unique/pkey indexes --- which is not totally arbitrary, since an index
 is usable as a query index regardless of its function as a constraint.
 It's probably a debatable point though.

Attached is a patch against HEAD that folds exclusion constraints into
\d's regular indexes list.  With this, the above example produces

  Table public.foo
 Column |  Type   | Modifiers 
+-+---
 f1 | integer | not null
 f2 | integer | 
 f3 | integer | 
Indexes:
foo_pkey PRIMARY KEY, btree (f1), tablespace ts1
foo_f2_exclusion EXCLUDE USING btree (f2 WITH =), tablespace ts1
foo_f3_exclusion EXCLUDE USING btree (f3 WITH =) DEFERRABLE INITIALLY 
DEFERRED

Any objections?

regards, tom lane

? psql
Index: describe.c
===
RCS file: /cvsroot/pgsql/src/bin/psql/describe.c,v
retrieving revision 1.240
diff -c -r1.240 describe.c
*** describe.c	11 Mar 2010 04:36:43 -	1.240
--- describe.c	11 Mar 2010 05:18:28 -
***
*** 1105, 
  		bool		hasrules;
  		bool		hastriggers;
  		bool		hasoids;
- 		bool		hasexclusion;
  		Oid			tablespace;
  		char	   *reloptions;
  		char	   *reloftype;
--- 1105,1110 
***
*** 1128,1135 
  		printfPQExpBuffer(buf,
  			  SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, 
  		  c.relhastriggers, c.relhasoids, 
! 		  %s, c.reltablespace, c.relhasexclusion, 
! 		  CASE WHEN c.reloftype = 0 THEN '' ELSE c.reloftype::pg_catalog.regtype::text END\n
  		  FROM pg_catalog.pg_class c\n 
  		   LEFT JOIN pg_catalog.pg_class tc ON (c.reltoastrelid = tc.oid)\n
  		  WHERE c.oid = '%s'\n,
--- 1127,1134 
  		printfPQExpBuffer(buf,
  			  SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, 
  		  c.relhastriggers, c.relhasoids, 
! 		  %s, c.reltablespace, 
! 		  CASE WHEN c.reloftype = 0 THEN '' ELSE c.reloftype::pg_catalog.regtype::pg_catalog.text END\n
  		  FROM pg_catalog.pg_class c\n 
  		   LEFT JOIN pg_catalog.pg_class tc ON (c.reltoastrelid = tc.oid)\n
  		  WHERE c.oid = '%s'\n,
***
*** 1207,1216 
  		strdup(PQgetvalue(res, 0, 6)) : 0;
  	tableinfo.tablespace = (pset.sversion = 8) ?
  		atooid(PQgetvalue(res, 0, 7)) : 0;
! 	tableinfo.hasexclusion = (pset.sversion = 9) ?
! 		strcmp(PQgetvalue(res, 0, 8), t) == 0 : false;
! 	tableinfo.reloftype = (pset.sversion = 9  strcmp(PQgetvalue(res, 0, 9), ) != 0) ?
! 		strdup(PQgetvalue(res, 0, 9)) : 0;
  	PQclear(res);
  	res = NULL;
  
--- 1206,1213 
  		strdup(PQgetvalue(res, 0, 6)) : 0;
  	tableinfo.tablespace = (pset.sversion = 8) ?
  		atooid(PQgetvalue(res, 0, 7)) : 0;
! 	tableinfo.reloftype = (pset.sversion = 9  strcmp(PQgetvalue(res, 0, 8), ) != 0) ?
! 		strdup(PQgetvalue(res, 0, 8)) : 0;
  	PQclear(res);
  	res = NULL;
  
***
*** 1545,1571 
  appendPQExpBuffer(buf, i.indisvalid, );
  			else
  appendPQExpBuffer(buf, true as indisvalid, );
! 			appendPQExpBuffer(buf, pg_catalog.pg_get_indexdef(i.indexrelid, 0, true));
  			if (pset.sversion = 9)
  appendPQExpBuffer(buf,
!   ,\n  (NOT i.indimmediate) AND 
!   EXISTS (SELECT 1 FROM pg_catalog.pg_constraint 
!   WHERE conrelid = i.indrelid AND 
!   conindid = i.indexrelid AND 
!   contype IN ('p','u','x') AND 
!   condeferrable) AS condeferrable
!   ,\n  (NOT 

Re: [HACKERS] Re: Hot Standby query cancellation and Streaming Replication integration

2010-03-10 Thread Josh Berkus

 Why isn't vacuum_defer_cleanup_age listed on postgresql.conf.sample?
 Though I also tried to test the effect of it, I was unable to find it
 in the conf file.

Using it has some bugs we need to clean up, apparently.

--Josh Berkus


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Type Mismatch Error in Set Returning Functions

2010-03-10 Thread Noel Proffitt
As reported in pgsql-bugs, in 9, a set returning function will raise an
error Returned type .. does not match expected type ..  when the
source type does not exactly match the target type. For example
VARCHAR(3) to VARCHAR(4) or NUMERIC(4,2) to NUMERIC(5,2). Previously,
this was not an issue.

It was pointed out in pgsql-bugs that this new behavior was expected and
the result of the logic used by ConvertRowtypeExpr. The old behavior is
considered wrong. 

To me, it seems like in most other parts of Pg types are
cast sensibly without complaint. For example, in 9.0 and 8.4 we can do things 
like:

  CREATE TABLE foo (n NUMERIC(10,2));
  INSERT INTO foo values (42.77::NUMERIC(12,2));
  INSERT INTO foo values (42.77::NUMERIC(8,2));
  INSERT INTO foo values (42.77::NUMERIC(14,8));
  SELECT * FROM foo 
  JOIN (VALUES ( 42.78::NUMERIC(5,3) )) AS bar(m) ON foo.n = bar.m;

The values are rounded and cast; Same with varchar of various sizes. 
Also note that returning a table with a different type still works in 9..

CREATE TABLE a_table ( val VARCHAR(3) );
INSERT INTO a_table VALUES ('abc');

CREATE FUNCTION check_varchar() RETURNS
TABLE (val VARCHAR(4)) AS
$$
DECLARE
BEGIN
  SELECT * INTO val FROM a_table;
  RETURN NEXT;
  RETURN;
END;
$$ LANGUAGE 'plpgsql' IMMUTABLE;
SELECT * FROM check_varchar();

-- above works in pg 9
-- while the more traditional function returning SETOF does not..

CREATE TABLE b_table ( val VARCHAR(4) );
DROP FUNCTION check_varchar();
CREATE FUNCTION check_varchar() RETURNS SETOF b_table AS
$$
DECLARE
  myrec RECORD;
BEGIN
  SELECT * INTO myrec FROM a_table;
  RETURN NEXT myrec;
  RETURN;
END;
$$ LANGUAGE 'plpgsql' IMMUTABLE;
SELECT * FROM check_varchar();

Regards,
-Noel Proffitt


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers