[HACKERS] memory leak in libxml2 - fix

2010-11-26 Thread Pavel Stehule
Hello

our customer showed a very significant memory leak in xml2.

try to

select xpath_number('data' || generate_series || '/data','/data')
from generate_series(1,50);

attention! take all memory very fast

It never release a memory allocated for context and doctree.

Regards

Pavel Stehule
*** ./xpath.c.orig	2010-03-03 20:10:29.0 +0100
--- ./xpath.c	2010-11-26 09:50:42.492451256 +0100
***
*** 53,58 
--- 53,61 
  
  static xmlXPathObjectPtr pgxml_xpath(text *document, xmlChar *xpath);
  
+ /* should be global, to be realeased */
+ static xmlDocPtr	doctree;
+ static xmlXPathContextPtr ctxt;
  
  /*
   * Initialize for xml parsing.
***
*** 238,243 
--- 241,250 
  
  	if (xpres == NULL)
  		PG_RETURN_NULL();
+ 
+ 	xmlXPathFreeContext(ctxt);
+ 	xmlFreeDoc(doctree);
+ 
  	PG_RETURN_TEXT_P(xpres);
  }
  
***
*** 272,277 
--- 279,288 
  
  	if (xpres == NULL)
  		PG_RETURN_NULL();
+ 
+ 	xmlXPathFreeContext(ctxt);
+ 	xmlFreeDoc(doctree);
+ 
  	PG_RETURN_TEXT_P(xpres);
  }
  
***
*** 310,315 
--- 321,330 
  
  	if (xpres == NULL)
  		PG_RETURN_NULL();
+ 
+ 	xmlXPathFreeContext(ctxt);
+ 	xmlFreeDoc(doctree);
+ 
  	PG_RETURN_TEXT_P(xpres);
  }
  
***
*** 341,346 
--- 356,365 
  
  	fRes = xmlXPathCastToNumber(res);
  
+ 	/* release a used context and document */
+ 	xmlXPathFreeContext(ctxt);
+ 	xmlFreeDoc(doctree);
+ 
  	if (xmlXPathIsNaN(fRes))
  		PG_RETURN_NULL();
  
***
*** 375,380 
--- 394,402 
  
  	bRes = xmlXPathCastToBoolean(res);
  
+ 	xmlXPathFreeContext(ctxt);
+ 	xmlFreeDoc(doctree);
+ 
  	PG_RETURN_BOOL(bRes);
  }
  
***
*** 385,392 
  static xmlXPathObjectPtr
  pgxml_xpath(text *document, xmlChar *xpath)
  {
- 	xmlDocPtr	doctree;
- 	xmlXPathContextPtr ctxt;
  	xmlXPathObjectPtr res;
  	xmlXPathCompExprPtr comppath;
  	int32		docsize;
--- 407,412 

-- 
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] [JDBC] JDBC and Binary protocol error, for some statements

2010-11-26 Thread Radosław Smogura
This what I done is 
1. Send bind
2. Put on stack the requested format types
3. On bind complete get requestedFormats from stack.
4. When execute is complete (or portal suspend) then, use requestedFormats
to change the field formats received from describe or previously cached.

I assume server can't change formats after bind, even the describe portal
was fired. Is it all good? I don't know much about server internals.

Kind regards,
Radek.


On Fri, 26 Nov 2010 01:02:25 -0500, Tom Lane t...@sss.pgh.pa.us wrote:
 Maciek Sakrejda msakre...@truviso.com writes:
 21:43:02.264 (26)  FE= Describe(statement=S_1)
 You're still doing the statement-flavor Describe. As Tom pointed out,
 this won't tell you the result types because it doesn't know them.
 Actually, technically if you issue a statement-flavor Describe *after*
 a Bind, the server does have this information, but I'm not surprised
 that it doesn't send it correctly, since it seems pointless to send
 the statement variation after already doing a Bind.
 
 In principle you could open more than one Portal off a Statement
 at the same time, so it wouldn't necessarily be well-defined anyway.
 
   regards, tom lane

-- 
--
Radosław Smogura
http://www.softperience.eu

-- 
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] memory leak in libxml2 - fix

2010-11-26 Thread Itagaki Takahiro
On Fri, Nov 26, 2010 at 17:59, Pavel Stehule pavel.steh...@gmail.com wrote:
 our customer showed a very significant memory leak in xml2.
 It never release a memory allocated for context and doctree.

Why did you change doctree and ctxt to global variables?
I'm not sure why /* xmlFreeDoc(doctree); */ is commented out
at the end of pgxml_xpath(), but is it enough to enable the code?

-- 
Itagaki Takahiro

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


[HACKERS] ToDo: enhanced diagnostic for plpgsql

2010-11-26 Thread Pavel Stehule
Hello

there is difficult working with exception still. Isn't time to move forward?

http://archives.postgresql.org/pgsql-general/2010-11/msg01129.php

I am thinking, so we can little bit enhance a GET DIAGNOSTICS statement

condition item names are defined in SQL 92 - mainly: TABLE_NAME,
COLUM_NAME and MESSAGE_TEXT.

Regards

Pavel Stehule

-- 
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] memory leak in libxml2 - fix

2010-11-26 Thread Pavel Stehule
2010/11/26 Itagaki Takahiro itagaki.takah...@gmail.com:
 On Fri, Nov 26, 2010 at 17:59, Pavel Stehule pavel.steh...@gmail.com wrote:
 our customer showed a very significant memory leak in xml2.
 It never release a memory allocated for context and doctree.

 Why did you change doctree and ctxt to global variables?
 I'm not sure why /* xmlFreeDoc(doctree); */ is commented out
 at the end of pgxml_xpath(), but is it enough to enable the code?


I am thinking, so you must not to call xmlFreeDoc(doctree) early.
Probably xmlXPathCastToXXX reading a doctree.

xmlXPathFreeContext(ctxt); xmlFreeDoc(doctree); are called now only
when function doesn't return a value.

Regards

Pavel Stehule


 --
 Itagaki Takahiro


-- 
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] pg_execute_from_file review

2010-11-26 Thread Dimitri Fontaine
Joshua Tolley eggyk...@gmail.com writes:
  * I'd like to see the docs slightly expanded, specifically to describe
parameter replacement. I wondered for a while if I needed to set of
parameters in any specific way, before reading the code and realizing 
  they
can be whatever I want.
 
 My guess is that you knew that in the CREATE EXTENSION context, it has
 been proposed to use the notation @extschema@ as a placeholder, and
 you've then been confused. I've refrained from imposing any style with
 respect to what the placeholder would look like in the mecanism-patch.
 
 Do we still want to detail in the docs that there's nothing expected
 about the placeholder syntax of format?

 Perhaps such docs will show up with the rest of the EXTENSION work, but I'd
 like a brief mention somewhere.

I'm unclear about how to spell out what you'd like to be seen in there,
so I'm not proposing a newer version of the patch.

  * Shouldn't it include SPI_push() and SPI_pop()?
 
 ENOCLUE

 My guess is yes, because that was widely hailed as a good idea when I did
 PL/LOLCODE. I suspect it would only matter if someone were using
 pg_execute_from_file within some other function, which isn't entirely
 unlikely.

In fact, per the fine manual, it seems unnecessary doing so when using
SPI_execute(), which is the case here:

  http://www.postgresql.org/docs/9.0/interactive/spi-spi-push.html

  Note that SPI_execute and related functions automatically do the
  equivalent of SPI_push before passing control back to the SQL
  execution engine, so it is not necessary for you to worry about this
  when using those functions.

For information, we've been talking about the case on IRC and Joshua and
we are agreeing on this conclusion.

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

-- 
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] contrib: auth_delay module

2010-11-26 Thread Robert Haas
On Thu, Nov 25, 2010 at 9:54 PM, KaiGai Kohei kai...@ak.jp.nec.com wrote:
 I'll revise my patch. How about _PG_fini()?

In modules like auto_explain and pg_stat_statements, we have some
token code in there to handle unload, but I don't believe there's any
way to invoke it, nor would it work if there were multiple users of
the hook.  passwordcheck, on the other hand, has no _PG_fini, which
seems like the more sensible approach since it isn't going to work
anyway.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] Assertion failure on hot standby

2010-11-26 Thread Robert Haas
On Fri, Nov 26, 2010 at 1:11 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Simon Riggs si...@2ndquadrant.com writes:
 That would mean running GetCurrentTransactionId() inside LockAcquire()

 if (lockmode = AccessExclusiveLock 
     locktag-locktag_type == LOCKTAG_RELATION 
     !RecoveryInProgress())
       (void) GetCurrentTransactionId();

 Any objections to that fix?

 Could we have a wal level test in there too please?  It's pretty awful
 in any case...

+1.

Incidentally, I haven't been able to wrap my head around why we need
to propagate AccessExclusiveLocks to the standby in the first place.
Can someone explain?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] Assertion failure on hot standby

2010-11-26 Thread Andres Freund
On Friday 26 November 2010 13:32:18 Robert Haas wrote:
 On Fri, Nov 26, 2010 at 1:11 AM, Tom Lane t...@sss.pgh.pa.us wrote:
  Simon Riggs si...@2ndquadrant.com writes:
  That would mean running GetCurrentTransactionId() inside LockAcquire()
  
  if (lockmode = AccessExclusiveLock 
  locktag-locktag_type == LOCKTAG_RELATION 
  !RecoveryInProgress())
(void) GetCurrentTransactionId();
  
  Any objections to that fix?
  
  Could we have a wal level test in there too please?  It's pretty awful
  in any case...
 Incidentally, I haven't been able to wrap my head around why we need
 to propagate AccessExclusiveLocks to the standby in the first place.
 Can someone explain?
To make the standby stop applying WAL when a local transaction on the standby 
uses an object.
E.g. dropping a table on the master need the standby top stop applying wal (or 
kill the local client using the table).
How would you want to protect against something like that otherwise?

Andres

-- 
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] improving foreign key locks

2010-11-26 Thread Florian Pflug
On Nov25, 2010, at 23:01 , Alvaro Herrera wrote:
 So I've been working on improving locks for foreign key checks, as
 discussed in a thread started by Joel Jacobson a while ago.  I've posted
 about this:
 http://www.commandprompt.com/blogs/alvaro_herrera/2010/11/fixing_foreign_key_deadlocks/
 http://www.commandprompt.com/blogs/alvaro_herrera/2010/11/fixing_foreign_key_deadlocks_part_2/

To me, the whole thing seems to be special case of allowing to not only lock 
whole tuples FOR UPDATE or FOR SHARE, but also individual fields or sets of 
fields. Except that for simplicity, only two sets are supported, which are
  A) All fields
  B) All fields which are included in some unique constraint, including primary 
keys.

I'd therefore suggest to extend the FOR SHARE / FOR UPDATE syntax to be 
  SELECT FOR { SHARE | UPDATE } [ OF table1[.field1], ... ]
and obtain what you call a KEY LOCK if (for a particular table) the set of 
fields is a subset of (B). Otherwise, we'd obtain a full SHARE lock. Thus we'd 
always lock at least the fields the user told us to, but sometimes more than 
those, for the sake of a more efficient implementation.

This leads quite naturally to the following behaviour regarding lock conflicts
.) An UPDATE conflicts with a SHARE or UPDATE lock if the update touches fields 
locked by the SHARE or UPDATE lock. Thus, in case (A) they always conflict 
while in case (B) they only conflict if the UPDATE touches a field contained in 
some unique index
.) A DELETE conflicts always conflicts with a SHARE or UPDATE lock since it, in 
a way, touches all fields
.) A UPDATE lock always conflicts with a SHARE lock, since there are no two 
non-overlapping sets of fields that we could lock.
.) SHARE locks never conflict with one another.

best regards,
Florian Pflug


-- 
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] duplicate connection failure messages

2010-11-26 Thread Peter Eisentraut
On lör, 2010-11-20 at 18:07 -0500, Bruce Momjian wrote:
 The output is as expected:
 
   $ psql -h localhost test
   psql: could not connect to server: Connection refused
   Is the server running on host localhost (127.0.0.1) and 
 accepting
   TCP/IP connections on port 5432?
   $ psql -h 127.0.0.1 test
   psql: could not connect to server: Connection refused
   Is the server running on host 127.0.0.1 and accepting
   TCP/IP connections on port 5432?

Thanks for working on this.  However, the example I posted at the
beginning of this thread now does this:

$ ./psql -p 5 -h localhost
psql: could not connect to server: Connection refused
Is the server running on host localhost (???) and accepting
TCP/IP connections on port 5?
could not connect to server: Connection refused
Is the server running on host localhost (127.0.0.1) and accepting
TCP/IP connections on port 5?

The ??? should presumably be ::1.

Also, this comment should be updated:

/*
 * Try to initiate a connection to one of the addresses
 * returned by pg_getaddrinfo_all().  conn-addr_cur is the
 * next one to try. We fail when we run out of addresses
 * (reporting the error returned for the *last* alternative,
 * which may not be what users expect :-().
 */



-- 
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] duplicate connection failure messages

2010-11-26 Thread Alvaro Herrera
Excerpts from Peter Eisentraut's message of vie nov 26 11:06:24 -0300 2010:

 Thanks for working on this.  However, the example I posted at the
 beginning of this thread now does this:
 
 $ ./psql -p 5 -h localhost
 psql: could not connect to server: Connection refused
 Is the server running on host localhost (???) and accepting
 TCP/IP connections on port 5?

Shouldn't connectFailureMessage receive addr_cur as parameter?
Otherwise it's not clear that it's getting the right params to report.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] Assertion failure on hot standby

2010-11-26 Thread Robert Haas
On Fri, Nov 26, 2010 at 7:41 AM, Andres Freund and...@anarazel.de wrote:
 On Friday 26 November 2010 13:32:18 Robert Haas wrote:
 On Fri, Nov 26, 2010 at 1:11 AM, Tom Lane t...@sss.pgh.pa.us wrote:
  Simon Riggs si...@2ndquadrant.com writes:
  That would mean running GetCurrentTransactionId() inside LockAcquire()
 
  if (lockmode = AccessExclusiveLock 
      locktag-locktag_type == LOCKTAG_RELATION 
      !RecoveryInProgress())
        (void) GetCurrentTransactionId();
 
  Any objections to that fix?
 
  Could we have a wal level test in there too please?  It's pretty awful
  in any case...
 Incidentally, I haven't been able to wrap my head around why we need
 to propagate AccessExclusiveLocks to the standby in the first place.
 Can someone explain?
 To make the standby stop applying WAL when a local transaction on the standby
 uses an object.
 E.g. dropping a table on the master need the standby top stop applying wal (or
 kill the local client using the table).
 How would you want to protect against something like that otherwise?

Hmm.  But it seems like that it would be enough to log any exclusive
locks held at commit time, rather than logging them as they're
acquired.  By then, the XID will be assigned (if you need it - if you
don't then you probably don't need to XLOG it anyway) and you avoid
holding the lock for more than a moment on the standby.

But it seems like an even better idea would be to actually XLOG the
operations that are problematic specifically.  Because, for example,
if a user session on the master does LOCK TABLE ... IN ACCESS
EXCLUSIVE MODE, AFAICS there's no reason for the standby to care.  Or
am I confused?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] Assertion failure on hot standby

2010-11-26 Thread Simon Riggs
On Fri, 2010-11-26 at 11:19 +0900, Fujii Masao wrote:

 On Fri, Nov 26, 2010 at 7:40 AM, Simon Riggs si...@2ndquadrant.com wrote:
  As to solutions, it cannot be acceptable to ignore some locks just
  because an xid has not been assigned.
 
 Even if GetRunningTransactionLocks ignores such a lock, it's eventually
 WAL-logged by LogAccessExclusiveLock, isn't it?

If it were true always, I would much prefer your solution.

Assuming that would then cause a race condition between the logging of
the RunningXactsData and the lock, which wouldn't move us forwards.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services
 


-- 
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] Assertion failure on hot standby

2010-11-26 Thread Heikki Linnakangas

On 26.11.2010 17:28, Robert Haas wrote:

On Fri, Nov 26, 2010 at 7:41 AM, Andres Freundand...@anarazel.de  wrote:

On Friday 26 November 2010 13:32:18 Robert Haas wrote:

On Fri, Nov 26, 2010 at 1:11 AM, Tom Lanet...@sss.pgh.pa.us  wrote:

Simon Riggssi...@2ndquadrant.com  writes:

That would mean running GetCurrentTransactionId() inside LockAcquire()

if (lockmode= AccessExclusiveLock
 locktag-locktag_type == LOCKTAG_RELATION
 !RecoveryInProgress())
   (void) GetCurrentTransactionId();

Any objections to that fix?


Could we have a wal level test in there too please?  It's pretty awful
in any case...

Incidentally, I haven't been able to wrap my head around why we need
to propagate AccessExclusiveLocks to the standby in the first place.
Can someone explain?

To make the standby stop applying WAL when a local transaction on the standby
uses an object.
E.g. dropping a table on the master need the standby top stop applying wal (or
kill the local client using the table).
How would you want to protect against something like that otherwise?


Hmm.  But it seems like that it would be enough to log any exclusive
locks held at commit time, rather than logging them as they're
acquired.  By then, the XID will be assigned (if you need it - if you
don't then you probably don't need to XLOG it anyway) and you avoid
holding the lock for more than a moment on the standby.

But it seems like an even better idea would be to actually XLOG the
operations that are problematic specifically.  Because, for example,
if a user session on the master does LOCK TABLE ... IN ACCESS
EXCLUSIVE MODE, AFAICS there's no reason for the standby to care.  Or
am I confused?


Let's approach this from a different direction:

If you have operation A in the master that currently acquires an 
AccessExclusiveLock on a table, do you think it's safe for another 
transaction to peek at the table at the same time? Say, run a heap scan 
simultaneously. If yes, why did you take an AccessExclusiveLock in the 
first place. If not, then surely it's not safe to have a heap scan 
running against the table in the standby either. The read-only query in 
the standby sees the same actions as a read-only query on the master 
would see.


You can only take AccessShareLocks on the standby, and the only locks 
that conflict with an AccessShareLock is the AccessExclusiveLock. (Hmm, 
looking at the code, we also allow RowShareLock and RowExclusiveLock in 
the standby. You can't actually insert/update/delete tuples or set xmax 
as SELECT FOR SHARE does on standby, though, so why do we allow that? )


As a concrete example, VACUUM acquires an AccessExclusiveLock when it 
wants to truncate the relation. A sequential scan running against the 
table in the standby will get upset, if the startup process replays a 
truncation record on the table without warning.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.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] Assertion failure on hot standby

2010-11-26 Thread Simon Riggs
On Fri, 2010-11-26 at 17:53 +0200, Heikki Linnakangas wrote:
 Hmm, 
 looking at the code, we also allow RowShareLock and RowExclusiveLock
 in 
 the standby. You can't actually insert/update/delete tuples or set
 xmax 
 as SELECT FOR SHARE does on standby, though, so why do we allow that? 

It was considered sensible to allow PREPARE on DML on a standby, which
required those lock levels. Archives...

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services
 


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


[HACKERS] libpq/Makefile cleanup abandoned

2010-11-26 Thread Bruce Momjian
I thought I could improve the way libpq/Makefile pulls in C files, but
it turns out MSVC scrapes the OBJS lines from that file, so I had to
revert most of my cleanup, attached.

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

  + It's impossible for everything to be true. +
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 2359491..ceaadc6 100644
*** /tmp/on3x1b_Makefile	Fri Nov 26 11:08:58 2010
--- src/interfaces/libpq/Makefile	Fri Nov 26 11:05:12 2010
*** endif
*** 29,57 
  # platforms require special flags.
  LIBS := $(LIBS:-lpgport=)
  
! # libpgport C files that are always used by libpq
! PGPORT = inet_net_ntop noblock pgstrcasecmp thread
! ifeq ($(PORTNAME), win32)
! PGPORT += pgsleep
! endif
! # libpgport C files are used by libpq if identified by configure
! PGPORT += $(basename $(filter $(addsuffix .o, crypt getaddrinfo inet_aton open snprintf strerror strlcpy win32error), $(LIBOBJS)))
! 
! # other external C files
! BACKEND_LIBPQ = md5 ip
! UTILS_MB = encnames wchar
! 
  OBJS=	fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o \
  	fe-protocol2.o fe-protocol3.o pqexpbuffer.o pqsignal.o fe-secure.o \
  	libpq-events.o \
! 	$(addsuffix .o, $(PGPORT) $(BACKEND_LIBPQ) $(UTILS_MB))
  
  ifeq ($(PORTNAME), cygwin)
  override shlib = cyg$(NAME)$(DLSUFFIX)
  endif
  
  ifeq ($(PORTNAME), win32)
! OBJS += win32.o libpqrc.o
  
  libpqrc.o: libpq.rc
  	$(WINDRES) -i $ -o $@
--- 29,54 
  # platforms require special flags.
  LIBS := $(LIBS:-lpgport=)
  
! # 'filter' is used for libpgport C files that are needed by libpq if
! # identified by configure, and we optionally add pgsleep.o below.
! # We can't use Makefile variables here because the MSVC build system scrapes
! # OBJS from this file.
! # The last two lines come from backend/libpq and utils/mb.
  OBJS=	fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o \
  	fe-protocol2.o fe-protocol3.o pqexpbuffer.o pqsignal.o fe-secure.o \
  	libpq-events.o \
! 	inet_net_ntop.o noblock.o pgstrcasecmp.o thread.o \
! 	$(filter crypt.o getaddrinfo.o inet_aton.o open.o snprintf.o strerror.o strlcpy.o win32error.o, $(LIBOBJS)) \
! 	ip.o md5.o \
! 	encnames.o wchar.o
  
  ifeq ($(PORTNAME), cygwin)
  override shlib = cyg$(NAME)$(DLSUFFIX)
  endif
  
  ifeq ($(PORTNAME), win32)
! # pgsleep.o is from libpgport
! OBJS += pgsleep.o win32.o libpqrc.o
  
  libpqrc.o: libpq.rc
  	$(WINDRES) -i $ -o $@
*** backend_src = $(top_srcdir)/src/backend
*** 86,100 
  # We use several backend modules verbatim, but since we need to
  # compile with appropriate options to build a shared lib, we can't
  # necessarily use the same object files as the backend uses. Instead,
! # we symlink the source files in here and build our own object files.
  
! $(addsuffix .c, $(PGPORT)): % : $(top_srcdir)/src/port/%
  	rm -f $@  $(LN_S) $ .
  
! $(addsuffix .c, $(BACKEND_LIBPQ)): % : $(backend_src)/libpq/%
  	rm -f $@  $(LN_S) $ .
  
! $(addsuffix .c, $(UTILS_MB)): % : $(backend_src)/utils/mb/%
  	rm -f $@  $(LN_S) $ .
  
  
--- 83,99 
  # We use several backend modules verbatim, but since we need to
  # compile with appropriate options to build a shared lib, we can't
  # necessarily use the same object files as the backend uses. Instead,
! # symlink the source files in here and build our own object file.
! # For some libpgport modules, this only happens if configure decides 
! # the module is needed (see filter hack in OBJS, above).
  
! crypt.c getaddrinfo.c inet_aton.c inet_net_ntop.c noblock.c open.c pgsleep.c pgstrcasecmp.c snprintf.c strerror.c strlcpy.c thread.c win32error.c: % : $(top_srcdir)/src/port/%
  	rm -f $@  $(LN_S) $ .
  
! ip.c md5.c: % : $(backend_src)/libpq/%
  	rm -f $@  $(LN_S) $ .
  
! encnames.c wchar.c: % : $(backend_src)/utils/mb/%
  	rm -f $@  $(LN_S) $ .
  
  
*** uninstall: uninstall-lib
*** 131,139 
  	rm -f '$(DESTDIR)$(datadir)/pg_service.conf.sample'
  
  clean distclean: clean-lib
! 	rm -f $(OBJS) pg_config_paths.h pthread.h libpq.rc $(addsuffix .c, $(PGPORT) $(BACKEND_LIBPQ) $(UTILS_MB))
  # Might be left over from a Win32 client-only build
  	rm -f pg_config_paths.h
  
  maintainer-clean: distclean maintainer-clean-lib
  	rm -f libpq-dist.rc
--- 130,145 
  	rm -f '$(DESTDIR)$(datadir)/pg_service.conf.sample'
  
  clean distclean: clean-lib
! 	rm -f $(OBJS) pthread.h libpq.rc
  # Might be left over from a Win32 client-only build
  	rm -f pg_config_paths.h
+ 	rm -f inet_net_ntop.c noblock.c pgstrcasecmp.c thread.c
+ 	# optional libpgport files
+ 	rm -f crypt.c getaddrinfo.c inet_aton.c open.c snprintf.c strerror.c strlcpy.c win32error.c
+ 	# optional Win32
+ 	rm -f pgsleep.c	
+ 	rm -f md5.c ip.c
+ 	rm -f encnames.c wchar.c
  
  maintainer-clean: distclean maintainer-clean-lib
  	rm -f libpq-dist.rc

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To 

Re: [HACKERS] SQL/MED - core functionality

2010-11-26 Thread Tom Lane
Shigeru HANADA han...@metrosystems.co.jp writes:
 I'll revise the patch along the discussion.  Before starting code work,
 please let me summarize the discussion.

 * Generally, we should keep FDWs away from PostgreSQL internals,
 such as TupleTableSlot.

 * FDW should have planner hook which allows FDW to create FDW-specific
 plan (FdwPlan in Heikki's proposal) for a scan on a foreign table.

 * FdwPlan, a part of ForeignScan plan node, should be able to be
 copied in generic way because plans would be copied into another
 memory context during caching.  It might be better to represent
 FdwPlan with Node or List.

 * FdwExecutionState, a part of ForeignScanState, should be used
 instead of ForeignScanState to remove executor details from FDW
 implementation.
 # ISTM that FdwExecutionState would be replace FdwReply.

FWIW, I think that the notion that FDW can be kept away from Postgres
internals is ridiculous on its face.  Re-read the above list and ask
yourself exactly which of the bullet points above are not talking about
being hip-deep in Postgres internals.  In particular, arbitrarily
avoiding use of TupleTableSlot is silly.  It's a fundamental part of
many executor APIs.

It would be a good idea to avoid use of internals in the wire protocol
to another Postgres database; but the interfaces to the local database
can hardly avoid that, and we shouldn't bend them out of shape to meet
entirely-arbitrary requirements about what to use or not use.

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] duplicate connection failure messages

2010-11-26 Thread Bruce Momjian
Peter Eisentraut wrote:
 On l?r, 2010-11-20 at 18:07 -0500, Bruce Momjian wrote:
  The output is as expected:
  
  $ psql -h localhost test
  psql: could not connect to server: Connection refused
  Is the server running on host localhost (127.0.0.1) and 
  accepting
  TCP/IP connections on port 5432?
  $ psql -h 127.0.0.1 test
  psql: could not connect to server: Connection refused
  Is the server running on host 127.0.0.1 and accepting
  TCP/IP connections on port 5432?
 
 Thanks for working on this.  However, the example I posted at the
 beginning of this thread now does this:
 
 $ ./psql -p 5 -h localhost
 psql: could not connect to server: Connection refused
 Is the server running on host localhost (???) and accepting
 TCP/IP connections on port 5?
 could not connect to server: Connection refused
 Is the server running on host localhost (127.0.0.1) and accepting
 TCP/IP connections on port 5?
 
 The ??? should presumably be ::1.

OK, I updated the code to always use cur_addr in the code --- let me
know if that doesn't fix it.

 Also, this comment should be updated:
 
 /*
  * Try to initiate a connection to one of the addresses
  * returned by pg_getaddrinfo_all().  conn-addr_cur is the
  * next one to try. We fail when we run out of addresses
  * (reporting the error returned for the *last* alternative,
  * which may not be what users expect :-().
  */

Thanks, comment udpated.  It was wrong even before because we were
reporting all failures even before I Started.

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

  + It's impossible for everything to be true. +
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 6593f21..8b55167 100644
*** /tmp/HkJ20a_fe-connect.c	Fri Nov 26 11:48:13 2010
--- src/interfaces/libpq/fe-connect.c	Fri Nov 26 11:36:28 2010
*** connectFailureMessage(PGconn *conn, int 
*** 989,996 
  	{
  		char	host_addr[NI_MAXHOST];
  		bool 	display_host_addr;
- 		struct sockaddr_in *host_addr_struct = (struct sockaddr_in *)
- conn-raddr.addr;
  
  		/*
  		 *	Optionally display the network address with the hostname.
--- 989,994 
*** connectFailureMessage(PGconn *conn, int 
*** 998,1005 
  		 */
  		if (conn-pghostaddr != NULL)
  			strlcpy(host_addr, conn-pghostaddr, NI_MAXHOST);
! 		else if (inet_net_ntop(conn-addr_cur-ai_family, host_addr_struct-sin_addr,
!  host_addr_struct-sin_family == AF_INET ? 32 : 128,
   host_addr, sizeof(host_addr)) == NULL)
  			strcpy(host_addr, ???);
  
--- 996,1004 
  		 */
  		if (conn-pghostaddr != NULL)
  			strlcpy(host_addr, conn-pghostaddr, NI_MAXHOST);
! 		else if (inet_net_ntop(conn-addr_cur-ai_family,
!  conn-addr_cur-ai_addr,
!  conn-addr_cur-ai_family == AF_INET ? 32 : 128,
   host_addr, sizeof(host_addr)) == NULL)
  			strcpy(host_addr, ???);
  

-- 
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] duplicate connection failure messages

2010-11-26 Thread Bruce Momjian
Alvaro Herrera wrote:
 Excerpts from Peter Eisentraut's message of vie nov 26 11:06:24 -0300 2010:
 
  Thanks for working on this.  However, the example I posted at the
  beginning of this thread now does this:
  
  $ ./psql -p 5 -h localhost
  psql: could not connect to server: Connection refused
  Is the server running on host localhost (???) and accepting
  TCP/IP connections on port 5?
 
 Shouldn't connectFailureMessage receive addr_cur as parameter?
 Otherwise it's not clear that it's getting the right params to report.

I am passing conn so I changed the code to always use addr_cur.

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

  + It's impossible for everything to be true. +

-- 
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] Instrument checkpoint sync calls

2010-11-26 Thread Greg Smith

Jeff Janes wrote:

For the individual file sync times emitted under debug1, it would be
very handy if the file being synced was identified, for example
relation base/16384/16523.


I was in the middle of looking into adding that already, so consider 
that part of the target for the next update I'm working on.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services and Supportwww.2ndQuadrant.us
PostgreSQL 9.0 High Performance: http://www.2ndQuadrant.com/books


--
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] Suggested easy TODO: pg_dump --from-list

2010-11-26 Thread Dmitriy Igrishin
Hey Dimitri, hackers,

Okay, there is a some getddl utility. But as for me, it does not
simplify the development. It is file-based pgAdmin with the
exception that I can use, e.g. Emacs for editing rather than
build-in editor of pgAdmin. But I can use Emacs from psql(1)...

Without some restoreddl which able to compile the set of
database objects from many files this tool (getddl) does nothing
because another developer can change the database directly
(from psql, pgAdmin, ...), commit his changes and when I restore
the database from entire dump (rather than from set of files created
by getddl) the result will be out of sync between files version control
system and restored database.

2010/11/26 Dimitri Fontaine dimi...@2ndquadrant.fr

 Robert Haas robertmh...@gmail.com writes:
  One thing I've often wished for is the ability to dump a specific
  function

 See getddl from OmniTI, or the alternative version I kept forgetting to
 put online somewhere:

  https://labs.omniti.com/labs/pgtreats/wiki/getddl
  https://github.com/dimitri/getddl

 The OmniTI version will output a single file with all objects into a
 single file, and my fork will do that in a directory structure with a
 file per object or about (a single file containing all functions sharing
 the same name, e.g.).

 Both project goal is to make it easy to version (as in git) your DDL and
 check for changes.

 Regards,
 --
 Dimitri Fontaine
 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

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




-- 
// Dmitriy.


Re: [HACKERS] SQL/MED - core functionality

2010-11-26 Thread Robert Haas
On Fri, Nov 26, 2010 at 11:16 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Shigeru HANADA han...@metrosystems.co.jp writes:
 I'll revise the patch along the discussion.  Before starting code work,
 please let me summarize the discussion.

 * Generally, we should keep FDWs away from PostgreSQL internals,
 such as TupleTableSlot.

 * FDW should have planner hook which allows FDW to create FDW-specific
 plan (FdwPlan in Heikki's proposal) for a scan on a foreign table.

 * FdwPlan, a part of ForeignScan plan node, should be able to be
 copied in generic way because plans would be copied into another
 memory context during caching.  It might be better to represent
 FdwPlan with Node or List.

 * FdwExecutionState, a part of ForeignScanState, should be used
 instead of ForeignScanState to remove executor details from FDW
 implementation.
 # ISTM that FdwExecutionState would be replace FdwReply.

 FWIW, I think that the notion that FDW can be kept away from Postgres
 internals is ridiculous on its face.  Re-read the above list and ask
 yourself exactly which of the bullet points above are not talking about
 being hip-deep in Postgres internals.  In particular, arbitrarily
 avoiding use of TupleTableSlot is silly.  It's a fundamental part of
 many executor APIs.

 It would be a good idea to avoid use of internals in the wire protocol
 to another Postgres database; but the interfaces to the local database
 can hardly avoid that, and we shouldn't bend them out of shape to meet
 entirely-arbitrary requirements about what to use or not use.

But there's probably some value in minimizing the number of
unnecessary interfaces that get exposed to the plugins.  There's no
help for the fact that the FDWs are going to need about Datums, but do
we gain anything by making them also know about TupleTableSlot?  If
so, fine; if not, don't.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] duplicate connection failure messages

2010-11-26 Thread Peter Eisentraut
On fre, 2010-11-26 at 11:53 -0500, Bruce Momjian wrote:
 OK, I updated the code to always use cur_addr in the code --- let me
 know if that doesn't fix it.

Now it's even more wrong:

psql: could not connect to server: Connection refused
Is the server running on host localhost (???) and accepting
TCP/IP connections on port 5?
could not connect to server: Connection refused
Is the server running on host localhost (232.106.56.8) and accepting
TCP/IP connections on port 5?


-- 
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] Assertion failure on hot standby

2010-11-26 Thread Robert Haas
On Fri, Nov 26, 2010 at 10:53 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 Incidentally, I haven't been able to wrap my head around why we need
 to propagate AccessExclusiveLocks to the standby in the first place.
 Can someone explain?

 To make the standby stop applying WAL when a local transaction on the
 standby
 uses an object.
 E.g. dropping a table on the master need the standby top stop applying
 wal (or
 kill the local client using the table).
 How would you want to protect against something like that otherwise?

 Hmm.  But it seems like that it would be enough to log any exclusive
 locks held at commit time, rather than logging them as they're
 acquired.  By then, the XID will be assigned (if you need it - if you
 don't then you probably don't need to XLOG it anyway) and you avoid
 holding the lock for more than a moment on the standby.

 But it seems like an even better idea would be to actually XLOG the
 operations that are problematic specifically.  Because, for example,
 if a user session on the master does LOCK TABLE ... IN ACCESS
 EXCLUSIVE MODE, AFAICS there's no reason for the standby to care.  Or
 am I confused?

 Let's approach this from a different direction:

 If you have operation A in the master that currently acquires an
 AccessExclusiveLock on a table, do you think it's safe for another
 transaction to peek at the table at the same time?

Beep, time out.  The notion of at the same time is extremely fuzzy
here.  The operations on the master and slave are not simultaneous, or
anything close to it.  Let's go back to the case of a dropped table.
Suppose that, on the master, someone begins a transaction, drops a
table, and heads out to lunch.  Upon returning, they commit the
transaction.  At what point does it became unsafe for readers on the
standby to be looking at the table?  Surely, the whole time the guy is
out to lunch, readers on the standby are free to do whatever they
want.  Only at the point when we actually remove the file does it
become a problem for somebody to be in the middle of using it.

In fact, you could apply the same logic to the master, if you were
willing to defer the removal of the actual physical file until all
transactions that were using it released their locks.  The reason we
don't do that - aside from complexity - is that it would result in an
unpredictable and indefinite delay between issuing the DROP TABLE
command and OS-level storage reclamation.  But in the standby
situation, there is *already* an unpredictable and indefinite delay.
The standby can fall behind in applying WAL, lose connectivity, have
replay paused, etc.  You lose nothing by waiting until the last
possible moment to kick everyone out.  (In fact, you gain something:
the standby is more usable.)

The problem here is not propagating operations from the master, but
making sure that actions performed by the startup process on the
standby are properly locked.  In the case of dropping a relation, the
problem is that the startup process only knows which relfilenode it
needs to blow away, not which relation that relfilenode is associated
with.  If the AccessShareLock were against the relfilenode rather than
the relation itself, the startup process would have no problem at all
generating a conflicting lock - it would simply lock each relfilenode
before dropping it, without any additional XLOG information at all.

 As a concrete example, VACUUM acquires an AccessExclusiveLock when it wants
 to truncate the relation. A sequential scan running against the table in the
 standby will get upset, if the startup process replays a truncation record
 on the table without warning.

This case is similar.  xl_smgr_truncate has only a relfilenode number,
not a relation OID, so there's no way for the startup process to
generate a conflicting lock request itself.  But if the standby
backends locked the relfilenode, or if the xl_smgr_truncate WAL record
included the relation OID, it would be simple.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] duplicate connection failure messages

2010-11-26 Thread Bruce Momjian
Peter Eisentraut wrote:
 On fre, 2010-11-26 at 11:53 -0500, Bruce Momjian wrote:
  OK, I updated the code to always use cur_addr in the code --- let me
  know if that doesn't fix it.
 
 Now it's even more wrong:
 
 psql: could not connect to server: Connection refused
 Is the server running on host localhost (???) and accepting
 TCP/IP connections on port 5?
 could not connect to server: Connection refused
 Is the server running on host localhost (232.106.56.8) and accepting
 TCP/IP connections on port 5?

Yep, even worse.  I have applied the attached patch, which gives me the
right IPv4 value.  I can't test IPv6.

I am finding the sock_addr structures confusing.

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

  + It's impossible for everything to be true. +
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 44a3c71..cdf8ee4 100644
*** /tmp/VwLJNa_fe-connect.c	Fri Nov 26 13:18:16 2010
--- src/interfaces/libpq/fe-connect.c	Fri Nov 26 13:11:12 2010
*** connectFailureMessage(PGconn *conn, int 
*** 989,994 
--- 989,996 
  	{
  		char	host_addr[NI_MAXHOST];
  		bool 	display_host_addr;
+ 		struct sockaddr_in *host_addr_struct = (struct sockaddr_in *)
+ 		conn-raddr.addr;
  
  		/*
  		 *	Optionally display the network address with the hostname.
*** connectFailureMessage(PGconn *conn, int 
*** 996,1004 
  		 */
  		if (conn-pghostaddr != NULL)
  			strlcpy(host_addr, conn-pghostaddr, NI_MAXHOST);
! 		else if (inet_net_ntop(conn-addr_cur-ai_family,
!  conn-addr_cur-ai_addr,
!  conn-addr_cur-ai_family == AF_INET ? 32 : 128,
   host_addr, sizeof(host_addr)) == NULL)
  			strcpy(host_addr, ???);
  
--- 998,1006 
  		 */
  		if (conn-pghostaddr != NULL)
  			strlcpy(host_addr, conn-pghostaddr, NI_MAXHOST);
! 		else if (inet_net_ntop(host_addr_struct-sin_family,
!  host_addr_struct-sin_addr.s_addr,
!  host_addr_struct-sin_family == AF_INET ? 32 : 128,
   host_addr, sizeof(host_addr)) == NULL)
  			strcpy(host_addr, ???);
  

-- 
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] Suggested easy TODO: pg_dump --from-list

2010-11-26 Thread Dimitri Fontaine
Dmitriy Igrishin dmit...@gmail.com writes:
 Without some restoreddl which able to compile the set of
 database objects from many files this tool (getddl) does nothing
 because another developer can change the database directly

Indeed, getddl does not try to solve that issue. It's more a VCS and
editor friendly export, to be able to see diffs that come from the real
current database state rather than what your code tracking makes you
think is in production already.

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

-- 
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] Assertion failure on hot standby

2010-11-26 Thread Heikki Linnakangas

On 26.11.2010 20:17, Robert Haas wrote:

On Fri, Nov 26, 2010 at 10:53 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com  wrote:

If you have operation A in the master that currently acquires an
AccessExclusiveLock on a table, do you think it's safe for another
transaction to peek at the table at the same time?


Beep, time out.  The notion of at the same time is extremely fuzzy
here.  The operations on the master and slave are not simultaneous, or
anything close to it.


In this context, at the same time means at the same point in WAL.


As a concrete example, VACUUM acquires an AccessExclusiveLock when it wants
to truncate the relation. A sequential scan running against the table in the
standby will get upset, if the startup process replays a truncation record
on the table without warning.


This case is similar.  xl_smgr_truncate has only a relfilenode number,
not a relation OID, so there's no way for the startup process to
generate a conflicting lock request itself.  But if the standby
backends locked the relfilenode, or if the xl_smgr_truncate WAL record
included the relation OID, it would be simple.


If you go down that path, you're going to spend a lot of time thinking 
through every single case that uses an AccessExclusiveLock, ensuring 
that the standby has enough information, and tinkering with the replay 
code to acquire locks at the right moment. And gain what, exactly?


WAL-logging AccessExclusiveLocks is a bit ugly, but it beats having to 
infer that information from other records hands down.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.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] memory leak in libxml2 - fix

2010-11-26 Thread Tom Lane
Pavel Stehule pavel.steh...@gmail.com writes:
 2010/11/26 Itagaki Takahiro itagaki.takah...@gmail.com:
 Why did you change doctree and ctxt to global variables?
 I'm not sure why /* xmlFreeDoc(doctree); */ is commented out
 at the end of pgxml_xpath(), but is it enough to enable the code?

 I am thinking, so you must not to call xmlFreeDoc(doctree) early.
 Probably xmlXPathCastToXXX reading a doctree.

Those static variables are really ugly, and what's more this patch only
stops some of the leakage.  Per experimentation, the result object from
pgxml_xpath has to be freed too, once it's been safely converted to
whatever the end result type is.  You can see this by watching

select sum(xpath_number('data' || generate_series || '/data','/data')) from
generate_series(1,50);

which still shows leakage with the submitted patch.  I cleaned it up
as per attached, which doesn't show any leakage.

regards, tom lane

diff --git a/contrib/xml2/xpath.c b/contrib/xml2/xpath.c
index 0df7647..e92ab66 100644
*** a/contrib/xml2/xpath.c
--- b/contrib/xml2/xpath.c
*** Datum		xpath_table(PG_FUNCTION_ARGS);
*** 40,45 
--- 40,54 
  
  void		pgxml_parser_init(void);
  
+ /* workspace for pgxml_xpath() */
+ 
+ typedef struct
+ {
+ 	xmlDocPtr	doctree;
+ 	xmlXPathContextPtr ctxt;
+ 	xmlXPathObjectPtr res;
+ } xpath_workspace;
+ 
  /* local declarations */
  
  static xmlChar *pgxmlNodeSetToText(xmlNodeSetPtr nodeset,
*** static text *pgxml_result_to_text(xmlXPa
*** 51,57 
  
  static xmlChar *pgxml_texttoxmlchar(text *textstring);
  
! static xmlXPathObjectPtr pgxml_xpath(text *document, xmlChar *xpath);
  
  
  /*
--- 60,69 
  
  static xmlChar *pgxml_texttoxmlchar(text *textstring);
  
! static xmlXPathObjectPtr pgxml_xpath(text *document, xmlChar *xpath,
! 	 xpath_workspace *workspace);
! 
! static void cleanup_workspace(xpath_workspace *workspace);
  
  
  /*
*** PG_FUNCTION_INFO_V1(xpath_nodeset);
*** 221,245 
  Datum
  xpath_nodeset(PG_FUNCTION_ARGS)
  {
! 	xmlChar*xpath,
! 			   *toptag,
! 			   *septag;
! 	int32		pathsize;
! 	text	   *xpathsupp,
! 			   *xpres;
! 
! 	/* PG_GETARG_TEXT_P(0) is document buffer */
! 	xpathsupp = PG_GETARG_TEXT_P(1);	/* XPath expression */
  
! 	toptag = pgxml_texttoxmlchar(PG_GETARG_TEXT_P(2));
! 	septag = pgxml_texttoxmlchar(PG_GETARG_TEXT_P(3));
  
! 	pathsize = VARSIZE(xpathsupp) - VARHDRSZ;
  
! 	xpath = pgxml_texttoxmlchar(xpathsupp);
  
! 	xpres = pgxml_result_to_text(pgxml_xpath(PG_GETARG_TEXT_P(0), xpath),
!  toptag, septag, NULL);
  
  	pfree(xpath);
  
--- 233,254 
  Datum
  xpath_nodeset(PG_FUNCTION_ARGS)
  {
! 	text	   *document = PG_GETARG_TEXT_P(0);
! 	text	   *xpathsupp = PG_GETARG_TEXT_P(1);	/* XPath expression */
! 	xmlChar*toptag = pgxml_texttoxmlchar(PG_GETARG_TEXT_P(2));
! 	xmlChar*septag = pgxml_texttoxmlchar(PG_GETARG_TEXT_P(3));
! 	xmlChar*xpath;
! 	text	   *xpres;
! 	xmlXPathObjectPtr res;
! 	xpath_workspace workspace;
  
! 	xpath = pgxml_texttoxmlchar(xpathsupp);
  
! 	res = pgxml_xpath(document, xpath, workspace);
  
! 	xpres = pgxml_result_to_text(res, toptag, septag, NULL);
  
! 	cleanup_workspace(workspace);
  
  	pfree(xpath);
  
*** PG_FUNCTION_INFO_V1(xpath_list);
*** 257,279 
  Datum
  xpath_list(PG_FUNCTION_ARGS)
  {
! 	xmlChar*xpath,
! 			   *plainsep;
! 	int32		pathsize;
! 	text	   *xpathsupp,
! 			   *xpres;
! 
! 	/* PG_GETARG_TEXT_P(0) is document buffer */
! 	xpathsupp = PG_GETARG_TEXT_P(1);	/* XPath expression */
  
! 	plainsep = pgxml_texttoxmlchar(PG_GETARG_TEXT_P(2));
  
! 	pathsize = VARSIZE(xpathsupp) - VARHDRSZ;
  
! 	xpath = pgxml_texttoxmlchar(xpathsupp);
  
! 	xpres = pgxml_result_to_text(pgxml_xpath(PG_GETARG_TEXT_P(0), xpath),
!  NULL, NULL, plainsep);
  
  	pfree(xpath);
  
--- 266,286 
  Datum
  xpath_list(PG_FUNCTION_ARGS)
  {
! 	text	   *document = PG_GETARG_TEXT_P(0);
! 	text	   *xpathsupp = PG_GETARG_TEXT_P(1);	/* XPath expression */
! 	xmlChar*plainsep = pgxml_texttoxmlchar(PG_GETARG_TEXT_P(2));
! 	xmlChar*xpath;
! 	text	   *xpres;
! 	xmlXPathObjectPtr res;
! 	xpath_workspace workspace;
  
! 	xpath = pgxml_texttoxmlchar(xpathsupp);
  
! 	res = pgxml_xpath(document, xpath, workspace);
  
! 	xpres = pgxml_result_to_text(res, NULL, NULL, plainsep);
  
! 	cleanup_workspace(workspace);
  
  	pfree(xpath);
  
*** PG_FUNCTION_INFO_V1(xpath_string);
*** 288,300 
  Datum
  xpath_string(PG_FUNCTION_ARGS)
  {
  	xmlChar*xpath;
  	int32		pathsize;
! 	text	   *xpathsupp,
! 			   *xpres;
! 
! 	/* PG_GETARG_TEXT_P(0) is document buffer */
! 	xpathsupp = PG_GETARG_TEXT_P(1);	/* XPath expression */
  
  	pathsize = VARSIZE(xpathsupp) - VARHDRSZ;
  
--- 295,307 
  Datum
  xpath_string(PG_FUNCTION_ARGS)
  {
+ 	text	   *document = PG_GETARG_TEXT_P(0);
+ 	text	   *xpathsupp = PG_GETARG_TEXT_P(1);	/* XPath expression */
  	xmlChar*xpath;
  	int32		pathsize;
! 	text	   

Re: [HACKERS] Re: [BUGS] BUG #5650: Postgres service showing as stopped when in fact it is running

2010-11-26 Thread Bruce Momjian
Fujii Masao wrote:
 On Fri, Nov 26, 2010 at 3:11 AM, Bruce Momjian br...@momjian.us wrote:
  I have applied this patch, with modified wording of the cannot connect
  case:
 
  ? ? ? ?$ pg_ctl -w -l /dev/null start
  ? ? ? ?waiting for server to start done
  ? ? ? ?server started
  ? ? ? ?warning: ?could not connect, perhaps due to invalid authentication or
  ? ? ? ?misconfiguration.
 
 This patch breaks the behavior that pg_ctl -w start waits until the standby
 has been ready to accept read-only queries. IOW, pg_ctl without this patch
 continues to check the connection even if the connection is rejected because
 the database has not been consistent yet. But pg_ctl with this patch treats
 that rejection as success of the standby starting and prints the above
 messages.
 
 I agree to treat the receipt of password request from the server as success
 of the server starting. But I don't think that we should treat other rejection
 cases that way and change the existing behavior.

OK, that is easy to fix.  The only downside is that if you misconfigured
.pgpass (which is what I used for testing), you have to wait 60 seconds
to get the cannot connect error message.  Is that OK?

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

  + It's impossible for everything to be true. +

-- 
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] duplicate connection failure messages

2010-11-26 Thread Peter Eisentraut
On fre, 2010-11-26 at 13:27 -0500, Bruce Momjian wrote:
 Peter Eisentraut wrote:
  On fre, 2010-11-26 at 11:53 -0500, Bruce Momjian wrote:
   OK, I updated the code to always use cur_addr in the code --- let me
   know if that doesn't fix it.
  
  Now it's even more wrong:
  
  psql: could not connect to server: Connection refused
  Is the server running on host localhost (???) and accepting
  TCP/IP connections on port 5?
  could not connect to server: Connection refused
  Is the server running on host localhost (232.106.56.8) and 
  accepting
  TCP/IP connections on port 5?
 
 Yep, even worse.  I have applied the attached patch, which gives me the
 right IPv4 value.  I can't test IPv6.

We're back to

psql: could not connect to server: Connection refused
Is the server running on host localhost (???) and accepting
TCP/IP connections on port 5?
could not connect to server: Connection refused
Is the server running on host localhost (127.0.0.1) and accepting
TCP/IP connections on port 5?



-- 
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] memory leak in libxml2 - fix

2010-11-26 Thread Alvaro Herrera
Excerpts from Tom Lane's message of vie nov 26 16:14:08 -0300 2010:

 Those static variables are really ugly, and what's more this patch only
 stops some of the leakage.  Per experimentation, the result object from
 pgxml_xpath has to be freed too, once it's been safely converted to
 whatever the end result type is.  You can see this by watching
 
 select sum(xpath_number('data' || generate_series || '/data','/data')) 
 from
 generate_series(1,50);
 
 which still shows leakage with the submitted patch.  I cleaned it up
 as per attached, which doesn't show any leakage.

This looks great.  As this fixes a problem that was reported to us two
days ago, I'm interested in backpatching it.  Are you going to do it?
Otherwise I can work on that.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] duplicate connection failure messages

2010-11-26 Thread Bruce Momjian
Peter Eisentraut wrote:
 On fre, 2010-11-26 at 13:27 -0500, Bruce Momjian wrote:
  Peter Eisentraut wrote:
   On fre, 2010-11-26 at 11:53 -0500, Bruce Momjian wrote:
OK, I updated the code to always use cur_addr in the code --- let me
know if that doesn't fix it.
   
   Now it's even more wrong:
   
   psql: could not connect to server: Connection refused
   Is the server running on host localhost (???) and accepting
   TCP/IP connections on port 5?
   could not connect to server: Connection refused
   Is the server running on host localhost (232.106.56.8) and 
   accepting
   TCP/IP connections on port 5?
  
  Yep, even worse.  I have applied the attached patch, which gives me the
  right IPv4 value.  I can't test IPv6.
 
 We're back to
 
 psql: could not connect to server: Connection refused
 Is the server running on host localhost (???) and accepting
 TCP/IP connections on port 5?
 could not connect to server: Connection refused
 Is the server running on host localhost (127.0.0.1) and accepting
 TCP/IP connections on port 5?

OK, good.  :-O  I just realize I can easily test this on Ubuntu so let
me get that working now.

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

  + It's impossible for everything to be true. +

-- 
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] memory leak in libxml2 - fix

2010-11-26 Thread Tom Lane
Alvaro Herrera alvhe...@commandprompt.com writes:
 This looks great.  As this fixes a problem that was reported to us two
 days ago, I'm interested in backpatching it.  Are you going to do it?

Yeah, I'm on it.  It's a bit tedious because the back branches are
different ...

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] improving foreign key locks

2010-11-26 Thread Alvaro Herrera
Excerpts from Florian Pflug's message of vie nov 26 10:48:39 -0300 2010:
 On Nov25, 2010, at 23:01 , Alvaro Herrera wrote:
  So I've been working on improving locks for foreign key checks, as
  discussed in a thread started by Joel Jacobson a while ago.  I've posted
  about this:
  http://www.commandprompt.com/blogs/alvaro_herrera/2010/11/fixing_foreign_key_deadlocks/
  http://www.commandprompt.com/blogs/alvaro_herrera/2010/11/fixing_foreign_key_deadlocks_part_2/
 
 To me, the whole thing seems to be special case of allowing to not only lock 
 whole tuples FOR UPDATE or FOR SHARE, but also individual fields or sets of 
 fields. Except that for simplicity, only two sets are supported, which are
   A) All fields
   B) All fields which are included in some unique constraint, including 
 primary keys.
 
 I'd therefore suggest to extend the FOR SHARE / FOR UPDATE syntax to be 
   SELECT FOR { SHARE | UPDATE } [ OF table1[.field1], ... ]
 and obtain what you call a KEY LOCK if (for a particular table) the set of 
 fields is a subset of (B). Otherwise, we'd obtain a full SHARE lock. Thus 
 we'd always lock at least the fields the user told us to, but sometimes more 
 than those, for the sake of a more efficient implementation.

The problem with this idea is that it's not possible to implement it.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] memory leak in libxml2 - fix

2010-11-26 Thread Pavel Stehule
2010/11/26 Tom Lane t...@sss.pgh.pa.us:
 Pavel Stehule pavel.steh...@gmail.com writes:
 2010/11/26 Itagaki Takahiro itagaki.takah...@gmail.com:
 Why did you change doctree and ctxt to global variables?
 I'm not sure why /* xmlFreeDoc(doctree); */ is commented out
 at the end of pgxml_xpath(), but is it enough to enable the code?

 I am thinking, so you must not to call xmlFreeDoc(doctree) early.
 Probably xmlXPathCastToXXX reading a doctree.

 Those static variables are really ugly, and what's more this patch only
 stops some of the leakage.  Per experimentation, the result object from
 pgxml_xpath has to be freed too, once it's been safely converted to
 whatever the end result type is.  You can see this by watching

 select sum(xpath_number('data' || generate_series || '/data','/data')) 
 from
 generate_series(1,50);

 which still shows leakage with the submitted patch.  I cleaned it up
 as per attached, which doesn't show any leakage.

great

thank you very much

regards

Pavel Stehule

                        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: [BUGS] BUG #5650: Postgres service showing as stopped when in fact it is running

2010-11-26 Thread Dmitriy Igrishin
Hey hackers,

I am sorry, but is it possible to implement BTW ability to
check exactly status of authentication from libpq ? As for now,
the only way to check failed authentication is parsing the error
message, that is sadly.

2010/11/26 Bruce Momjian br...@momjian.us

 Fujii Masao wrote:
  On Fri, Nov 26, 2010 at 3:11 AM, Bruce Momjian br...@momjian.us wrote:
   I have applied this patch, with modified wording of the cannot
 connect
   case:
  
   ? ? ? ?$ pg_ctl -w -l /dev/null start
   ? ? ? ?waiting for server to start done
   ? ? ? ?server started
   ? ? ? ?warning: ?could not connect, perhaps due to invalid
 authentication or
   ? ? ? ?misconfiguration.
 
  This patch breaks the behavior that pg_ctl -w start waits until the
 standby
  has been ready to accept read-only queries. IOW, pg_ctl without this
 patch
  continues to check the connection even if the connection is rejected
 because
  the database has not been consistent yet. But pg_ctl with this patch
 treats
  that rejection as success of the standby starting and prints the above
  messages.
 
  I agree to treat the receipt of password request from the server as
 success
  of the server starting. But I don't think that we should treat other
 rejection
  cases that way and change the existing behavior.

 OK, that is easy to fix.  The only downside is that if you misconfigured
 .pgpass (which is what I used for testing), you have to wait 60 seconds
 to get the cannot connect error message.  Is that OK?

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

  + It's impossible for everything to be true. +

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




-- 
// Dmitriy.


Re: [HACKERS] Re: [BUGS] BUG #5650: Postgres service showing as stopped when in fact it is running

2010-11-26 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 Fujii Masao wrote:
 I agree to treat the receipt of password request from the server as success
 of the server starting. But I don't think that we should treat other 
 rejection
 cases that way and change the existing behavior.

 OK, that is easy to fix.

It's wrong though.  If you get back a password rejected error, or most
other types of errors, it still indicates that the server started.
We just went over this a few days ago.

 The only downside is that if you misconfigured
 .pgpass (which is what I used for testing), you have to wait 60 seconds
 to get the cannot connect error message.  Is that OK?

No; it's useless and unnecessary behavior.

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: [BUGS] BUG #5650: Postgres service showing as stopped when in fact it is running

2010-11-26 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 Fujii Masao wrote:
 This patch breaks the behavior that pg_ctl -w start waits until the standby
 has been ready to accept read-only queries. IOW, pg_ctl without this patch
 continues to check the connection even if the connection is rejected because
 the database has not been consistent yet. But pg_ctl with this patch treats
 that rejection as success of the standby starting and prints the above
 messages.

The reason this is a problem is that somebody, in a fit of inappropriate
optimization, took out the code that allowed canAcceptConnections to
distinguish the not consistent yet state.  We need to put that back,
not try to kluge around the problem from the client side.

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] ToDo: enhanced diagnostic for plpgsql

2010-11-26 Thread Alvaro Herrera
Excerpts from Pavel Stehule's message of vie nov 26 07:37:42 -0300 2010:
 Hello
 
 there is difficult working with exception still. Isn't time to move forward?
 
 http://archives.postgresql.org/pgsql-general/2010-11/msg01129.php
 
 I am thinking, so we can little bit enhance a GET DIAGNOSTICS statement
 
 condition item names are defined in SQL 92 - mainly: TABLE_NAME,
 COLUM_NAME and MESSAGE_TEXT.

This is probably connected to my proposal here:
http://archives.postgresql.org/message-id/20090804171210.gl6...@alvh.no-ip.org

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] ToDo: enhanced diagnostic for plpgsql

2010-11-26 Thread Pavel Stehule
2010/11/26 Alvaro Herrera alvhe...@commandprompt.com:
 Excerpts from Pavel Stehule's message of vie nov 26 07:37:42 -0300 2010:
 Hello

 there is difficult working with exception still. Isn't time to move forward?

 http://archives.postgresql.org/pgsql-general/2010-11/msg01129.php

 I am thinking, so we can little bit enhance a GET DIAGNOSTICS statement

 condition item names are defined in SQL 92 - mainly: TABLE_NAME,
 COLUM_NAME and MESSAGE_TEXT.

 This is probably connected to my proposal here:
 http://archives.postgresql.org/message-id/20090804171210.gl6...@alvh.no-ip.org


yes, your proposal means first step

Pavel




 --
 Álvaro Herrera alvhe...@commandprompt.com
 The PostgreSQL Company - Command Prompt, Inc.
 PostgreSQL Replication, Consulting, Custom Development, 24x7 support


-- 
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] Tab completion for view triggers in psql

2010-11-26 Thread David Fetter
On Wed, Nov 24, 2010 at 11:01:28PM -0500, Josh Kupershmidt wrote:
 On Tue, Nov 23, 2010 at 10:21 PM, David Fetter da...@fetter.org wrote:
  Please find attached a patch changing both this and updateable to
  updatable, also per the very handy git grep I just learned about :)
 
 I looked a little more at this patch today. I didn't find any serious
 problems, though it would have helped me (and maybe other reviewers)
 to have a comprehensive list of the SQL statements which the patch
 implements autocompletion for.

OK, will add those to the description.

 Not sure how hard this would be to add in, but the following gets
 autocompleted with an INSERT:
   CREATE TRIGGER mytrg BEFORE IN[TAB]
 while the following doesn't find any autocompletions:
   CREATE TRIGGER mytrg BEFORE INSERT OR UP[TAB]

I might be able to hack something like this together :)

 Also, this existed before the patch, but this SQL:
   CREATE TRIGGER mytrg INSTEAD OF INSERT ON [TAB]
 autocompletes to:
   CREATE TRIGGER mytrg INSTEAD OF INSERT ON SET
 
 not sure how that SET is getting in there -- some incorrect tab
 completion match?

Very likely.  At some point, we will have to redo this stuff entirely
with a not-yet-invented parser API which will require a live
connection to the database in order to work correctly.

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] Assertion failure on hot standby

2010-11-26 Thread Robert Haas
On Fri, Nov 26, 2010 at 2:06 PM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 As a concrete example, VACUUM acquires an AccessExclusiveLock when it
 wants
 to truncate the relation. A sequential scan running against the table in
 the
 standby will get upset, if the startup process replays a truncation
 record
 on the table without warning.

 This case is similar.  xl_smgr_truncate has only a relfilenode number,
 not a relation OID, so there's no way for the startup process to
 generate a conflicting lock request itself.  But if the standby
 backends locked the relfilenode, or if the xl_smgr_truncate WAL record
 included the relation OID, it would be simple.

 If you go down that path, you're going to spend a lot of time thinking
 through every single case that uses an AccessExclusiveLock, ensuring that
 the standby has enough information, and tinkering with the replay code to
 acquire locks at the right moment. And gain what, exactly?

Well, fewer useless locks on the standby, for one thing, in all
likelihood, and less WAL traffic.  We probably don't have much of a
choice but to put in Simon's suggested fix (with a wal_level check
added) for 9.0.  But I suspect we're going to end up revisiting this
down the road.  Beyond the usability issues, the current approach
sounds brittle as heck to me.  All somebody has to do is introduce a
mechanism that drops or rewrites a relation file without an access
exclusive lock, and this whole approach snaps right off (e.g. CLUSTER
CONCURRENTLY, taking only an EXCLUSIVE lock, with some kind of garbage
collection for the old files left behind; or an on-line
table-compaction operation that tries to systematically move tuples to
lower CTIDs by - for example - writing the new tuple, waiting until
all scans in progress at the time of the write have finished, and then
pruning the old tuples).

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] ALTER OBJECT any_name SET SCHEMA name

2010-11-26 Thread Robert Haas
On Thu, Nov 25, 2010 at 5:00 PM, Dimitri Fontaine
dimi...@2ndquadrant.fr wrote:
 Robert Haas robertmh...@gmail.com writes:
 Please do.  Tab completion support should really be included in the
 patch - adding it as a separate patch is better than not having it, of
 course.

 Please find attached version 9 of the patch, which includes psql
 completion support of the SET SCHEMA variant of already supported
 ALTER commands.

 That means I didn't add ALTER OPERATOR [CLASS,FAMILY] completion
 support, my guess being there's no demand here, or the existing syntax
 variants would be there already. And if there's demand, I don't feel
 like it should be implemented as part of this very patch.

Committed, after various changes and corrections.  One noteworthy one
is that I removed the _oid variants, since those would be dead code at
the moment.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] duplicate connection failure messages

2010-11-26 Thread Bruce Momjian
Bruce Momjian wrote:
 Peter Eisentraut wrote:
  On fre, 2010-11-26 at 13:27 -0500, Bruce Momjian wrote:
   Peter Eisentraut wrote:
On fre, 2010-11-26 at 11:53 -0500, Bruce Momjian wrote:
 OK, I updated the code to always use cur_addr in the code --- let me
 know if that doesn't fix it.

Now it's even more wrong:

psql: could not connect to server: Connection refused
Is the server running on host localhost (???) and accepting
TCP/IP connections on port 5?
could not connect to server: Connection refused
Is the server running on host localhost (232.106.56.8) and 
accepting
TCP/IP connections on port 5?
   
   Yep, even worse.  I have applied the attached patch, which gives me the
   right IPv4 value.  I can't test IPv6.
  
  We're back to
  
  psql: could not connect to server: Connection refused
  Is the server running on host localhost (???) and accepting
  TCP/IP connections on port 5?
  could not connect to server: Connection refused
  Is the server running on host localhost (127.0.0.1) and accepting
  TCP/IP connections on port 5?
 
 OK, good.  :-O  I just realize I can easily test this on Ubuntu so let
 me get that working now.

OK, Tom and I both found the problem --- our data type assumed INET6 was
INET + 1, while libc had other ideas.  Here is the new, I guess correct,
output from Ubuntu:

$ /usr/local/pgsql/bin/psql -h localhost test
psql: could not connect to server: Connection refused
Is the server running on host localhost (::) and accepting
TCP/IP connections on port 5432?
could not connect to server: Connection refused
Is the server running on host localhost (127.0.0.1) and
accepting
TCP/IP connections on port 5432?

Is :: correct?

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

  + It's impossible for everything to be true. +

-- 
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: [BUGS] BUG #5650: Postgres service showing as stopped when in fact it is running

2010-11-26 Thread Tom Lane
I wrote:
 The reason this is a problem is that somebody, in a fit of inappropriate
 optimization, took out the code that allowed canAcceptConnections to
 distinguish the not consistent yet state.

Oh, no, that's not the case --- the PM_RECOVERY postmaster state does
still distinguish not-ready from ready.  The real problem is that what
Bruce implemented has practically nothing to do with what was discussed
last week.  PQping is supposed to be smarter about classifying errors
than this.

Speaking of classifying errors, should we have a fourth result value to
cover obviously bogus parameters?  Right now you'll get PQNORESPONSE
for cases like incorrect syntax in the conninfo string.  I'm not sure
how tense we ought to try to be about distinguishing, but if libpq
failed before even attempting a connection, PQNORESPONSE seems a bit
misleading.

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] Assertion failure on hot standby

2010-11-26 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Fri, Nov 26, 2010 at 2:06 PM, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com wrote:
 If you go down that path, you're going to spend a lot of time thinking
 through every single case that uses an AccessExclusiveLock, ensuring that
 the standby has enough information, and tinkering with the replay code to
 acquire locks at the right moment. And gain what, exactly?

 Well, fewer useless locks on the standby, for one thing, in all
 likelihood, and less WAL traffic.

I think it's not only useless from a performance standpoint, but
probably actually dangerous, to not take AccessExclusiveLock on the
standby when it's taken on the master.  If you try to delay taking the
lock, then locks will be taken in a different order on master and
standby, which is quite likely to lead to deadlock situations.

Speaking of which, is there any code in there to ensure that a deadlock
in the standby is resolved by killing HS queries and not the replay
process?  Because deadlocks are certainly going to be possible no matter
what.

 All somebody has to do is introduce a
 mechanism that drops or rewrites a relation file without an access
 exclusive lock, and this whole approach snaps right off

... as would queries on the master, so that's not ever happening.

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] duplicate connection failure messages

2010-11-26 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 Is :: correct?

I don't think so ... I get this, even sillier, response:

$ psql -h ::1 -p 5433 regression
psql: could not connect to server: Connection refused
Is the server running on host ::1 (::) and accepting
TCP/IP connections on port 5433?

Seems like a logic bug in inet_net_ntop_ipv6.  Before we waste mental
effort finding it for ourselves, has anyone checked for fixes in the
upstream code lately?

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] Assertion failure on hot standby

2010-11-26 Thread Robert Haas
On Fri, Nov 26, 2010 at 6:35 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Fri, Nov 26, 2010 at 2:06 PM, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com wrote:
 If you go down that path, you're going to spend a lot of time thinking
 through every single case that uses an AccessExclusiveLock, ensuring that
 the standby has enough information, and tinkering with the replay code to
 acquire locks at the right moment. And gain what, exactly?

 Well, fewer useless locks on the standby, for one thing, in all
 likelihood, and less WAL traffic.

 I think it's not only useless from a performance standpoint, but
 probably actually dangerous, to not take AccessExclusiveLock on the
 standby when it's taken on the master.  If you try to delay taking the
 lock, then locks will be taken in a different order on master and
 standby, which is quite likely to lead to deadlock situations.

As far as I can see, that's complete nonsense.  Deadlocks between what
and what?  To get a deadlock on the standby, you have to have a cycle
in the lock-wait graph; and since recovery is single-threaded, all
locks from the master are held by the startup process.  The only
possible cycle is between the startup process and some HS backend; and
why should we assume that the HS backend will acquire locks in the
same order as the transactions running on the master rather than any
other order?  Perhaps that could be accomplished by very careful
application design, but in normal usage it doesn't sound very likely.

In fact, now that I think about it, what I'm proposing would actually
substantially REDUCE the risk of deadlock on the standby, because the
master would only ever need to lock a backing file long enough to drop
or truncate it, whereas under the present system the startup process
might need to hold many locks at once.  It'd help reduce the chance of
lock table overflow, too.

 Speaking of which, is there any code in there to ensure that a deadlock
 in the standby is resolved by killing HS queries and not the replay
 process?  Because deadlocks are certainly going to be possible no matter
 what.

I believe the place to look is ResolveRecoveryConflictWithLock().

 All somebody has to do is introduce a
 mechanism that drops or rewrites a relation file without an access
 exclusive lock, and this whole approach snaps right off

 ... as would queries on the master, so that's not ever happening.

There might be a problem with what I wrote in the part you didn't
quote here, but if there is, an explanation would be a lot more
helpful than a categorical statement unsupported by any argument.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] [GENERAL] column-level update privs + lock table

2010-11-26 Thread Robert Haas
2010/11/25 KaiGai Kohei kai...@ak.jp.nec.com:
 (2010/10/16 4:49), Josh Kupershmidt wrote:
 [Moving to -hackers]

 On Fri, Oct 15, 2010 at 3:43 AM, Simon Riggssi...@2ndquadrant.com  wrote:
 On Mon, 2010-10-11 at 09:41 -0400, Josh Kupershmidt wrote:
 On Thu, Oct 7, 2010 at 7:43 PM, Josh Kupershmidtschmi...@gmail.com  
 wrote:

 I noticed that granting a user column-level update privileges doesn't
 allow that user to issue LOCK TABLE with any mode other than Access
 Share.

 Anyone think this could be added as a TODO?

 Seems so to me, but you raise on Hackers.

 Thanks, Simon. Attached is a simple patch to let column-level UPDATE
 privileges allow a user to LOCK TABLE in a mode higher than Access
 Share. Small doc. update and regression test update are included as
 well. Feedback is welcome.


 I checked your patch, then I'd like to mark it as ready for committer.

 The point of this patch is trying to solve an incompatible behavior
 between SELECT ... FOR SHARE/UPDATE and LOCK command.

 On ExecCheckRTEPerms(), it allows the required accesses when no columns
 are explicitly specified in the query and the current user has necessary
 privilege on one of columns within the target relation.
 If we stand on the perspective that LOCK command should take same
 privileges with the case when we use SELECT ... FOR SHARE/UPDATE without
 specifying explicit columns, like COUNT(*), the existing LOCK command
 seems to me odd.

 I think this patch fixes the behavior as we expected.

I'm not totally convinced that this is the correct behavior.  It seems
a bit surprising that UPDATE privilege on a single column is enough to
lock out all SELECT activity from the table.  It's actually a bit
surprising that even full-table UPDATE privileges are enough to do
this, but this change would allow people to block access to data they
can neither see nor modify.  That seems counterintuitive, if not a
security hole.

 BTW, how about backporting this patch?
 It seems to me a bug fix, although it contains user visible changes.

I don't think it's a bug fix; and even if could be so construed, I
don't think it's important enough to back-patch.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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: [BUGS] BUG #5650: Postgres service showing as stopped when in fact it is running

2010-11-26 Thread Robert Haas
On Fri, Nov 26, 2010 at 6:30 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Speaking of classifying errors, should we have a fourth result value to
 cover obviously bogus parameters?

+1.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] duplicate connection failure messages

2010-11-26 Thread Tom Lane
I wrote:
 Seems like a logic bug in inet_net_ntop_ipv6.

um ... no, it's connectFailureMessage's fault.

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] Assertion failure on hot standby

2010-11-26 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Fri, Nov 26, 2010 at 6:35 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 I think it's not only useless from a performance standpoint, but
 probably actually dangerous, to not take AccessExclusiveLock on the
 standby when it's taken on the master.

 As far as I can see, that's complete nonsense.  Deadlocks between what
 and what?

master locks table A, then table B.  Some transaction on the standby
locks table A, then table B (perhaps only with AccessShareLock).
This will work reliably only as long as we don't change the order in
which replay acquires locks.

Now, if the standby process takes its locks in the other order, then
it's at risk anyway.  But coding that works reliably against the master
should not randomly fail on the slave.

 In fact, now that I think about it, what I'm proposing would actually
 substantially REDUCE the risk of deadlock on the standby, because the
 master would only ever need to lock a backing file long enough to drop
 or truncate it, whereas under the present system the startup process
 might need to hold many locks at once.

Now you're the one spouting nonsense.  Consider a master transaction
that does

begin;
lock table foo;
alter table foo --- some rewriting table operation;
alter table foo --- some rewriting table operation;
alter table foo --- some rewriting table operation;
alter table foo --- some rewriting table operation;
alter table foo --- some rewriting table operation;
alter table foo --- some rewriting table operation;
alter table foo --- some rewriting table operation;
commit;

On the master, no other transaction can see the intermediate states.
We don't want that property to disappear on the slave.

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: [BUGS] BUG #5650: Postgres service showing as stopped when in fact it is running

2010-11-26 Thread Bruce Momjian
Tom Lane wrote:
 I wrote:
  The reason this is a problem is that somebody, in a fit of inappropriate
  optimization, took out the code that allowed canAcceptConnections to
  distinguish the not consistent yet state.
 
 Oh, no, that's not the case --- the PM_RECOVERY postmaster state does
 still distinguish not-ready from ready.  The real problem is that what
 Bruce implemented has practically nothing to do with what was discussed
 last week.  PQping is supposed to be smarter about classifying errors
 than this.

I was not aware this was discussed last week because I am behind on
email.  I was fixing a report from a month ago.  I did explain how I was
doing the tests.

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

  + It's impossible for everything to be true. +

-- 
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] libpq changes for synchronous replication

2010-11-26 Thread Alvaro Herrera
Excerpts from Fujii Masao's message of jue nov 25 10:47:12 -0300 2010:

 The attached patch s/CopyXLog/CopyBoth/g and adds the description
 about CopyBoth into the COPY section.

I gave this a look.  It seems good, but I'm not sure about this bit:

+   case 'W':   /* Start Copy Both */
+   /*
+* We don't need to use getCopyStart here since 
CopyBothResponse
+* specifies neither the copy format nor the number of 
columns in
+* the Copy data. They should be always zero.
+*/
+   conn-result = PQmakeEmptyPGresult(conn, PGRES_COPY_OUT);
+   if (!conn-result)
+   return;
+   conn-asyncStatus = PGASYNC_COPY_BOTH;
+   conn-copy_already_done = 0;
+   break;

I guess this was OK when this was conceived as CopyXlog, but since it's
now a generic mechanism, this seems a bit unwise.  Should this be
reconsidered so that it's possible to change the format or number of
columns?

(The paragraph added to the docs is also a bit too specific about this
being used exclusively in streaming replication, ISTM)

 While modifying the code, it occurred to me that we might have to add new
 ExecStatusType like PGRES_COPY_BOTH and use that for CopyBoth mode,
 for the sake of consistency. But since it's just alias of PGRES_COPY_BOTH
 for now, i.e., there is no specific behavior for that ExecStatusType, I don't
 think that it's worth adding that yet.

I'm not so sure about this.  If we think that it's worth adding a new
possible state, we should do so now; we will not be able to change this
behavior later.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] Spread checkpoint sync

2010-11-26 Thread Ron Mayer
Josh Berkus wrote:
 On 11/20/10 6:11 PM, Jeff Janes wrote:
 True, but I think that changing these from their defaults is not
 considered to be a dark art reserved for kernel hackers, i.e they are
 something that sysadmins are expected to tweak to suite their work
 load, just like the shmmax and such. 
 
 I disagree.  Linux kernel hackers know about these kinds of parameters,
 and I suppose that Linux performance experts do.  But very few
 sysadmins, in my experience, have any idea.

To me, a lot of this conversation feels parallel to the
arguments the occasionally come up debating writing directly
to raw disks bypassing the filesystems altogether.

Might smoother checkpoints be better solved by talking
to the OS vendors  virtual-memory-tunning-knob-authors
to work with them on exposing the ideal knobs; rather than
saying that our only tool is a hammer(fsync) so the problem
must be handled as a nail.


Hypothetically - what would the ideal knobs be?

Something like madvise WONTNEED but that leaves pages
in the OS's cache after writing them?


-- 
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] improving foreign key locks

2010-11-26 Thread Florian Pflug
On Nov26, 2010, at 21:06 , Alvaro Herrera wrote:
 Excerpts from Florian Pflug's message of vie nov 26 10:48:39 -0300 2010:
 On Nov25, 2010, at 23:01 , Alvaro Herrera wrote:
 So I've been working on improving locks for foreign key checks, as
 discussed in a thread started by Joel Jacobson a while ago.  I've posted
 about this:
 http://www.commandprompt.com/blogs/alvaro_herrera/2010/11/fixing_foreign_key_deadlocks/
 http://www.commandprompt.com/blogs/alvaro_herrera/2010/11/fixing_foreign_key_deadlocks_part_2/
 
 To me, the whole thing seems to be special case of allowing to not only lock 
 whole tuples FOR UPDATE or FOR SHARE, but also individual fields or sets of 
 fields. Except that for simplicity, only two sets are supported, which are
  A) All fields
  B) All fields which are included in some unique constraint, including 
 primary keys.
 
 I'd therefore suggest to extend the FOR SHARE / FOR UPDATE syntax to be 
  SELECT FOR { SHARE | UPDATE } [ OF table1[.field1], ... ]
 and obtain what you call a KEY LOCK if (for a particular table) the set of 
 fields is a subset of (B). Otherwise, we'd obtain a full SHARE lock. Thus 
 we'd always lock at least the fields the user told us to, but sometimes more 
 than those, for the sake of a more efficient implementation.
 
 The problem with this idea is that it's not possible to implement it.

How so? The implementation you proposed in your blog should work fine for this. 
XMAX_KEY_LOCK would signal that only fields from set (B) are locked, while 
XMAX_SHARE_LOCK (or however thats called, didn't check the code) would signal 
that all fields are locked. These are the only two field sets that we'd 
support, and for any set of columns the user specified we'd pick the smallest 
superset of the set we *do* support and use that (Thus, we obtain a key lock if 
only fields from a unique index where specified, and a share lock otherwise).

The only difference is that instead of presenting this to the user as an 
entirely new lock type, we instead present it as a generalization of SHARE 
locks. The advantage being that *if* we ever figure out a way to support more 
fine-grained locking of fields, (say, locking only the fields contain in some 
*specific* index, maybe by storing locking the index tuple), we can do so 
completely transparent to the user.

greetings,
Florian Pflug


-- 
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: [BUGS] BUG #5650: Postgres service showing as stopped when in fact it is running

2010-11-26 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 Tom Lane wrote:
 PQping is supposed to be smarter about classifying errors
 than this.

 I was not aware this was discussed last week because I am behind on
 email.  I was fixing a report from a month ago.  I did explain how I was
 doing the tests.

Um, you did respond in that thread, several times even:
http://archives.postgresql.org/pgsql-hackers/2010-11/msg01102.php
so I kind of assumed that the patch you presented this week did
what was agreed to last week.

I have committed a patch to make PQping do what was agreed to.

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] ToDo: enhanced diagnostic for plpgsql

2010-11-26 Thread Pavel Stehule
Hello

do you plan do some work on this job?

Regards

Pavel Stehule

2010/11/26 Alvaro Herrera alvhe...@commandprompt.com:
 Excerpts from Pavel Stehule's message of vie nov 26 07:37:42 -0300 2010:
 Hello

 there is difficult working with exception still. Isn't time to move forward?

 http://archives.postgresql.org/pgsql-general/2010-11/msg01129.php

 I am thinking, so we can little bit enhance a GET DIAGNOSTICS statement

 condition item names are defined in SQL 92 - mainly: TABLE_NAME,
 COLUM_NAME and MESSAGE_TEXT.

 This is probably connected to my proposal here:
 http://archives.postgresql.org/message-id/20090804171210.gl6...@alvh.no-ip.org

 --
 Álvaro Herrera alvhe...@commandprompt.com
 The PostgreSQL Company - Command Prompt, Inc.
 PostgreSQL Replication, Consulting, Custom Development, 24x7 support


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