Re: [HACKERS] CommitFest 2009-07 - End of Week 1

2009-07-22 Thread Magnus Hagander
On Wed, Jul 22, 2009 at 05:35, Tom Lanet...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 There are a few patches that have some level of committer endorsement
 from previous discussions, and it's not clear whether any round-robin
 review is required.  These are:

 Fix memory leak in win32 security functions (Magnus)
 do_tup_output_datum v2 (Tom)

 These might be worth a quick look by the respective committers to see
 whether these can just go in.  If not, we'll assign round-robin
 reviewers.

 Roger, I'll look at do_tup_output_datum in the morning, but I think
 it's noncontroversial.

 Magnus just got back from sailing and presumably will catch up on
 his commitments in due course ...

Yeah, I'm still backlogged but I'm getting there. That specific patch
is something I've already reviewed, so I just need to verify there has
been no drift (not likely) and get it in. I expect to get to
commitfest work within a day or two.

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

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


Re: [HACKERS] [PATCH] SE-PgSQL/tiny rev.2193

2009-07-22 Thread Peter Eisentraut
On Monday 20 July 2009 17:52:44 Joshua Brindle wrote:
 That is your (and the communities) prerogative. Linus wasn't very
 supportive of SELinux in the kernel either but it is the only way Linux got
 an EAL4+ LSPP evaluation for use in certain government systems. I
 personally would love to see an open source DBMS evaluated for systems like
 this because the current state of the art is fairly sad.

This would actually be a reasonable baseline to work against, if we define a 
project goal to be satisfying this standard.

This is presumably the web site that describes this standard: http://www.niap-
ccevs.org/cc-scheme/pp/pp_os_ls_v1.b/  There I see

Succeeded By:  pp_os_ml_mr2.0_v1.91  
Sunset Date: 16 September 2007

And the successor document is vastly more comprehensive than implementing a 
MAC scheme.

So how do we realistically get from here to there (and where is there)?

-- 
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] Non-blocking communication between a frontend and a backend (pqcomm)

2009-07-22 Thread Fujii Masao
Hi,

On Wed, Jul 22, 2009 at 2:20 AM, Robert Haasrobertmh...@gmail.com wrote:
 Fujii Masao,

 Are you planning to update this patch based on Martin's review?

Sure. Attached is an updated patch.

 On Fri, Jul 17, 2009 at 5:26 PM, Martin Pihlakmartin.pih...@gmail.com wrote:
 Here's my initial review of the non-blocking pqcomm patch. The patch applies
 cleanly and passes regression. Generally looks nice and clean. Couple of 
 remarks
 from the department of nitpicking:

Thanks for reviewing the patch!

 * In secure_poll() the handling of timeouts is different depending whether
  poll(), select() or SSL_pending() is used. The latter doesn't use the
  timeout value at all, and for select() it is impossible to specify 
 indefinite
  timeout.

Fixed. I tweaked the handling of the fifth argument 'timeout' of select(); when
a negative number is specified to a timeout of secure_poll(), NULL is set to
that 'timeout', which can block select() indefinitely.

Since SSL_pending() doesn't wait for data to arrive (i.e., doesn't use timeout),
I didn't change the code related to that function.

 * occasional blank lines consisting of a single tab character -- maybe
  a left-over from editor auto-indent. Not sure of how much a problem this
  is, given that the blanks will be removed by pg_indent.

Fixed.

 * Comment on pq_wait() seems to have a typo: -1 if an error directly.

Fixed.

Regards,

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


nonblocking_pqcomm_0722.patch
Description: Binary data

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


Re: [HACKERS] [PATCH] SE-PgSQL/tiny rev.2193

2009-07-22 Thread KaiGai Kohei
Peter Eisentraut wrote:
 On Monday 20 July 2009 17:52:44 Joshua Brindle wrote:
 That is your (and the communities) prerogative. Linus wasn't very
 supportive of SELinux in the kernel either but it is the only way Linux got
 an EAL4+ LSPP evaluation for use in certain government systems. I
 personally would love to see an open source DBMS evaluated for systems like
 this because the current state of the art is fairly sad.
 
 This would actually be a reasonable baseline to work against, if we define a 
 project goal to be satisfying this standard.
 
 This is presumably the web site that describes this standard: http://www.niap-
 ccevs.org/cc-scheme/pp/pp_os_ls_v1.b/  There I see
 
 Succeeded By:  pp_os_ml_mr2.0_v1.91  
 Sunset Date: 16 September 2007
 
 And the successor document is vastly more comprehensive than implementing a 
 MAC scheme.

The target of this protection profile is operating system,
so it is a bit mismatch.

Referring to the prior case, Oracle Label Security (it also provides
label based access controls, but no collaboration with OS) is cerfified
with the BR-DBMSPP (U.S.Government Protection Profile Database Management
Systems For Basic Robustness Environments) and additional functional
components (such as FDP_IFC.1: Subset Information Flow Control,
FDP_IFF.2: Hierarchical Security Attributes, and so on).

http://www.commoncriteriaportal.org/products_DB.html#DB

* ST for Oracle Label Security for Oracle Database 10g Release 2
http://www.commoncriteriaportal.org/files/epfiles/20080306_0402b.pdf

 So how do we realistically get from here to there (and where is there)?

Security functionality is a factor to get ISO/IEC15408 certification,
but not all. (For example, who can defray the cost for evaluation?)
However, it is important to pull up the baseline functionality to
satisfy these security functional components.

If certification is only valid on the patched version, rest of people
will not be happy.

-- 
OSS Platform Development Division, NEC
KaiGai Kohei kai...@ak.jp.nec.com

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


Re: [HACKERS] Higher TOAST compression.

2009-07-22 Thread Laurent Laborde
My 1st applied patch is the safest and simpliest :
in pg_lzcompress.c :

static const PGLZ_Strategy strategy_default_data = {
256,/* Data chunks less than 256 are not compressed */
256,/* force compression on data chunks on record = 256bytes */
1,  /* compression rate below 1% fall back to uncompressed*/
256,/* Stop history lookup if a match of 256 bytes is found   */
6   /* lower good match size b 6% at every lookup iteration   */
};
const PGLZ_Strategy *const PGLZ_strategy_default = strategy_default_data;

I need to test for a few days. But the firsts tests show that we're
still IObound :)
The most obvious effect is reduction by a factor 2~10 of the size of
some TOAST table.
It seems that a lot of record are now kept in-line instead of being
stored in TOAST.

I will come back later with some numbers :)

Next patch will be a modified kevin's patch. (it doesn't directly
apply to our source code as i'm using postgresql 8.3 and his patch is
for 8.4) and a change in TOAST thresold and target.

What do you think about the parameters i used in the compression strategy ?
PS : biggest records are french text and html. (blog data : articles,
comments, ...)
Thank you.

-- 
Laurent Laborde
Sysadmin @ http://www.over-blog.com/

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


Re: [HACKERS] [PATCH] user mapping extension to pg_ident.conf

2009-07-22 Thread Magnus Hagander
On Tue, Jul 21, 2009 at 16:06, Tom Lanet...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 On Tue, Jul 21, 2009 at 15:58, Tom Lanet...@sss.pgh.pa.us wrote:
 Are you not describing a behavior that you yourself removed in 8.4,
 ie the libpq code that looked aside at Kerberos for a username?

 Yes, partially I am :-)

 But it was not documented, and done in a fairly hackish way. If we
 want it, it should work the same for *all* external authentication
 methods (where it would be possible).

 Well, the problem with it of course was that it happened even when the
 selected auth method was not Kerberos.

That was the core problem, yes. IIRC there were some other minor
issues with it as well.


 Doing it on the client presents a certain challenge

 Yup, you would need a protocol change that would allow the client to
 change its mind about what the username was after it got the auth
 challenge.  And then what effects does that have on username-sensitive
 pg_hba.conf decisions?  We go back and change our minds about the
 challenge type, perhaps?  The whole thing seems like a nonstarter to me.

challenge type? Not sure I understand what you are referring to here.


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

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


Re: [HACKERS] psql - small fix in \du

2009-07-22 Thread Andreas Wenk

ANdreas Wenk schrieb:

Hi,

attached you can find a very small patch for the help in psql (\?). It's 
possible to use \du also as \du+ . The [+] was missing in help.


I was asking about this at the general list and Peter E. was asking me 
to provide a patch. I sent the patch there but realized now, that this 
was the wrong place to do so. Sorry for the flood ...


Cheers

Andy



As written in another post \dg misses also the [+] so I changed that. Now I recognized 
that I also have to change psql-ref.sgml (sorry did not see it in help.c). So I wanted to 
update it and I am now wondering, what will be written in the description row when using 
\du+ or \dg+. I am not sure when the function shobj_description(oid, name) is giving a 
result.


Can anybody give me a hint or a scenario so that I can update the patch 
correctly?

Thanks a lot

Andy

--
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] WIP: Deferrable unique constraints

2009-07-22 Thread Dean Rasheed
2009/7/22 Dean Rasheed dean.a.rash...@googlemail.com:
 OK, here's an updated patch.


In case it's not obvious, I've opted for backend/commands for the new file.

 - Dean

-- 
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] Upgrading our minimum required flex version for 8.5

2009-07-22 Thread Magnus Hagander
On Tue, Jul 14, 2009 at 05:10, Andrew Dunstanand...@dunslane.net wrote:
 Tom Lane wrote:

 Maybe for the time being we need to think about keeping scan.c in CVS.
 It's not like scan.l gets updated all that often.


 We could if we had to, though it amounts to saying that Windows-based
 developers don't get to touch the scanner.




 True, but I'm not going to invest a large number of cycles on porting this.
 I'm not very happy about it either. I guess anyone wanting to develop on
 Windows and affect the scanner could install Cygwin or MSys.

I think requiring that for messing with the scanner is acceptable. As
it is now, requiring that to do *any* development or compiling on
HEAD, is a serious regression.

FWIW, it seems the version that Andrew put up doesn't work in one of
my test environments, and also not in at last one of Dave's. I will
test it in my second test environment later today to be sure.

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

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


Re: [HACKERS] Upgrading our minimum required flex version for 8.5

2009-07-22 Thread Dave Page
On Wed, Jul 22, 2009 at 12:42 PM, Magnus Hagandermag...@hagander.net wrote:

 FWIW, it seems the version that Andrew put up doesn't work in one of
 my test environments, and also not in at last one of Dave's. I will
 test it in my second test environment later today to be sure.

It doesn't work in any of my bf animals, or build machines :-(

-- 
Dave Page
EnterpriseDB UK:   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] Upgrading our minimum required flex version for 8.5

2009-07-22 Thread Andrew Dunstan



Dave Page wrote:

On Wed, Jul 22, 2009 at 12:42 PM, Magnus Hagandermag...@hagander.net wrote:

  

FWIW, it seems the version that Andrew put up doesn't work in one of
my test environments, and also not in at last one of Dave's. I will
test it in my second test environment later today to be sure.



It doesn't work in any of my bf animals, or build machines :-(
  



Darn. I'm working on it.

cheers

andrew

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


Re: [HACKERS] [PATCH] user mapping extension to pg_ident.conf

2009-07-22 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 Yup, you would need a protocol change that would allow the client to
 change its mind about what the username was after it got the auth
 challenge.  And then what effects does that have on username-sensitive
 pg_hba.conf decisions?  We go back and change our minds about the
 challenge type, perhaps?  The whole thing seems like a nonstarter to me.

 challenge type? Not sure I understand what you are referring to here.

The point is that pg_hba.conf allows the selection of auth method to
depend on username.  What happens if, after being told auth method is
(say) Kerberos, the client comes back and wants to use a different
username that should have resulted in a different auth method according
to pg_hba.conf?  It's not hard to construct scenarios where that would
be seen as a security breach.

regards, tom lane

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


Re: [HACKERS] [PATCH] user mapping extension to pg_ident.conf

2009-07-22 Thread Magnus Hagander
On Wed, Jul 22, 2009 at 14:53, Tom Lanet...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 Yup, you would need a protocol change that would allow the client to
 change its mind about what the username was after it got the auth
 challenge.  And then what effects does that have on username-sensitive
 pg_hba.conf decisions?  We go back and change our minds about the
 challenge type, perhaps?  The whole thing seems like a nonstarter to me.

 challenge type? Not sure I understand what you are referring to here.

 The point is that pg_hba.conf allows the selection of auth method to
 depend on username.  What happens if, after being told auth method is
 (say) Kerberos, the client comes back and wants to use a different
 username that should have resulted in a different auth method according
 to pg_hba.conf?  It's not hard to construct scenarios where that would
 be seen as a security breach.

Oh. Now I get it. Good point. Forgot about the username being part of
that. Yeah, that basicalliy says it has to be a client-side
implementation only.

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

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


Re: [HACKERS] Upgrading our minimum required flex version for 8.5

2009-07-22 Thread Tom Lane
Dave Page dp...@pgadmin.org writes:
 It doesn't work in any of my bf animals, or build machines :-(

?? narwhal seems to have gone green.

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] psql - small fix in \du

2009-07-22 Thread Tom Lane
Andreas Wenk a.w...@netzmeister-st-pauli.de writes:
 I am not sure when the function shobj_description(oid, name) is giving a 
 result.

That retrieves the comment for the object (the role, in this case).

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] Upgrading our minimum required flex version for 8.5

2009-07-22 Thread Dave Page
On Wed, Jul 22, 2009 at 2:16 PM, Tom Lanet...@sss.pgh.pa.us wrote:
 Dave Page dp...@pgadmin.org writes:
 It doesn't work in any of my bf animals, or build machines :-(

 ?? narwhal seems to have gone green.

Narwhal is mingw/msys. The misbehaving flex is the one provided by
Andrew for use with VC++ (where the aim is to avoid having mingw/msys
of course).


-- 
Dave Page
EnterpriseDB UK:   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] Upgrading our minimum required flex version for 8.5

2009-07-22 Thread Andrew Dunstan



Tom Lane wrote:

Dave Page dp...@pgadmin.org writes:
  

It doesn't work in any of my bf animals, or build machines :-(



?? narwhal seems to have gone green.
  



Yeah, the problem is with MSVC, Narwal is a Mingw box.

cheers

andrew

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


Re: [HACKERS] Higher TOAST compression.

2009-07-22 Thread Kevin Grittner
Tom Lane t...@sss.pgh.pa.us wrote:
 Kevin Grittner kevin.gritt...@wicourts.gov writes:
 It seems like it might be reasonable to have a separate threshold
 for compression from that for out-of-line storage.  Since I've been
 in that code recently, I would be pretty comfortable doing
 something about that; but, as is so often the case, the problem
 will probably be getting agreement on what would be a good change.
 
 I'm not clear how this would work.  The toast code is designed
 around hitting a target for the overall tuple size; how would it
 make sense to treat compression and out-of-lining differently?
 
The current steps are:
 
1: Inline compress attributes with attstorage 'x', and store very
   large attributes with attstorage 'x' or 'e' external immediately
2: Store attributes with attstorage 'x' or 'e' external
3: Inline compress attributes with attstorage 'm'
4: Store attributes with attstorage 'm' external
 
If we had separate compression and external storage tuple targets:
 
Is there any reason not to include 'm' in the first inline compression
phase (step 1)?  It does seem reasonable to store very large
attributes externally in the first pass, but it would be pretty easy
to include 'm' in the compression but skip it for the external storage
during step 1.  In this phase we would use the compression target.
 
Step 2 would use the target tuple size for external storage, which
would probably usually be = the compression target.  If we want to
allow a compression target  external storage target, I guess we would
have to allow the smaller target to go first; however, I'm not really
sure if there is a sane use case for a larger compression target than
external storage target.
 
Step 3 would go away, since its work could be moved to step 1.
 
Step 4 would maintain the behavior created by the recent patch.
 
 And especially, how could you have per-column targets?
 
 I could see having a reloption that allowed per-table adjustment of
 the target tuple width...
 
Yeah, this would have to be done by table, not by column.
 
The compression configuration mentioned by Laurent, if we want to make
that tunable, which could make sense by column; but the toast tuple
sizes targets would clearly need to be by table.
 
-Kevin

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


Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows

2009-07-22 Thread Magnus Hagander
On Tue, Jul 21, 2009 at 14:06, Magnus Hagandermag...@hagander.net wrote:
 On Wed, Jul 15, 2009 at 11:20, Tsutomu Yamadatsut...@sraoss.co.jp wrote:
 Hello,

 Alvaro Herrera alvhe...@commandprompt.com wrote:
   Tsutomu Yamada wrote:
  
    This patch using VirtualAlloc()/VirtualFree() to avoid failing in
    reattach to shared memory.
   
    Can this be added to CommitFest ?
  
   Since this fixes a very annoying bug present in older versions, I think
   this should be backpatched all the way back to 8.2.
  
   Some notes about the patch itself:
  
   - please use ereport() instead of elog() for error messages
   - Are you really putting the pgwin32_ReserveSharedMemory declaration
   inside a function?  Please move that into the appropriate header file.
   - Failure to reserve memory in pgwin32_ReserveSharedMemory should be a
   FATAL error I think, not simply LOG.

 In this case,
 the parent process operates child's memory by using VirtualAlloc().
 If VirtualAlloc failed and be a FATAL error, master process will be stopped.

 I think that is not preferable.
 So, when VirtualAlloc failed, parent reports error and terminates child.

 Revised patch

 - move function declaration to include/port/win32.h
 - add error check.
  when VirtualAlloc failed, parent will terminate child process.

 This patch looks a lot like one I've had sitting in my tree since
 before I left for three weeks of vacation, based on the same
 suggestion on the list. I will check if we have any actual functional
 differences, and merge yours with mine. The one I had worked fine in
 my testing.

 Once that is done, I propose the following:

 * Apply to HEAD. That will give us buildfarm coverage.
 * Produce a modified 8.4.0 *and* 8.3.7 binary for this, and ask people
 to test this. Both people with and without the problem.
 * Assuming it works for all users, backpatch to 8.2, 8.3 and 8.4.

Attached are two updated versions of this patch, one for 8.4 and one
for 8.3. They differ only in line numbers. I've merged your patch with
mine, which mainly contained of more comments. One functionality check
- to make sure the VirtualAllocEx() call returns the same address as
our base one. It should always do this, but my patch adds a check t
make sure this is true.

Dave has built binaries for 8.3.7 and 8.4.0 for this, available at:

http://developer.pgadmin.org/~dpage/postgres_exe_virtualalloc-8_3.zip
http://developer.pgadmin.org/~dpage/postgres_exe_virtualalloc-8_4.zip


We would like as many people as possible to test this both on systems
that currently experience the problem and systems that don't, and let
us know the status. To test, just replace your current postgres.exe
binary with the one in the appropriate ZIP file above. Obviously, take
a backup before you do it! These binaries contain just this one patch
- the rest of what's been applied to the 8.3 and 8.4 branches for the
next minor version is *not* included.

-- 
 Magnus Hagander
 Self: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/
*** src/backend/port/win32_shmem.c
--- src/backend/port/win32_shmem.c
***
*** 18,23 
--- 18,24 
  
  unsigned long UsedShmemSegID = 0;
  void	   *UsedShmemSegAddr = NULL;
+ static Size UsedShmemSegSize = 0;
  
  static void pgwin32_SharedMemoryDelete(int status, Datum shmId);
  
***
*** 229,234  PGSharedMemoryCreate(Size size, bool makePrivate, int port)
--- 230,236 
  
  	/* Save info for possible future use */
  	UsedShmemSegAddr = memAddress;
+ 	UsedShmemSegSize = size;
  	UsedShmemSegID = (unsigned long) hmap2;
  
  	return hdr;
***
*** 253,258  PGSharedMemoryReAttach(void)
--- 255,267 
  	Assert(UsedShmemSegAddr != NULL);
  	Assert(IsUnderPostmaster);
  
+ 	/*
+ 	 * Release memory region reservation that was made by the postmaster
+ 	 */
+ 	if (VirtualFree(UsedShmemSegAddr, 0, MEM_RELEASE) == 0)
+ 		elog(FATAL, failed to release reserved memory region (addr=%p): %lu,
+ 			 UsedShmemSegAddr, GetLastError());
+ 
  	hdr = (PGShmemHeader *) MapViewOfFileEx((HANDLE) UsedShmemSegID, FILE_MAP_READ | FILE_MAP_WRITE, 0, 0, 0, UsedShmemSegAddr);
  	if (!hdr)
  		elog(FATAL, could not reattach to shared memory (key=%d, addr=%p): %lu,
***
*** 298,300  pgwin32_SharedMemoryDelete(int status, Datum shmId)
--- 307,359 
  	if (!CloseHandle((HANDLE) DatumGetInt32(shmId)))
  		elog(LOG, could not close handle to shared memory: %lu, GetLastError());
  }
+ 
+ /*
+  * pgwin32_ReserveSharedMemoryRegion(hChild)
+  *
+  * Reserve the memory region that will be used for shared memory in a child
+  * process. It is called before the child process starts, to make sure the
+  * memory is available.
+  *
+  * Once the child starts, DLLs loading in different order or threads getting
+  * scheduled differently may allocate memory which can conflict with the
+  * address space we need for our shared memory. By reserving the shared
+  * memory region before the child starts, and freeing it only 

Re: [HACKERS] Higher TOAST compression.

2009-07-22 Thread Kevin Grittner
I wrote: 
 If we want to allow a compression target  external storage target,
 I guess we would have to allow the smaller target to go first
 
Scratch that part -- even with a compression target  the external
storage target, it would make sense use the same sequence of steps.
 
-Kevin

-- 
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] Higher TOAST compression.

2009-07-22 Thread Kevin Grittner
Sorry I responded that quickly this early.  I keep having additional
thoughts
 
Kevin Grittner kevin.gritt...@wicourts.gov wrote: 
 Tom Lane t...@sss.pgh.pa.us wrote:
 
 And especially, how could you have per-column targets?
 
 Yeah, this would have to be done by table, not by column.
 
If we had an optional two targets by column, we could pass any columns
with such targets as a step 0, before starting the tuple size
checks.  I think that makes sense, so I'm flip-flopping on that as a
possibility.
 
Now, whether that's overkill is another question.
 
-Kevin

-- 
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] revised hstore patch

2009-07-22 Thread Andrew Gierth
 Tom == Tom Lane t...@sss.pgh.pa.us writes:

  I'm prepared to give slightly more consideration to option #3:
  make the new code read the old format as well as the new one.

 Tom If you think you can make that work, it would solve the problem.

I was hoping to do it without changing the new format, but that turns
out not to be achievable, thanks to the fact (which I just discovered)
that the old hstore can leave unlimited amounts of wasted space at the
end of the object (does this count as a bug?).

It's doable via a small change to the new format of course. This would
require some care in handling the (few) users of pgfoundry hstore-new,
but all that means is that users of the pre-release hstore-new would
have to ensure at least one update of stored data before doing a binary
migrate to 8.5, which I think isn't too much of a burden.

The other option would be to fix the wasted-space bug in the existing
hstore, and document that stored data must be updated at least once
subsequent to that fix before doing a binary migrate to 8.5.

(The pathological case is _very_ rare, requiring an 0-length key with
exactly 32kb of data, followed by a specific series of key lengths
with values of length 1, and with more than 28kbytes of wasted space
at the end of the value, and only on little-endian machines. The only
way to get that much wasted space is to do several thousand iterations
of UPDATE ... SET hs = hs || ''; each of which adds 8 bytes of wasted
space to the end of the value. But even so, somebody, somewhere,
probably has a value that matches...)

-- 
Andrew (irc:RhodiumToad)

-- 
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] revised hstore patch

2009-07-22 Thread David E. Wheeler

On Jul 22, 2009, at 8:55 AM, Andrew Gierth wrote:


The other option would be to fix the wasted-space bug in the existing
hstore, and document that stored data must be updated at least once
subsequent to that fix before doing a binary migrate to 8.5.


Given this…


(The pathological case is _very_ rare, requiring an 0-length key with
exactly 32kb of data, followed by a specific series of key lengths
with values of length 1, and with more than 28kbytes of wasted space
at the end of the value, and only on little-endian machines. The only
way to get that much wasted space is to do several thousand iterations
of UPDATE ... SET hs = hs || ''; each of which adds 8 bytes of wasted
space to the end of the value. But even so, somebody, somewhere,
probably has a value that matches...)


Could it be that only folks with such a manifestation of the bug would  
need to do a binary upgrade? If so, I certainly think it'd be worth it  
to fix the bug.


Best,

David
--
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] change do_tup_output to take Datum arguments rather than cstring

2009-07-22 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Mon, Jun 15, 2009 at 1:20 PM, Tom Lanet...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 it looks like I can probably rip that member out of TupOutputState
 altogether.
 
 Yeah, that's exactly what I was thinking.

 Excellent.  Revised patch attached.

Applied with minor editorialization.

It strikes me that it would now be quite easy to make the
efficiency improvement alluded to in do_tup_output's comment:

 * XXX This could be made more efficient, since in reality we probably only
 * need a virtual tuple.

but I didn't have time to investigate that right now (got to leave for
a dentist appointment :-()

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] revised hstore patch

2009-07-22 Thread Dimitri Fontaine

Hi,

Le 22 juil. 09 à 02:56, Robert Haas a écrit :

On Tue, Jul 21, 2009 at 7:25 PM, Tom Lanet...@sss.pgh.pa.us wrote:

Or maybe we should think about having two versions of hstore.  This
is all tied up in the problem of having a decent module  
infrastructure

(which I hope somebody is working on for 8.5).


I indeed still intend to provide some patch in the 8.5 cycle. While  
the user design issue didn't receive any push back, some big items  
remain to be solved. So here's my current TODO about it:


 - get some more familiar and involved in backend code by being one  
of the RRR

 - consider the proposed syntax ok for a first stab at it
 - make the pg_catalog.pg_extension entry and the associated commands
 with version as text, one thing at a time please
 - bootstrap core components in pg_extension for dependancy on them  
(plpgsql, ...)
 - implement a backend function pg_execute_commands_from_file('path/ 
to/file.sql');

 reserved to superuser, file in usual accepted places
 - implement INSTALL EXTENSION with the previous function
- adding a static backend local variable installing_extension (oid)
- modifying each SQL object create statement to add entries in  
pg_depend
 - add an specific type for handling version numbers and operators  
comparing them


Here are from memory the problems we don't have a solution for yet:
 - how to give user the ability to install the extension's objects in  
another schema than the pg_extension default one
 - how to provide extension author a way to have major PG version  
dependant code without having to implement and maintain a specific  
function in their install.sql file


Please go comment on this other thread if you think the syntax is  
awful or for helping me through the big tickets:

  http://archives.postgresql.org/pgsql-hackers/2009-06/msg01281.php


A decent module infrastructure is probably not going to fix this
problem unless it links with -ldwiw.  There are really only two
options here:


I beg to defer. The way for a decent *extension* facility to handle  
the case is by providing an upgrade function which accepts too  
arguments: old and new version of the module. Then the module author  
is able to run custom code from within the module upgrade transaction,  
where migrating on disk data representation is entirely possible.  
pg_depend would have to allow for easy finding of a given datatype  
column I guess.



(I am also not aware that anyone is working on the module
infrastructure problem, though of course that doesn't mean that no-one
is; but the point is that's neither here nor there as respects the
present problem.  The module infrastructure is just a management layer
around the same underlying issues.)


Of course if anyone wants to join, I'd appreciate. Some have offered  
help and I've been failing to provide them with my todo list... but  
getting a first patch for next commit fest is a goal.


Regards,
--
dim
--
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] revised hstore patch

2009-07-22 Thread Robert Haas
On Wed, Jul 22, 2009 at 1:40 PM, Dimitri Fontainedfonta...@hi-media.com wrote:
 Hi,

 Le 22 juil. 09 à 02:56, Robert Haas a écrit :

 On Tue, Jul 21, 2009 at 7:25 PM, Tom Lanet...@sss.pgh.pa.us wrote:

 Or maybe we should think about having two versions of hstore.  This
 is all tied up in the problem of having a decent module infrastructure
 (which I hope somebody is working on for 8.5).

 I indeed still intend to provide some patch in the 8.5 cycle. While the user
 design issue didn't receive any push back, some big items remain to be
 solved. So here's my current TODO about it:

  - get some more familiar and involved in backend code by being one of the
 RRR
  - consider the proposed syntax ok for a first stab at it
  - make the pg_catalog.pg_extension entry and the associated commands
     with version as text, one thing at a time please
  - bootstrap core components in pg_extension for dependancy on them
 (plpgsql, ...)
  - implement a backend function
 pg_execute_commands_from_file('path/to/file.sql');
     reserved to superuser, file in usual accepted places
  - implement INSTALL EXTENSION with the previous function
    - adding a static backend local variable installing_extension (oid)
    - modifying each SQL object create statement to add entries in pg_depend
  - add an specific type for handling version numbers and operators comparing
 them

 Here are from memory the problems we don't have a solution for yet:
  - how to give user the ability to install the extension's objects in
 another schema than the pg_extension default one
  - how to provide extension author a way to have major PG version dependant
 code without having to implement and maintain a specific function in their
 install.sql file

 Please go comment on this other thread if you think the syntax is awful or
 for helping me through the big tickets:
  http://archives.postgresql.org/pgsql-hackers/2009-06/msg01281.php

 A decent module infrastructure is probably not going to fix this
 problem unless it links with -ldwiw.  There are really only two
 options here:

 I beg to defer. The way for a decent *extension* facility to handle the case
 is by providing an upgrade function which accepts too arguments: old and new
 version of the module. Then the module author is able to run custom code
 from within the module upgrade transaction, where migrating on disk data
 representation is entirely possible. pg_depend would have to allow for easy
 finding of a given datatype column I guess.

 (I am also not aware that anyone is working on the module
 infrastructure problem, though of course that doesn't mean that no-one
 is; but the point is that's neither here nor there as respects the
 present problem.  The module infrastructure is just a management layer
 around the same underlying issues.)

 Of course if anyone wants to join, I'd appreciate. Some have offered help
 and I've been failing to provide them with my todo list... but getting a
 first patch for next commit fest is a goal.

This would have been a good time to start a new thread with a
different subject line.

...Robert

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


Re: [HACKERS] revised hstore patch

2009-07-22 Thread Andrew Gierth
 David == David E Wheeler da...@kineticode.com writes:

  The other option would be to fix the wasted-space bug in the
  existing hstore, and document that stored data must be updated at
  least once subsequent to that fix before doing a binary migrate to
  8.5.

 [...]

 David Could it be that only folks with such a manifestation of the
 David bug would need to do a binary upgrade? If so, I certainly
 David think it'd be worth it to fix the bug.

Let's go through the options.

A)
 - don't fix the wasted-space bug (or don't rely on it, anyway)
 - change the new format to be more distinguishable
  Result:
 - seamless binary upgrade for contrib/hstore users
 - users of unreleased CVS hstore-new will have to ensure all
   values are updated after installing a release version and
   before doing a binary upgrade to 8.5

B)
 - fix the wasted-space bug
 - leave the new format as-is
  Result:
 - seamless binary upgrade for hstore-new users
 - contrib/ users will have to remove wasted space from at least
   any hstore with a zero-length key before doing a binary upgrade

To me (A) is looking like the obvious choice (the people smart enough
to be using hstore-new from CVS already can handle the minor pain of
updating the on-disk format).

Unless I hear any objections I will proceed accordingly...

-- 
Andrew (irc:RhodiumToad)

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


[HACKERS] Extension Facility

2009-07-22 Thread Dimitri Fontaine

Hi,

The same mail as before in a new thread, per Robert comment. Including  
the body rather than an archive link for various reasons, including  
making it easy to comment here rather than there.


Le 22 juil. 09 à 02:56, Robert Haas a écrit :

On Tue, Jul 21, 2009 at 7:25 PM, Tom Lanet...@sss.pgh.pa.us wrote:

Or maybe we should think about having two versions of hstore.  This
is all tied up in the problem of having a decent module  
infrastructure

(which I hope somebody is working on for 8.5).


I indeed still intend to provide some patch in the 8.5 cycle. While  
the user design issue didn't receive any push back, some big items  
remain to be solved. So here's my current TODO about it:


- get some more familiar and involved in backend code by being one of  
the RRR

- consider the proposed syntax ok for a first stab at it
- make the pg_catalog.pg_extension entry and the associated commands
with version as text, one thing at a time please
- bootstrap core components in pg_extension for dependancy on them  
(plpgsql, ...)
- implement a backend function pg_execute_commands_from_file('path/to/ 
file.sql');

reserved to superuser, file in usual accepted places
- implement INSTALL EXTENSION with the previous function
   - adding a static backend local variable installing_extension (oid)
   - modifying each SQL object create statement to add entries in  
pg_depend
- add an specific type for handling version numbers and operators  
comparing them


Here are from memory the problems we don't have a solution for yet:
- how to give user the ability to install the extension's objects in  
another schema than the pg_extension default one
- how to provide extension author a way to have major PG version  
dependant code without having to implement and maintain a specific  
function in their install.sql file


Please go comment on this other thread if you think the syntax is  
awful or for helping me through the big tickets:

 http://archives.postgresql.org/pgsql-hackers/2009-06/msg01281.php


A decent module infrastructure is probably not going to fix this
problem unless it links with -ldwiw.  There are really only two
options here:


I beg to defer. The way for a decent *extension* facility to handle  
the case is by providing an upgrade function which accepts too  
arguments: old and new version of the module. Then the module author  
is able to run custom code from within the module upgrade transaction,  
where migrating on disk data representation is entirely possible.  
pg_depend would have to allow for easy finding of a given datatype  
column I guess.



(I am also not aware that anyone is working on the module
infrastructure problem, though of course that doesn't mean that no-one
is; but the point is that's neither here nor there as respects the
present problem.  The module infrastructure is just a management layer
around the same underlying issues.)


Of course if anyone wants to join, I'd appreciate. Some have offered  
help and I've been failing to provide them with my todo list... but  
getting a first patch for next commit fest is a goal.


Regards,
--
dim


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


[HACKERS] SRPMs?

2009-07-22 Thread Andrew Dunstan


Where are the SRPMs to go with the binary RPMs on our download sites (or 
for that matter on yum.pgsqlrpms.org). ISTM we should not be publishing 
binary RPMs without simultaneously publishing the corresponding SRPMs.


cheers

andrew

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


Re: [HACKERS] psql - small fix in \du

2009-07-22 Thread Andreas Wenk

Tom Lane wrote:

Andreas Wenk a.w...@netzmeister-st-pauli.de writes:
I am not sure when the function shobj_description(oid, name) is giving a 
result.


That retrieves the comment for the object (the role, in this case).

regards, tom lane


Tom, thank you. I will provide an updated patch tomorrow.

Cheers

Andy

--
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] GEQO vs join order restrictions

2009-07-22 Thread Andres Freund
Hi Tom, Robert, Hi all,
nks,
On Sunday 19 July 2009 19:23:18 Tom Lane wrote:
 Andres Freund and...@anarazel.de writes:
  On Saturday 18 July 2009 17:48:14 Tom Lane wrote:
  I'm inclined to address this by rewriting gimme_tree so that it *always*
  finds a valid join order based on the given tour.  This would involve
  searching the whole stack for a possible join partner instead of
  considering only the topmost stack entry.  It sounds inefficient, but
  it's not nearly as bad as wasting the entire cycle by failing near
  the end, which is what happens now.
 
  I guess this should be relatively easy to implement and test?
 With the patch, GEQO manages to bumble through and produce a plan
 even at high collapse limits.  It's a whole lot slower than the
 regular planner, and I found out that this time consists largely
 of desirable_join() checks in gimme_tree().  I said earlier that
 the regular planner does that too ... but GEQO is doing it a lot
 more, because it repeats the whole planning cycle 500 times.
 In previous tests we've seen regular planner runtime cross over
 GEQO time around collapse_limit 12.  It seems the reason that
 this case is so different is that the total problem is so much
 larger, and so there is a very large equivalence-class list
 (1289 items!) that gets trawled through in each desirable_join check.
 That's why have_relevant_eclass_joinclause shows up so high in oprofile.

 My conclusions are:
 1.  I should clean up and apply the attached patch.  Even though it's
 not the whole answer, it clearly makes things a good deal better.
I did some testing with the original queries and the unsurprising result is, 
that the planning time is *hugely* smaller (multiple orders of magnitude) and 
the execution time is bigger (~15% ) with the few variation of settings I 
tested.

Many unplanable queries are now planable. 

Thanks,

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] extension facility (was: revised hstore patch)

2009-07-22 Thread Robert Haas
On Tue, Jul 21, 2009 at 8:56 PM, Robert Haasrobertmh...@gmail.com wrote:
 A decent module infrastructure is probably not going to fix this
 problem unless it links with -ldwiw.  There are really only two
 options here:

 - Keep the old version around for compatibility and add a new version
 that isn't compatible, plus provide a migration path from the old
 version to the new version.

 - Make the new version read the format written by the old version.

On Wed, Jul 22, 2009 at 1:40 PM, Dimitri Fontainedfonta...@hi-media.com wrote:
 I beg to defer. The way for a decent *extension* facility to handle the case
 is by providing an upgrade function which accepts too arguments: old and new
 version of the module. Then the module author is able to run custom code
 from within the module upgrade transaction, where migrating on disk data
 representation is entirely possible. pg_depend would have to allow for easy
 finding of a given datatype column I guess.

Technically I suppose you're right,  but I don't think that's going to
work very well in practice.  The whole point of binary upgrade is that
you have a really big database where dump and reload is not practical.
 The numbers we've seen for pg_migrator make copy mode look WAY slower
than link mode, and that's without doing any transformation on the
data.

If you make the new code read the old on-disk format, you can upgrade
your data a bit at a time if you want to.  You can, for example, write
a script to do a no-op update on a few thousand tuples every hour
until they're all updated.  That's really important for big datasets.

If you keep an old and a new version of the datatype, you can't
upgrade a tuple at a time, but you can at least upgrade one column at
a time, which is still better than a kick in the head.

If you make the extension-upgrade facility rewrite everything, you
have to do your entire cluster in one shot.  That will work for some
people, but not for all.  And unless you ship both versions of hstore
with either PG 8.4 or PG 8.5, you're going to need the conversion to
be done inside pg_migrator, which introduces a whole new level of
complexity that I think we'd be better off without.

...Robert

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


[HACKERS] Aggregate-function space leakage

2009-07-22 Thread Tom Lane
I looked into Chris Spotts' recent report of massive memory leakage in
8.4, in a case involving array_agg() executed in a GROUP BY query:
http://archives.postgresql.org/pgsql-general/2009-07/msg00858.php
The reason for that turns out to be that we deliberately lobotomized
array_agg that way, just last month:
http://archives.postgresql.org/pgsql-committers/2009-06/msg00259.php
in response to this problem:
http://archives.postgresql.org/pgsql-hackers/2009-06/msg01186.php

We need a better idea.

One possibility is to have nodeAgg.c reset the aggcontext between
groups when in group mode.  I think this would be more than a one-liner
fix because it probably has pointers to transvalues that are in that
context, but it's surely doable.

Another possibility is to modify array_agg (and other aggregates that
try to use aggcontext) so that they distinguish hash- and group-mode
aggregation.  If we had array_agg attach its working context to the Agg
node's per-output-tuple context instead of aggcontext when in group
mode, I think everything would work as desired.  The main disadvantage
of this is modifying a coding convention that third-party code probably
is using.  (But OTOH, we have already required any such code to change
for 8.4.)

I haven't tested either of the above ideas --- just brainstorming
at this point.

Comments, better ideas?

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] Aggregate-function space leakage

2009-07-22 Thread Jeff Davis
On Wed, 2009-07-22 at 17:14 -0400, Tom Lane wrote:
 One possibility is to have nodeAgg.c reset the aggcontext between
 groups when in group mode.  I think this would be more than a one-liner
 fix because it probably has pointers to transvalues that are in that
 context, but it's surely doable.

Is there a disadvantage to this approach, or is it just more work?

Regards,
Jeff Davis


-- 
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] Aggregate-function space leakage

2009-07-22 Thread Tom Lane
Jeff Davis pg...@j-davis.com writes:
 On Wed, 2009-07-22 at 17:14 -0400, Tom Lane wrote:
 One possibility is to have nodeAgg.c reset the aggcontext between
 groups when in group mode.  I think this would be more than a one-liner
 fix because it probably has pointers to transvalues that are in that
 context, but it's surely doable.

 Is there a disadvantage to this approach, or is it just more work?

Don't know ... haven't tried to do it yet.

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] Aggregate-function space leakage

2009-07-22 Thread Greg Stark
On Wed, Jul 22, 2009 at 10:14 PM, Tom Lanet...@sss.pgh.pa.us wrote:
 The reason for that turns out to be that we deliberately lobotomized
 array_agg that way, just last month:
 http://archives.postgresql.org/pgsql-committers/2009-06/msg00259.php
 in response to this problem:
 http://archives.postgresql.org/pgsql-hackers/2009-06/msg01186.php

 We need a better idea.


Rereading your diagnosis of Merlin Moncure's original problem I'm a
bit puzzled. Why do we have to rerun the final function when we rescan
the hash table? Surely the logical thing to do is to store the final
value in the hash table with some flag saying that value has been
finalized rather than to reexecute the final function every time it's
rescanned.

I'm not sure that really solves anything though since there's no
guarantee that the first scan was finished when it's reset so there
could still be unfinalized elements in the hash table. Would it be too
costly to finalize all the hash elements in a single pass before
returning any?

-- 
greg
http://mit.edu/~gsstark/resume.pdf

-- 
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] Determining client_encoding from client locale

2009-07-22 Thread Jaime Casanova
On Mon, Jul 6, 2009 at 10:00 AM, Heikki
Linnakangasheikki.linnakan...@enterprisedb.com wrote:
 Here's my first attempt at setting client_encoding automatically from
 locale.

 It adds a new conninfo parameter to libpq, client_encoding. If set to
 auto, libpq uses the encoding as returned by
 pg_get_encoding_from_locale(). Any other value is passed through to the
 server as is.


i was trying to test this and make a simple program based on the first
libpq example that only shows client_encoding

this little test compiles fine until i applied your patch :(

postg...@casanova1:~/pg_releases/pgtests$ gcc -o test-libpq
test-libpq.o -L/usr/local/pgsql/head/lib -lpq
/usr/local/pgsql/head/lib/libpq.so: undefined reference to
`pg_get_encoding_from_locale'
collect2: ld returned 1 exit status

just in case i attached the test program.

-- 
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157
/*
 * testlibpq.c
 *
 *  Test the C version of libpq, the PostgreSQL frontend library.
 */
#include stdio.h
#include stdlib.h
#include libpq-fe.h

static void
exit_nicely(PGconn *conn)
{
PQfinish(conn);
exit(1);
}

int
main(int argc, char **argv)
{
const char *conninfo;
PGconn *conn;
PGresult   *res;

/*
 * If the user supplies a parameter on the command line, use it as the
 * conninfo string; otherwise default to setting dbname=postgres and using
 * environment variables or defaults for all other connection parameters.
 */
if (argc  1)
conninfo = argv[1];
else
conninfo = dbname = postgres;

/* Make a connection to the database */
conn = PQconnectdb(conninfo);

/* Check to see that the backend connection was successfully made */
if (PQstatus(conn) != CONNECTION_OK)
{
fprintf(stderr, Connection to database failed: %s,
PQerrorMessage(conn));
exit_nicely(conn);
}

	/* i wanna know what the client_encoding is */
res = PQexec(conn, SHOW client_encoding);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
{
fprintf(stderr, SHOW command failed: %s, PQerrorMessage(conn));
PQclear(res);
exit_nicely(conn);
}

printf(client_encoding: %s, PQgetvalue(res, 0, 0));
printf(\n);

PQclear(res);

/* close the connection to the database and cleanup */
PQfinish(conn);

return 0;
}

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


[HACKERS] join regression failure on cygwin

2009-07-22 Thread Andrew Dunstan


My Cygwin buildfarm member started failing (hanging, in fact) recently. 
It seems to hang consistently in join.sql and the only way I can get it 
to complete is to kill the backend fairly violently. See 
http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=brown_batdt=2009-07-22%2023:10:21 
It looks like this is the result of the semi-join ordering bug fix Tom 
applied a few days ago, but it is building and running 8.4 quite 
happily, and I think that branch got the same changes, so I'm not quite 
sure about it.


I'm going to try reversing that change locally to see if it fixes the 
problem. Unfortunately, this isn't an easy platform to debug.


cheers

andrew

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


Re: [HACKERS] Determining client_encoding from client locale

2009-07-22 Thread Alvaro Herrera
Jaime Casanova wrote:

 this little test compiles fine until i applied your patch :(
 
 postg...@casanova1:~/pg_releases/pgtests$ gcc -o test-libpq
 test-libpq.o -L/usr/local/pgsql/head/lib -lpq
 /usr/local/pgsql/head/lib/libpq.so: undefined reference to
 `pg_get_encoding_from_locale'

Do you have an older version of libpq.so around?

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
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] [PATCH] DefaultACLs

2009-07-22 Thread Robert Haas
On Fri, Jul 17, 2009 at 9:46 PM, Joshua Tolleyeggyk...@gmail.com wrote:
 On Tue, Jul 14, 2009 at 11:10:00PM +0200, Petr Jelinek wrote:
 Hello,

 this is first public version of our DefaultACLs patch as described on
 http://wiki.postgresql.org/wiki/DefaultACL .

 Ok, here's my first crack at a comprehensive review. There's more I need to
 look at, eventually. Some of these are very minor stylistic comments, and some
 are probably just because I've much less of a clue, in general, than I'd like
 to think I have.

 First, as you've already pointed out, this needs documentation.

 Once I added the missing semicolon mentioned earlier, it applies and builds
 fine. The regression tests, however, seem to assume that they'll be run as the
 postgres user, and the privileges test failed. Here's part of a diff between
 expected/privileges.out and results/privileges.out as an example of what I
 mean:

  ALTER SCHEMA regressns DROP DEFAULT PRIVILEGES ON TABLE ALL FROM
 regressuser2;
 ***
 *** 895,903 
  (1 row)

  SELECT relname, relacl FROM pg_class WHERE relname = 'acltest2';
 !  relname  |                        relacl
 ! --+--
 !  acltest2 | {postgres=arwdDxt/postgres,regressgroup1=r/postgres}
  (1 row)

  CREATE FUNCTION regressns.testfunc1() RETURNS int AS 'select 1;' LANGUAGE
 sql;
 --- 895,903 
  (1 row)

  SELECT relname, relacl FROM pg_class WHERE relname = 'acltest2';
 !  relname  |                  relacl
 ! --+--
 !  acltest2 | {josh=arwdDxt/josh,regressgroup1=r/josh}
  (1 row)

  CREATE FUNCTION regressns.testfunc1() RETURNS int AS 'select 1;' LANGUAGE
 sql;

 Very minor stylistic or comment issues:

 * There's a stray newline added in pg_class.h (no other changes were made to
  that file by this patch)
 * It feels to me like the comment Continue with standard grant in aclchk.c
  interrupts the flow of the code, though such a comment was likely useful
  when the patch was being written.
 * pg_namespace_default_acl.h:71 should read objects stored *in* pg_class
 * The comment at the beginning of InsertPgClassTuple() in catalog/heap.c
  should probably be updated to say that relation's ACLs aren't always NULL by
  default
 * copy_from in gram.y got changed to to_from, but to_from isn't ever used in
  the default ACL grammar. I wondered if this was changed so you could use the
  same TO/FROM code as COPY uses, and then you decided to hardcode TO and FROM?

 In my perusal of the patch, I didn't see any code that screamed at me as
 though it were a bad idea; quite likely there weren't any really bad ideas but
 I can't say with confidence I'd have spotted them if there were. The addition
 of both the NSPDEFACLOBJ_* defines alongside the NSP_ACL_OBJ_* defines kinda
 made me think there were too many sets of constants that had to be kept in
 sync, but I'm not sure that's much of an issue in reality, given how unlikely
 it is that schema object types to which default ACLs should apply are likely
 to be added or removed.

 I don't know how patches that require catalog version changes are generally
 handled; should the patch include that change?

 More testing to follow.

So are these warts fixed in the latest revision of this patch?

http://archives.postgresql.org/pgsql-hackers/2009-07/msg01216.php

I am gathering that this patch is still a bit of a WIP.  I think it
might be best to mark it returned with feedback and let Petr resubmit
for the next CommitFest when it is closer to being done.  But I don't
want to do that if it's really already to go now.

I am also a bit unsure as to whether Josh Tolley is still conducting a
more in-depth review.  Josh?

Thanks,

...Robert

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


Re: [HACKERS] [PATCH 1/2 v3] [libpq] rework sigpipe-handling macros

2009-07-22 Thread Robert Haas
 On Mon, Jul 20, 2009 at 3:14 AM, Jeremy Kerrj...@ozlabs.org wrote:
 That code is not broken as it stands, and doesn't appear to really
 gain anything from the proposed change.  Why should we risk any
 portability questions when the code isn't going to get either simpler
 or shorter?

 OK, after attempting a macro version of this, we end up with:

 #define DECLARE_SIGPIPE_INFO(var) struct sigpipe_info var;

 #define DISABLE_SIGPIPE(info, conn) \
        ((SIGPIPE_MASKED(conn)) ? 0 : \
                ((info)-got_epipe = false, \
                pg_block_sigpipe((info)-oldsigmask, 
 (info)-sigpipe_pending))  0)

 - kinda ugly, uses the ?: and , operators to return the result of
 pg_block_sigpipe. We could avoid the comma with a block though.

 If we want to keep the current 'failaction' style of macro:

 #define DISABLE_SIGPIPE(info, conn, failaction) \
   do { \
                if (!SIGPIPE_MASKED(conn)) { \
                        (info)-got_epipe = false; \
                        if (pg_block_sigpipe((info)-oldsigmask, \
                                        (info)-sigpipe_pending))  0) { \
                                failaction; \
                        } \
                } \
        } while (0)

 We could ditch struct sigpipe info, but that means we need to declare
 three separate vars in DECLARE_SIGPIPE_INFO() instead of the one, and it
 doesn't clean up the macro code much. I'd rather reduce the amount of
 stuff that we declare behind the caller's back.

 Compared to the static-function version:

 static inline int disable_sigpipe(PGconn *conn, struct sigpipe_info
 *info)
 {
        if (sigpipe_masked(conn))
                return 0;

        info-got_epipe = false;
        return pq_block_sigpipe(info-oldsigmask, info-sigpipe_pending)  0;
 }

 Personally, I think the static functions are a lot cleaner, and don't
 think we lose any portability from using these (since inline is #define-
 ed out on compilers that don't support it). On non-inlining compilers,
 we do gain an extra function call, but we're about to enter the kernel
 anyway, so this will probably be lost in the noise (especially if we
 save the sigpipe-masking syscalls).

 But in the end, it's not up to me - do you still prefer the macro
 approach?

Since Tom seems to prefer the macro approach, and since the current
code uses a macro, I think we should stick with doing it that way.

Also, as some of Tom's comments above indicate, I don't think it's
making anything any easier for anyone that you keep submitting this as
two separate patches.  It's one thing to submit a patch in pieces of
it is very large or complex and especially if the pieces are
independent, but that's not really the case here.

Because we are now over a week into this CommitFest, we need to get a
final, reviewable version of this patch as quickly as possible.  So
please make the requested changes and resubmit as soon as you can.

Thanks,

...Robert

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


Re: [HACKERS] [PATCH] Psql List Languages

2009-07-22 Thread Robert Haas
On Sun, Jul 19, 2009 at 4:00 AM, Peter Eisentrautpete...@gmx.net wrote:
 Please submit an updated patch.

Fernando,

If you would like to have this change committed during this
CommitFest, please submit an updated patch ASAP.  Otherwise, you can
resubmit for the next CommitFest in September.

Thanks,

...Robert

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


Re: [HACKERS] [PATCH] Psql List Languages

2009-07-22 Thread Greg Stark
On Thu, Jul 23, 2009 at 2:23 AM, Robert Haasrobertmh...@gmail.com wrote:
 If you would like to have this change committed during this
 CommitFest, please submit an updated patch ASAP.  Otherwise, you can
 resubmit for the next CommitFest in September.


You know, I don't think we have any rules against people responding to
patches submitted outside commitfest periods...

-- 
greg
http://mit.edu/~gsstark/resume.pdf

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


Re: [HACKERS] [PATCH] Psql List Languages

2009-07-22 Thread Robert Haas
On Wed, Jul 22, 2009 at 9:35 PM, Greg Starkgsst...@mit.edu wrote:
 On Thu, Jul 23, 2009 at 2:23 AM, Robert Haasrobertmh...@gmail.com wrote:
 If you would like to have this change committed during this
 CommitFest, please submit an updated patch ASAP.  Otherwise, you can
 resubmit for the next CommitFest in September.

 You know, I don't think we have any rules against people responding to
 patches submitted outside commitfest periods...

I never suggested otherwise.  In fact, I think it works much better
when people DO respond to patches submitted outside CommitFests.
What's your point?

...Robert

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


Re: [HACKERS] Upgrading our minimum required flex version for 8.5

2009-07-22 Thread Greg Smith
I got bit by this tonight as part of testing a patch on CentOS 5, which 
like RHEL 5 still ships flex 2.5.4.  I just wrote a little guide on how to 
grab a source RPM from a Fedora version and install it to work around that 
problem: 
http://notemagnet.blogspot.com/2009/07/upgrading-flex-from-source-rpm-to.html


Kind of annoying, but as special software you have to install on a server 
just to build something from CVS goes it's only a minor inconvenience.


--
* Greg Smith gsm...@gregsmith.com http://www.gregsmith.com Baltimore, MD

--
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] join regression failure on cygwin

2009-07-22 Thread Andrew Dunstan

I wrote:


My Cygwin buildfarm member started failing (hanging, in fact) 
recently. It seems to hang consistently in join.sql and the only way I 
can get it to complete is to kill the backend fairly violently. See 
http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=brown_batdt=2009-07-22%2023:10:21 
It looks like this is the result of the semi-join ordering bug fix Tom 
applied a few days ago, but it is building and running 8.4 quite 
happily, and I think that branch got the same changes, so I'm not 
quite sure about it.


I'm going to try reversing that change locally to see if it fixes the 
problem. Unfortunately, this isn't an easy platform to debug.





Nope, it wasn't that. So my next candidate is the prior 
join_is_legal/GEQO changes. But reverting that would get rid of the new 
regression test where we seem to be failing, so I'm not quite sure where 
to go on it.


cheers

andrew

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


Re: [HACKERS] [PATCH] Psql List Languages

2009-07-22 Thread Robert Haas
On Wed, Jul 22, 2009 at 9:43 PM, Robert Haasrobertmh...@gmail.com wrote:
 On Wed, Jul 22, 2009 at 9:35 PM, Greg Starkgsst...@mit.edu wrote:
 On Thu, Jul 23, 2009 at 2:23 AM, Robert Haasrobertmh...@gmail.com wrote:
 If you would like to have this change committed during this
 CommitFest, please submit an updated patch ASAP.  Otherwise, you can
 resubmit for the next CommitFest in September.

 You know, I don't think we have any rules against people responding to
 patches submitted outside commitfest periods...

 I never suggested otherwise.  In fact, I think it works much better
 when people DO respond to patches submitted outside CommitFests.
 What's your point?

To follow up on that thought a little more...

The patch author is certainly free to resubmit the patch at any time,
and Peter (or any other committer) can commit it at any time, and may
well do so.  However, as far as the CommitFest process is concerned,
we need to wrap things up within a finite time period that is not very
long, and that can only happen if both reviews and resubmits happen in
a timely fashion, so, since I've been tapped to manage this
CommitFest, I'm trying to do what I can to keep on top of that.

If my email struck you as rude, I certainly apologize for that.  I'm
trying really hard to be efficient about this without stepping on
anyone's feelings, but that's a fine line to walk and I'm not sure
I'll always be on the right side of it.

Also, as a practical matter, while patches CAN get committed at any
given time, one of the purposes of CommitFests, at least AIUI, is to
give reviewers and committers a break from dealing with other people's
patches.  In other words, when it is NOT time for a CommitFest, as Tom
put it, people shouldn't feel guilty about working on their own
patches rather than reviewing.  When it IS time for a CommitFest, they
should.  So the time when patches are *most likely* to be committed is
during a CommitFest, and that is why I think there would be a benefit
to the author of this patch in resubmitting it soon.

I am trying to introduce something of a procedural change from the
last CommitFest in that I think that the deadline for any particular
patch to be resubmitted should be based at least in part on when it
was reviewed, and not just on how many days we have left in the
CommitFest, to avoid a huge pile of resubmissions just before some
arbitrary cutoff, which then creates an unsupportable burden for
reviewers and committers, or else just makes the CommitFest drag on
and on.  I think that's pretty reasonable.  In fact, it's far MORE
reasonable than what we did last time, where patches that got reviewed
early sat around and were resubmitted at a very liesurely pace over a
period of months, while other patches that got reviewed late got
looked at once (or maybe twice) and then punted.

Hopefully that seems reasonable - if not, let's discuss (perhaps after
changing to a new thread).

...Robert

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


Re: [HACKERS] multi-threaded pgbench

2009-07-22 Thread Greg Smith
I just took multi-threaded pgbench for an initial spin, looks good overall 
with only a couple of small rough edges.


The latest code works differently depending on whether you compiled with 
--enable-thread-safety or not, it defines some structures based on fork if 
it's not enabled:


#elif defined(ENABLE_THREAD_SAFETY)
#include pthread.h
#else
#include sys/wait.h
typedef struct fork_pthread*pthread_t;
typedef int pthread_attr_t;
static int pthread_create(pthread_t *thread, pthread_attr_t *attr, void 
* (*start_routine)(void *), void * arg);

static int pthread_join(pthread_t th, void **thread_return);
#endif

That second code path, when --enable-thread-safety is turned off, crashes 
and burns on my Linux system:


gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith 
-Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -fwrapv 
-I../../src/interfaces/libpq -I. -I../../src/include -D_GNU_SOURCE   -c -o 
pgbench.o pgbench.c -MMD -MP -MF .deps/pgbench.Po

pgbench.c:72: error: conflicting types for pthread_t
/usr/include/bits/pthreadtypes.h:50: error: previous declaration of 
pthread_t was here

pgbench.c:73: error: conflicting types for pthread_attr_t
/usr/include/bits/pthreadtypes.h:57: error: previous declaration of 
pthread_attr_t was here


So that's the first problem to sort out, I was planning to test that path 
as well as the regular threaded one.  Since I'd expect there to be Linux 
packages built both with and without thread safety enabled, they both 
should work, even though people should always be turning safety on 
nowadays.


We should try to get a Windows based tester here too at some point, 
there's a completely different set of thread wrapper code for that OS that 
could use a look by somebody more familiar than me with that platform.


The second thing that concerns me is that there's a limitation in the code 
where the number of clients must be a multiple of the number of workers. 
When I tried to gradually step up the client volume the tests wouldn't 
run:


$ ./pgbench -j 16 -S -c 24 -t 1 pgbench
number of clients (24) must be a multiple number of threads (16)

Once the larger issues are worked out, I would be much friendlier if it 
were possible to pass new threads a client count so that the last in the 
pool could service a smaller number.  The logic for that is kind of a 
pain--in this case you'd want 8 threads running 2 clients each while 8 ran 
1 client--but it would really be much friendlier and flexible that way.


Onto performance.  My test system has a 16 cores of Xeon X5550 @ 2.67GHz. 
I created a little pgbench database (-s 10) and used the default 
postgresql.conf parameters for everything but max_connections for a rough 
initial test.


Performance on this box drops off pretty fast once you get past 16 
clients; using the original, unpatched pgbench:


c   tps
16  86887
24  70685
32  63787
64  64712
128 60602

A quick test of the new version suggest that there's no glaring 
performance regression running it with a single client thread:


$ ./pgbench.orig -S -c 64 -t 1 pgbench
tps = 64712.451737 (including connections establishing)

$ ./pgbench -S -c 64 -t 1 pgbench
tps = 63806.494046 (including connections establishing)

So I moved onto to testing with a worker thread per CPU:

./pgbench -j 16 -S -c 16 -t 10 pgbench
./pgbench -j 16 -S -c 32 -t 5 pgbench
./pgbench -j 16 -S -c 64 -t 1 pgbench
./pgbench -j 16 -S -c 128 -t 1 pgbench

And got considerably better results:

c  tps
16  96223
32  89014
64  82487
128 74217

That's as much as a 40% speedup @ 32 clients, and even a decent win at 
lower counts.


The patch looks like it accomplishes its performance goals quite well 
here.  I'll be glad to run some more extensive performance tests, but I'd 
like to at least see the version without --enable-thread-safety fixed 
first so that I can queue up and compare both versions when I go through 
that.


--
* Greg Smith gsm...@gregsmith.com http://www.gregsmith.com Baltimore, MD

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


Re: [HACKERS] [PATCH] Psql List Languages

2009-07-22 Thread Greg Stark
On Thu, Jul 23, 2009 at 3:11 AM, Robert Haasrobertmh...@gmail.com wrote:

 If my email struck you as rude, I certainly apologize for that.  I'm
 trying really hard to be efficient about this without stepping on
 anyone's feelings, but that's a fine line to walk and I'm not sure
 I'll always be on the right side of it.

I didn't think your email was rude at all. Just that it could be
(mis)construed to mean that if a patch isn't committed now the only
time they'll get feedback again is next commitfest.

-- 
greg
http://mit.edu/~gsstark/resume.pdf

-- 
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] Determining client_encoding from client locale

2009-07-22 Thread Jaime Casanova
On Wed, Jul 22, 2009 at 7:30 PM, Alvaro
Herreraalvhe...@commandprompt.com wrote:
 Jaime Casanova wrote:

 this little test compiles fine until i applied your patch :(

 postg...@casanova1:~/pg_releases/pgtests$ gcc -o test-libpq
 test-libpq.o -L/usr/local/pgsql/head/lib -lpq
 /usr/local/pgsql/head/lib/libpq.so: undefined reference to
 `pg_get_encoding_from_locale'

 Do you have an older version of libpq.so around?


the one that installed with 8.4.0 but i thougth that when you specify
-L to gcc you're telling it where to pick libraries from, no?

-- 
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

-- 
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] generic explain options v3

2009-07-22 Thread Robert Haas
On Tue, Jul 21, 2009 at 10:29 PM, Robert Haasrobertmh...@gmail.com wrote:
 On Tue, Jul 21, 2009 at 10:05 PM, Tom Lanet...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Tue, Jul 21, 2009 at 7:47 PM, Tom Lanet...@sss.pgh.pa.us wrote:
 Also, I'd suggest changing the ExplainStmt struct to have a list of
 DefElem options instead of hard-wiring the option set at that level.

 What is the advantage of that?

 Fewer places to change when you add a new option; in particular, not
 copyfuncs or equalfuncs.  Also, the way you are doing it is gratuitously
 unlike every other command that has similar issues to deal with.
 Everybody else parses their DefElem list at execution time.  I think
 you should have the legacy ANALYZE and VERBOSE syntax elements generate
 DefElem list members that get examined at execution.

 Not having to touch copyfuncs or equalfuncs for future options is a
 definite plus, so I'll rework along these lines.

Ugh.  I took a look at this and it turns out that there are some
tentacles.  It doesn't seem very sane to actually do anything with a
list of DefElem nodes, so we really need to parse that list and
convert it to a more sensible format right away (this also seems
important for proper error checking).

The natural place to do this would be in ExplainPrintPlan(), which is
already copying the relevant fields from the ExplainStmt over into an
ExplainState, but that's too far down the call tree, which (for a
non-utility statement when ExplainOneQuery_hook is null) looks like
this:

ExplainQuery - ExplainOneQuery - ExplainOnePlan - ExplainPrintPlan

The obvious solution to that is to create the ExplainState sooner,
back up at the ExplainQuery level.  If we do that, though, then
ExplainState will need to become a public API, because
contrib/auto_explain calls ExplainPrintPlan().  And if we do that,
then probably we should declare it in include/nodes/execnodes.h and
make it a node type...  and if we do that then we'll be back to a
copyfuncs/equalfuncs change every time we add a flag.

Now that's not to say there's no advantage in the proposed refactoring
- it's still more consistent with the way things are done elsewhere.
But since it's going to be a fair amount of work and fail to achieve
one of the two goals you set forth for it, I'd like to get
confirmation before proceeding if possible, and any suggestions you
may have for how to make it as clean as possible.

Thanks,

...Robert

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


Re: [HACKERS] [PATCH] DefaultACLs

2009-07-22 Thread Joshua Tolley
On Wed, Jul 22, 2009 at 08:54:19PM -0400, Robert Haas wrote:
 I am gathering that this patch is still a bit of a WIP. 

I don't consider it a WIP. Petr posted a patch a couple of days ago, but I've
not been able to verify its changes or perform some additional testing I had
in mind, because of my own schedule. I probably should have made that clear a
while ago. I consider the ball entirely in my court on this one, and plan to
complete my review using the latest version of the patch as soon as my time
permits; I expect this will happen before Saturday.

 I am also a bit unsure as to whether Josh Tolley is still conducting a
 more in-depth review.  Josh?

Yes, I am, but if you've read this far you know that already :)

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com


signature.asc
Description: Digital signature


Re: [HACKERS] multi-threaded pgbench

2009-07-22 Thread Itagaki Takahiro

Greg Smith gsm...@gregsmith.com wrote:

 That second code path, when --enable-thread-safety is turned off, crashes 
 and burns on my Linux system:

It comes from confliction of identifiers.
Renaming identifiers with #define can solve the errors:

#define pthread_t   pg_pthread_t
#define pthread_attr_t  pg_pthread_attr_t
#define pthread_create  pg_pthread_create
#define pthread_joinpg_pthread_join
typedef struct fork_pthread*pthread_t;
...

Another idea is that we don't use pthread and add 'pg_thread' wrapper
module on the top of pthread.

We can choose either of implementations... Which is better?


 $ ./pgbench -j 16 -S -c 24 -t 1 pgbench
 number of clients (24) must be a multiple number of threads (16)

It's hard on forking-thread platforms because multiple threads need
to access the job queue. We need to put the queue on inter-process
shared memory, but it introduces additional complexities.

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center



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


Re: [HACKERS] [PATCH] DefaultACLs

2009-07-22 Thread Petr Jelinek

Robert Haas wrote:

So are these warts fixed in the latest revision of this patch?

http://archives.postgresql.org/pgsql-hackers/2009-07/msg01216.php

I am gathering that this patch is still a bit of a WIP.  I think it
might be best to mark it returned with feedback and let Petr resubmit
for the next CommitFest when it is closer to being done.  But I don't
want to do that if it's really already to go now.
  
See http://archives.postgresql.org/pgsql-hackers/2009-07/msg01151.php 
(the revision before the one you pasted).
The docs are not complete but that's up to Stephen, otherwise the patch 
should be finished. But I am not the reviewer :)


Anyway, while this patch might not necessary get commited in this commit 
fest, I'd still like to have opinion from one of the commiters on the 
VIEW problem which also affects grant on all patch ( see 
http://archives.postgresql.org/pgsql-hackers/2009-07/msg00957.php ) and 
I fear returned with feedback might prevent that until next commit fest.


--
Regards
Petr Jelinek (PJMODOS)


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


Re: [HACKERS] [PATCH] DefaultACLs

2009-07-22 Thread Robert Haas
On Wed, Jul 22, 2009 at 11:21 PM, Joshua Tolleyeggyk...@gmail.com wrote:
 On Wed, Jul 22, 2009 at 08:54:19PM -0400, Robert Haas wrote:
 I am gathering that this patch is still a bit of a WIP.

 I don't consider it a WIP. Petr posted a patch a couple of days ago, but I've
 not been able to verify its changes or perform some additional testing I had
 in mind, because of my own schedule. I probably should have made that clear a
 while ago. I consider the ball entirely in my court on this one, and plan to
 complete my review using the latest version of the patch as soon as my time
 permits; I expect this will happen before Saturday.

OK, I stand corrected.  Thanks for the update.

...Robert

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


Re: [HACKERS] [PATCH] DefaultACLs

2009-07-22 Thread Robert Haas
On Wed, Jul 22, 2009 at 11:26 PM, Petr Jelinekpjmo...@pjmodos.net wrote:
 Robert Haas wrote:

 So are these warts fixed in the latest revision of this patch?

 http://archives.postgresql.org/pgsql-hackers/2009-07/msg01216.php

 I am gathering that this patch is still a bit of a WIP.  I think it
 might be best to mark it returned with feedback and let Petr resubmit
 for the next CommitFest when it is closer to being done.  But I don't
 want to do that if it's really already to go now.


 See http://archives.postgresql.org/pgsql-hackers/2009-07/msg01151.php (the
 revision before the one you pasted).

Ah, woops.  Sorry I missed that.  Actually, now that I read it, I
remember that I saw it before, but I didn't remember that before
sending my previous email, of course...

 The docs are not complete but that's up to Stephen, otherwise the patch
 should be finished. But I am not the reviewer :)

Well, perhaps we had better prod Stephen then, since complete docs are
a requirement for commit.

 Anyway, while this patch might not necessary get commited in this commit
 fest, I'd still like to have opinion from one of the commiters on the VIEW
 problem which also affects grant on all patch ( see
 http://archives.postgresql.org/pgsql-hackers/2009-07/msg00957.php ) and I
 fear returned with feedback might prevent that until next commit fest.

OK, hopefully one of them will chime in with an opinion.  As I say, I
would like to see this get committed if it's done, I just wasn't sure
it was.  But now it seems that was due to insufficiently careful
reading of the thread, with the exception of this issue and the need
to finish the docs.

Thanks,

...Robert

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


Re: [HACKERS] [PATCH] DefaultACLs

2009-07-22 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote:
 On Wed, Jul 22, 2009 at 11:26 PM, Petr Jelinekpjmo...@pjmodos.net wrote:
  The docs are not complete but that's up to Stephen, otherwise the patch
  should be finished. But I am not the reviewer :)
 
 Well, perhaps we had better prod Stephen then, since complete docs are
 a requirement for commit.

I didn't think the docs posted were terrible, but I am working on
improving them.

  Anyway, while this patch might not necessary get commited in this commit
  fest, I'd still like to have opinion from one of the commiters on the VIEW
  problem which also affects grant on all patch ( see
  http://archives.postgresql.org/pgsql-hackers/2009-07/msg00957.php ) and I
  fear returned with feedback might prevent that until next commit fest.
 
 OK, hopefully one of them will chime in with an opinion.  As I say, I
 would like to see this get committed if it's done, I just wasn't sure
 it was.  But now it seems that was due to insufficiently careful
 reading of the thread, with the exception of this issue and the need
 to finish the docs.

Working on it...  Not that we've never committed stuff with less than
complete docs before.. ;)

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Determining client_encoding from client locale

2009-07-22 Thread Jaime Casanova
On Wed, Jul 22, 2009 at 9:58 PM, Jaime
Casanovajcasa...@systemguards.com.ec wrote:
 On Wed, Jul 22, 2009 at 7:30 PM, Alvaro
 Herreraalvhe...@commandprompt.com wrote:
 Jaime Casanova wrote:

 this little test compiles fine until i applied your patch :(

 postg...@casanova1:~/pg_releases/pgtests$ gcc -o test-libpq
 test-libpq.o -L/usr/local/pgsql/head/lib -lpq
 /usr/local/pgsql/head/lib/libpq.so: undefined reference to
 `pg_get_encoding_from_locale'

 Do you have an older version of libpq.so around?


 the one that installed with 8.4.0 but i thougth that when you specify
 -L to gcc you're telling it where to pick libraries from, no?


more to the point when i used unpatched 8.5 tree it works just fine


-- 
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

-- 
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] join regression failure on cygwin

2009-07-22 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes:
 My Cygwin buildfarm member started failing (hanging, in fact) 
 recently. It seems to hang consistently in join.sql and the only way I 
 can get it to complete is to kill the backend fairly violently. See 
 http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=brown_batdt=2009-07-22%2023:10:21
  

 Nope, it wasn't that. So my next candidate is the prior 
 join_is_legal/GEQO changes. But reverting that would get rid of the new 
 regression test where we seem to be failing, so I'm not quite sure where 
 to go on it.

I see it claims to have working erand48, but maybe not so much in
reality?

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