Re: [HACKERS] [PATCH] "\ef " in psql

2008-09-05 Thread Tom Lane
I wrote:
> * the psql command seemed to have some ideas about supplying a blank
> CREATE OR REPLACE FUNCTION command for a nonexistent function, but this
> didn't actually work.  In any case it seemed poorly thought out, because
> you'd really need to pay some attention to *why* the regproc/regprocedure
> lookup failed.  I just ripped it out for the moment.  I'm not averse to
> the concept, if you can get it implemented properly.

While I was out at dinner, the obvious solution presented itself: define
\ef with no argument as being the command that presents an empty CREATE
FUNCTION command template to fill in.  This isn't any more or less
typing than where I think you were going, and it eliminates all the
ambiguity about whether you meant to type a nonexistent function name
or just mistyped.

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] "\ef " in psql

2008-09-05 Thread Tom Lane
"Brendan Jurd" <[EMAIL PROTECTED]> writes:
> On Sat, Sep 6, 2008 at 10:10 AM, Tom Lane <[EMAIL PROTECTED]> wrote:
>> ... I changed
>> the exit code to PSQL_CMD_NEWEDIT instead of PSQL_CMD_SEND, which causes
>> the command to wait in the query buffer.

> The principle of least astonishment suggests that \ef should behave in
> the same way as \e.

Quite.

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: Fwd: [HACKERS] [Patch Review] TRUNCATE Permission

2008-09-05 Thread Robert Haas
Updated patch attached, based on comments from Ryan Bradetich and Tom
Lane, and sync'd to latest CVS version.

...Robert

On Mon, Sep 1, 2008 at 9:33 PM, Tom Lane <[EMAIL PROTECTED]> wrote:
> "Ryan Bradetich" <[EMAIL PROTECTED]> writes:
>> On Mon, Sep 1, 2008 at 1:00 PM, Tom Lane <[EMAIL PROTECTED]> wrote:
>>> [ something about "your patch" ]
>
>> This is Robert Haas's patch for the September 2008 commit-fest.
>> I am just offering my review.
>
> Sorry about that, I got confused by the reply-to-a-reply.
>
>> Does my first suggestion still make sense for removing the TRUNCATE in
>> pg_class_aclmask() when pg_Authid.rolcatupdate is not set?
>
> Probably.  AFAICS it should be treated exactly like ACL_DELETE, so
> anyplace that acl-whacking code is doing something for ACL_DELETE and
> the patch doesn't add in ACL_TRUNCATE, I'd be suspicious ...
>
>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
>
Index: doc/src/sgml/ddl.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ddl.sgml,v
retrieving revision 1.82
diff -c -r1.82 ddl.sgml
*** doc/src/sgml/ddl.sgml	9 May 2008 23:32:03 -	1.82
--- doc/src/sgml/ddl.sgml	6 Sep 2008 03:07:12 -
***
*** 1356,1362 

 There are several different privileges: SELECT,
 INSERT, UPDATE, DELETE,
!REFERENCES, TRIGGER,
 CREATE, CONNECT, TEMPORARY,
 EXECUTE, and USAGE.
 The privileges applicable to a particular
--- 1356,1362 

 There are several different privileges: SELECT,
 INSERT, UPDATE, DELETE,
!TRUNCATE, REFERENCES, TRIGGER,
 CREATE, CONNECT, TEMPORARY,
 EXECUTE, and USAGE.
 The privileges applicable to a particular
Index: doc/src/sgml/user-manag.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/user-manag.sgml,v
retrieving revision 1.39
diff -c -r1.39 user-manag.sgml
*** doc/src/sgml/user-manag.sgml	1 Feb 2007 00:28:18 -	1.39
--- doc/src/sgml/user-manag.sgml	6 Sep 2008 03:07:13 -
***
*** 293,299 
 granted.
 There are several different kinds of privilege: SELECT,
 INSERT, UPDATE, DELETE,
!REFERENCES, TRIGGER,
 CREATE, CONNECT, TEMPORARY,
 EXECUTE, and USAGE.
 For more information on the different types of privileges supported by
--- 293,299 
 granted.
 There are several different kinds of privilege: SELECT,
 INSERT, UPDATE, DELETE,
!TRUNCATE, REFERENCES, TRIGGER,
 CREATE, CONNECT, TEMPORARY,
 EXECUTE, and USAGE.
 For more information on the different types of privileges supported by
Index: doc/src/sgml/ref/grant.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/grant.sgml,v
retrieving revision 1.70
diff -c -r1.70 grant.sgml
*** doc/src/sgml/ref/grant.sgml	3 Jul 2008 15:59:55 -	1.70
--- doc/src/sgml/ref/grant.sgml	6 Sep 2008 03:07:14 -
***
*** 20,26 
  
   
  
! GRANT { { SELECT | INSERT | UPDATE | DELETE | REFERENCES | TRIGGER }
  [,...] | ALL [ PRIVILEGES ] }
  ON [ TABLE ] tablename [, ...]
  TO { [ GROUP ] rolename | PUBLIC } [, ...] [ WITH GRANT OPTION ]
--- 20,26 
  
   
  
! GRANT { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER }
  [,...] | ALL [ PRIVILEGES ] }
  ON [ TABLE ] tablename [, ...]
  TO { [ GROUP ] rolename | PUBLIC } [, ...] [ WITH GRANT OPTION ]
***
*** 193,198 
--- 193,208 
  
  
  
+  TRUNCATE
+  
+   
+Allows  on
+the specified table.
+   
+  
+ 
+ 
+ 
   REFERENCES
   

***
*** 421,428 
  => \z mytable
  Access privileges
   Schema |  Name   | Type  |  Access privileges   
! +-+---+--
!  public | mytable | table | miriam=arwdxt/miriam
: =r/miriam
: admin=arw/miriam
  (1 row)
--- 431,438 
  => \z mytable
  Access privileges
   Schema |  Name   | Type  |  Access privileges   
! +-+---+---
!  public | mytable | table | miriam=arwdDxt/miriam
: =r/miriam
: admin=arw/miriam
  (1 row)
***
*** 436,441 
--- 446,452 
w -- UPDATE ("write")
a -- INSERT ("append")
d -- DELETE
+   D -- TRUNCATE
x -- REFERENCES
t -- TRIGGER
X -- EXECUTE
***
*** 443,449 
C -- CREATE
c -- CONNECT
T -- TEMPORARY
!  arwdxt 

Re: [HACKERS] [PATCH] "\ef " in psql

2008-09-05 Thread Brendan Jurd
On Sat, Sep 6, 2008 at 10:10 AM, Tom Lane <[EMAIL PROTECTED]> wrote:
>
> * the way you had it set up, the CREATE OR REPLACE FUNCTION command
> would be sent to the backend instantaneously upon return from the
> editor, with no opportunity for the user to decide he didn't want his
> changes applied.  This seemed unacceptably dangerous to me.  I changed
> the exit code to PSQL_CMD_NEWEDIT instead of PSQL_CMD_SEND, which causes
> the command to wait in the query buffer.

The principle of least astonishment suggests that \ef should behave in
the same way as \e.

With \e (which I use a lot), the command(s) are immediately executed
by the backend as soon as you write and exit from the editor.  I don't
find that dangerous, and anyone who uses \e will already be very much
accustomed to it.  If \ef did something different, it would just be
weird.

If you're not sure you want to execute the contents of your \e editor
session after all, you can always delete the semicolon, or everything
in the file, before quitting.

Cheers,
BJ

-- 
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] New FSM allocation policy

2008-09-05 Thread Bruce Momjian
Heikki Linnakangas wrote:
> The way my rewritten FSM works at the moment is that pages are handed 
> out of the FSM in random order, to spread the load of multiple backends 
> to different pages. That's good for spreading the load, but it occurred 
> to me while observing a test case that inserts a lot of rows to an 
> almost empty table with plenty of free space, that that sucks for the 
> case of a single backend populating a table. Currently, FSM will hand 
> out pages in sequential order, so from OS point of view we're reading 
> and writing the pages sequentially. If the pages are handed out in 
> random order, we'll do random I/O instead.
> 
> Fortunately there's an easy fix for that. If we optimize 
> RecordAndGetPageWithFreeSpace so that it will always return the next 
> page if it has enough space, we'll be doing sequential I/O again. That's 
> trivial as long as the next heap page is on the same FSM page, and 
> probably not too hard even if it's not. If we limit this optimization to 
> within the same FSM page, we'll effectively be filling fully a 32MB stripes
> 
> Thoughts?
> 
> I'm running more tests, and there's more issues that need discussion, 
> but I'll start separate threads for each. I'll also post an updated 
> patch separately.

One other thing to keep in mind is that VACUUM can reduce a table's size
if the trailing blocks are empty, so there is some gain if the earlier
parts of the table are preferred for inserts.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

-- 
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] PostgreSQL SSL problem

2008-09-05 Thread Euler Taveira de Oliveira
Andriy Bakay escreveu:
> I have problems to setup SSL for PostgreSQL server. I did all the steps
> which described in the documentation (17.8. Secure TCP/IP Connections
> with SSL), but when I try to start the PostgreSQL server the pg_ctl gave
> me: "could not start server". And nothing in the logs (I enabled all of
> them). I googled around but did not find much.
> 
This is the wrong list to post it; try -general or -admin instead. Also
it's not polite cc'ing developers as you did.

> After I disable SSL option in postgresql.conf the server is starting
> successfully.
> 
There is something wrong with your setup. You don't post what steps you
followed. Are you sure there is nothing at the logs?


-- 
  Euler Taveira de Oliveira
  http://www.timbira.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] reducing statistics write overhead

2008-09-05 Thread Tom Lane
Euler Taveira de Oliveira <[EMAIL PROTECTED]> writes:
> If you can't afford a 500 msec pgstat time, then you need to make it
> tunable. Another ideas are (i) turn on/off pgstat per table or database
> and (ii) make the pgstat time tunable per table or database. You can use
> the reloptions column to store these info. These workarounds are much
> simpler than that you proposed and they're almost for free.

For normal usage on-demand dumping would be a really good thing; it'd
cut the overhead of having stats on tremendously, especially for people
who don't really use 'em.  The particular signaling proposed here is
bogus, but if Martin can make it work in a cleaner fashion I think it's
likely a good idea.

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] "\ef " in psql

2008-09-05 Thread Tom Lane
Abhijit Menon-Sen <[EMAIL PROTECTED]> writes:
> I have attached two patches:
> - funcdef.diff implements pg_get_functiondef()
> - edit.diff implements "\ef function" in psql based on (1).

I've applied this with some corrections (mostly around proper quoting)
and some outright editorialization:

* the psql command seemed to have some ideas about supplying a blank
CREATE OR REPLACE FUNCTION command for a nonexistent function, but this
didn't actually work.  In any case it seemed poorly thought out, because
you'd really need to pay some attention to *why* the regproc/regprocedure
lookup failed.  I just ripped it out for the moment.  I'm not averse to
the concept, if you can get it implemented properly.

* the way you had it set up, the CREATE OR REPLACE FUNCTION command
would be sent to the backend instantaneously upon return from the
editor, with no opportunity for the user to decide he didn't want his
changes applied.  This seemed unacceptably dangerous to me.  I changed
the exit code to PSQL_CMD_NEWEDIT instead of PSQL_CMD_SEND, which causes
the command to wait in the query buffer.  Unfortunately there's no
visual indication of that, other than a small change in the prompt
status.  It'd likely be better if we could get libreadline to redisplay
the query buffer contents --- anyone have an idea how to do that?
(I have some vague recollection that \e used to work that way, though
it definitely fails to do so now.)

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] reducing statistics write overhead

2008-09-05 Thread Euler Taveira de Oliveira
Martin Pihlak escreveu:
> I suspected that, but somehow managed to overlook it :( I guess it was
> too tempting to use it. I'll start looking for alternatives.
> 
If you can't afford a 500 msec pgstat time, then you need to make it
tunable. Another ideas are (i) turn on/off pgstat per table or database
and (ii) make the pgstat time tunable per table or database. You can use
the reloptions column to store these info. These workarounds are much
simpler than that you proposed and they're almost for free.


-- 
  Euler Taveira de Oliveira
  http://www.timbira.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] Need more reviewers!

2008-09-05 Thread Tom Lane
Alvaro Herrera <[EMAIL PROTECTED]> writes:
> Martijn van Oosterhout wrote:
>> I suppose what happens is the original patch comes with design and
>> later a newer version is posted with just changes. The commitfest page
>> points to the latter, losing former in the archive somewhere.

> Hmm, IMO this is a flaw in the commitfest entry for that patch -- it
> should point to both.

Yeah.  What I think we should do here is that the main entry for a patch
should point at the primary reference (first submission, or wherever
it's best discussed), and then any later updates of the patch should be
added as comments, instead of replacing the primary reference.  It's
been done this way for quite a few patches, but evidently not every one.

Also, readers should remember to look through the whole thread in the
archives, not just the single article linked to.  (Dunno if that would
have helped Martijn in this case, but there's often good material in the
rest of the thread.)

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] Proposal: new border setting in psql

2008-09-05 Thread Bruce Momjian
Gregory Stark wrote:
> I wonder if it's worth keeping two variants at all really. Why not just make
> psql's native table formatting exactly ReST? Is there any part of it that we
> don't like as much as our existing tables?

It doubles the number of display lines;  a very obvious shortcoming.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

-- 
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] Need more reviewers!

2008-09-05 Thread Alvaro Herrera
Martijn van Oosterhout wrote:

> Just one thing though, I picked a random patch and started reading.
> However, the commitfest page doesn't link to anywhere that actually
> describes *what* the patch is trying to do. Many patches do have the
> design and the patch in one page, but some don't.
> 
> I suppose what happens is the original patch comes with design and
> later a newer version is posted with just changes. The commitfest page
> points to the latter, losing former in the archive somewhere.

Hmm, IMO this is a flaw in the commitfest entry for that patch -- it
should point to both.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

-- 
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] Need more reviewers!

2008-09-05 Thread Martijn van Oosterhout
On Thu, Sep 04, 2008 at 09:54:02PM +0100, Simon Riggs wrote:
> * coding review - does it follow standard code guidelines? Are there
> portability issues? Will it work on Windows/BSD etc? Are there
> sufficient comments?
> 
> * code review - does it do what it says, correctly?

Just one thing though, I picked a random patch and started reading.
However, the commitfest page doesn't link to anywhere that actually
describes *what* the patch is trying to do. Many patches do have the
design and the patch in one page, but some don't.

I suppose what happens is the original patch comes with design and
later a newer version is posted with just changes. The commitfest page
points to the latter, losing former in the archive somewhere.

Have a nice day,
-- 
Martijn van Oosterhout   <[EMAIL PROTECTED]>   http://svana.org/kleptog/
> Please line up in a tree and maintain the heap invariant while 
> boarding. Thank you for flying nlogn airlines.


signature.asc
Description: Digital signature


Re: [HACKERS] [PATCHES] libpq events patch (with sgml docs)

2008-09-05 Thread Alvaro Herrera
Andrew Chernow escribió:
> Alvaro Herrera wrote:
>> Andrew Chernow escribió:

>>>* PQclear -
>>>*  free's the memory associated with a PGresult
>>>*/
>>
>> I'd add a comment here stating why the event name is not deallocated;
>> otherwise it just looks like it's being leaked.
>
> The event name is being deallocated.

Doh!  Sorry, you're right.  In that case, you're missing a NULL result
check from strdup() in dupEvents() ;-)

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

-- 
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] reducing statistics write overhead

2008-09-05 Thread Martin Pihlak
Tom Lane wrote:
> Martin Pihlak <[EMAIL PROTECTED]> writes:
>> So, as a simple optimization I am proposing that the file should be
>> only written when some backend requests statistics. This would
>> significantly reduce the undesired write traffic at the cost of
>> slightly slower stats access.
> 
> How necessary is this given the recent fixes to allow the stats file to
> be kept on a ramdisk?
> 

Ramdisk helps, but requires additional effort to set up. Also the stats
file has a tendency to creep up on you -- as the database evolves the file
size gradually increases and suddenly the DBA is left wondering what
happened to performance.

>> Attached is a WIP patch, which basically implements this:
> 
> This patch breaks deadlock checking and statement_timeout, because
> backends already use SIGALRM.  You can't just take over that signal.
> It's possible that you could get things to work by treating this as an
> additional reason for SIGALRM, but that code is unreasonably complex
> already.  I'd suggest finding some other way.
> 

I suspected that, but somehow managed to overlook it :( I guess it was
too tempting to use it. I'll start looking for alternatives.

regards,
Martin


-- 
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] [PATCHES] libpq events patch (with sgml docs)

2008-09-05 Thread Alvaro Herrera
Andrew Chernow escribió:

>   /*
>* PQmakeEmptyPGresult
>*   returns a newly allocated, initialized PGresult with given status.
>*   If conn is not NULL and status indicates an error, the conn's
>*   errorMessage is copied.
>*
>* Note this is exported --- you wouldn't think an application would need
>* to build its own PGresults, but this has proven useful in both libpgtcl
>* and the Perl5 interface, so maybe it's not so unreasonable.
> +  *
> +  * Updated April 2008 - If conn is not NULL, event states will be copied
> +  * from the PGconn to the created PGresult.
>*/

Don't do this either.  We don't really need to know when the thing was
changed; the comment should just state what the function does.  I had
folded the last paragraph into the introductory one, but I think you
lost that part of my change.

> + /* resultalloc the attribute names.  The above memcpy has the attr
> +  * names pointing at the callers provided attDescs memory.
> +  */

"resultalloc"?  Why not just "allocate"?

>* PQclear -
>*free's the memory associated with a PGresult
>*/

I'd add a comment here stating why the event name is not deallocated;
otherwise it just looks like it's being leaked.


-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

-- 
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] reducing statistics write overhead

2008-09-05 Thread Joshua Drake
On Fri, 05 Sep 2008 15:23:18 -0400
Tom Lane <[EMAIL PROTECTED]> wrote:

> Martin Pihlak <[EMAIL PROTECTED]> writes:
> > So, as a simple optimization I am proposing that the file should be
> > only written when some backend requests statistics. This would
> > significantly reduce the undesired write traffic at the cost of
> > slightly slower stats access.
> 
> How necessary is this given the recent fixes to allow the stats file
> to be kept on a ramdisk?

From an usability and integration perspective this patch is a nice
touch. On demand is a nice feature when used correctly.

Sincerely,

Joshua D. Drake

-- 
The PostgreSQL Company since 1997: http://www.commandprompt.com/ 
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: 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] 8.4devel out of memory

2008-09-05 Thread Kevin Grittner
>>> Tom Lane <[EMAIL PROTECTED]> wrote: 
 
> Also, does the leak still occur if
> you just run the query as-is rather than EXPLAIN ANALYZE it?
 
The machine became unresponsive similar to the early symptoms of the
apparent memory leak cited in this post:
 
http://archives.postgresql.org/pgsql-bugs/2008-07/msg00105.php
 
It was impossible to kill the offending process, since the connections
were unresponsive; we didn't wait for the OOM killer, but rebooted the
machine.
 
-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] reducing statistics write overhead

2008-09-05 Thread Tom Lane
Martin Pihlak <[EMAIL PROTECTED]> writes:
> So, as a simple optimization I am proposing that the file should be
> only written when some backend requests statistics. This would
> significantly reduce the undesired write traffic at the cost of
> slightly slower stats access.

How necessary is this given the recent fixes to allow the stats file to
be kept on a ramdisk?

> Attached is a WIP patch, which basically implements this:

This patch breaks deadlock checking and statement_timeout, because
backends already use SIGALRM.  You can't just take over that signal.
It's possible that you could get things to work by treating this as an
additional reason for SIGALRM, but that code is unreasonably complex
already.  I'd suggest finding some other way.

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] libpq events update

2008-09-05 Thread Merlin Moncure
On Fri, Sep 5, 2008 at 9:54 AM, Andrew Chernow <[EMAIL PROTECTED]> wrote:
> Andrew Chernow wrote:
>>
>> I think it got confused with the instanceData feature, which has nothing
>> to do with the event system and requires public functions.  libpqtypes
>> happens to use the instanceData functions within its eventproc, but this is
>> not a requirement.
>>
>
> I forgot to mention that the instanceData functions should be moved from
> libpq-events.h to libpq-fe.h because they are not part of the event system.
>  I plan on making this change as well, so let me know if you hate it.


An updated patch with docs and the above change is on -patches.
Should we have sent that here?

merlin

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


[HACKERS] reducing statistics write overhead

2008-09-05 Thread Martin Pihlak
Howdy,

The statistics collector currently dumps the stats file at every 500ms.  This
is a major problem if the file becomes large -- occasionally we've been forced
to disable stats collection to cope with it.  Another issue is that while the
file is frequently written, it is seldom read. Typically once a minute -
autovacuum plus possible user initiated stats queries.

So, as a simple optimization I am proposing that the file should be only
written when some backend requests statistics. This would significantly reduce
the undesired write traffic at the cost of slightly slower stats access.

Attached is a WIP patch, which basically implements this:

Disable periodic writing of the stats file. Introduce new stats message type -
PGSTAT_MTYPE_INQUIRY. Backends send this to notify collector that stats is 
needed.
Pid of the requestor is provided in the message. Backend then installs an alarm
handler and starts a timer.  Collector processes the messages and compiles a 
list
of pids to be notified. If there are any, the stats file is written and SIGALRM
is sent to the requestors. Backend then proceeds to read the stats file a usual.

Thoughts, comments?

regards,
Martin
Index: src/backend/postmaster/pgstat.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/postmaster/pgstat.c,v
retrieving revision 1.181
diff -c -r1.181 pgstat.c
*** src/backend/postmaster/pgstat.c	25 Aug 2008 18:55:43 -	1.181
--- src/backend/postmaster/pgstat.c	5 Sep 2008 18:36:24 -
***
*** 85,90 
--- 85,92 
  #define PGSTAT_SELECT_TIMEOUT	2		/* How often to check for postmaster
  		 * death; in seconds. */
  
+ #define PGSTAT_WRITE_TIMEOUT	5		/* How long to wait for the collector 
+ 		 * to finish writing the file; in seconds. */
  
  /* --
   * The initial size hints for the hash tables used in the collector.
***
*** 94,100 
  #define PGSTAT_TAB_HASH_SIZE	512
  #define PGSTAT_FUNCTION_HASH_SIZE	512
  
- 
  /* --
   * GUC parameters
   * --
--- 96,101 
***
*** 201,208 
--- 202,213 
   */
  static PgStat_GlobalStats globalStats;
  
+ /* Last time we wrote the stats file */
+ static TimestampTz last_statwrite;
+ 
  static volatile bool need_exit = false;
  static volatile bool need_statwrite = false;
+ static volatile bool stats_ready = false;
  static volatile bool got_SIGHUP = false;
  
  /*
***
*** 213,218 
--- 218,229 
  static instr_time total_func_time;
  
  
+ #define PGSTAT_MAX_INQUIRIES	16		/* Max number of outstanding stats inquiries */
+ 
+ /* List of backends that need notifications */
+ static pid_t inquiring_backends[PGSTAT_MAX_INQUIRIES];
+ static int num_inquiries = 0;
+ 
  /* --
   * Local function forward declarations
   * --
***
*** 223,229 
  
  NON_EXEC_STATIC void PgstatCollectorMain(int argc, char *argv[]);
  static void pgstat_exit(SIGNAL_ARGS);
! static void force_statwrite(SIGNAL_ARGS);
  static void pgstat_beshutdown_hook(int code, Datum arg);
  static void pgstat_sighup_handler(SIGNAL_ARGS);
  
--- 234,240 
  
  NON_EXEC_STATIC void PgstatCollectorMain(int argc, char *argv[]);
  static void pgstat_exit(SIGNAL_ARGS);
! static void pgstat_alarm_handler(SIGNAL_ARGS);
  static void pgstat_beshutdown_hook(int code, Datum arg);
  static void pgstat_sighup_handler(SIGNAL_ARGS);
  
***
*** 254,259 
--- 265,271 
  static void pgstat_recv_bgwriter(PgStat_MsgBgWriter *msg, int len);
  static void pgstat_recv_funcstat(PgStat_MsgFuncstat *msg, int len);
  static void pgstat_recv_funcpurge(PgStat_MsgFuncpurge *msg, int len);
+ static void pgstat_recv_inquiry(PgStat_MsgInquiry *msg, int len);
  
  
  /* 
***
*** 2463,2468 
--- 2475,2496 
  	hdr->m_type = mtype;
  }
  
+ /* --
+  * pgstat_send_inquiry() -
+  *
+  *		Notify collector that we need fresh data.
+  * --
+  */
+ static void
+ pgstat_send_inquiry(void)
+ {
+ 	PgStat_MsgInquiry msg;
+ 
+ 	elog(DEBUG1, "pgstat_send_inquiry: I am %u", getpid());
+ 	pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_INQUIRY);
+ 	msg.m_pid = getpid();
+ 	pgstat_send(&msg, sizeof(msg));
+ }
  
  /* --
   * pgstat_send() -
***
*** 2538,2545 
  NON_EXEC_STATIC void
  PgstatCollectorMain(int argc, char *argv[])
  {
- 	struct itimerval write_timeout;
- 	bool		need_timer = false;
  	int			len;
  	PgStat_Msg	msg;
  
--- 2566,2571 
***
*** 2577,2583 
  	pqsignal(SIGINT, SIG_IGN);
  	pqsignal(SIGTERM, SIG_IGN);
  	pqsignal(SIGQUIT, pgstat_exit);
- 	pqsignal(SIGALRM, force_statwrite);
  	pqsignal(SIGPIPE, SIG_IGN);
  	pqsignal(SIGUSR1, SIG_IGN);
  	pqsignal(SIGUSR2, SIG_IGN);
--- 2603,2608 
***
*** 2598,2608 
  	 */
  	need_statwrite = true;
  
- 	/* Preset the delay between status file writes */
- 	MemSet(&write_ti

Re: [HACKERS] CVS head has broken make

2008-09-05 Thread Merlin Moncure
On Fri, Sep 5, 2008 at 2:52 PM, Alvaro Herrera
<[EMAIL PROTECTED]> wrote:
> Tom Lane wrote:
>> Stefan Kaltenbrunner <[EMAIL PROTECTED]> writes:
>> > yeah the "code coverage" changes broke it - the buildfarm dashboard is
>> > pretty telling:
>>
>> Yes --- it looks like the configure.in patch is designed to look for
>> gcov AND lcov (do we really need both?) AND genhtml, and error out
>> if they're not present, even if you didn't say --enable-coverage.
>> Please fix.
>
> gcov and lcov do different things; they are both needed.  lcov is a GNU
> extension of gcov, which generates the HTML pages.

just confirmed that this is fixed.

merlin

-- 
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] Planner question

2008-09-05 Thread Tom Lane
Tom Raney <[EMAIL PROTECTED]> writes:
> Why does the planner consider both input variations of each symmetric merge 
> join?  The README says "there is not a lot of difference" between the two 
> options.  When are there any differences?

The righthand side needs to support mark/restore, the left doesn't;
so depending on plan types one way might need a helper Materialize
node that the other way doesn't.  Also, duplicated values are a bit
cheaper to process on the left than the right.

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] CVS head has broken make

2008-09-05 Thread Alvaro Herrera
Tom Lane wrote:
> Stefan Kaltenbrunner <[EMAIL PROTECTED]> writes:
> > yeah the "code coverage" changes broke it - the buildfarm dashboard is 
> > pretty telling:
> 
> Yes --- it looks like the configure.in patch is designed to look for
> gcov AND lcov (do we really need both?) AND genhtml, and error out
> if they're not present, even if you didn't say --enable-coverage.
> Please fix.

gcov and lcov do different things; they are both needed.  lcov is a GNU
extension of gcov, which generates the HTML pages.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

-- 
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] Verbosity of Function Return Type Checks

2008-09-05 Thread Alvaro Herrera
Volkan YAZICI wrote:

> BTW, Alvaro fixed my string concatenations which yielded in lines
> exceeding 80 characters width, but I'd want to ask twice if you're sure
> with this. Because, IMHO, PostgreSQL is also famous with the quality and
> readability of its source code -- that I'm quite proud of as a user,
> kudos to developers -- and I think it'd be better to stick with 80
> characters width convention as much as one can.

Yeah, I'm quite anal about that.  What will happen is that pgindent will
"push back" the strings so that they start earlier in the line to keep
the 80 char limit.  IMHO that's actually clearer than truncating the
string.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

-- 
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] Planner question

2008-09-05 Thread Jeff Davis
On Fri, 2008-09-05 at 11:21 -0700, Tom Raney wrote:
> Why does the planner consider both input variations of each symmetric merge 
> join?  The README says "there is not a lot of difference" between the two 
> options.  When are there any differences?
> 
> -Tom Raney
> 

http://archives.postgresql.org/pgsql-general/2008-08/msg00967.php

My understanding from that thread is that if one table has high
ndistinct and the other has low ndistinct, one plan may require more
re-scanning than the other.

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] [Review] Tests citext casts by David Wheeler.

2008-09-05 Thread David E. Wheeler

On Sep 5, 2008, at 11:30, Tom Lane wrote:


Thanks for reviewing.  I've committed this with your suggestions and
one additional non-cosmetic change: schema-qualify names in the
bodies of the SQL functions so that they are not search_path  
dependent.


Thanks, I'll check that out.


One thing that didn't make a lot of sense to me was the last new
function:

CREATE OR REPLACE FUNCTION translate( citext, citext, text ) RETURNS  
TEXT AS $$
   SELECT  
pg_catalog.translate( pg_catalog.translate( $1::pg_catalog.text,  
pg_catalog.lower($2::pg_catalog.text), $3),  
pg_catalog.upper($2::pg_catalog.text), $3);

$$ LANGUAGE SQL IMMUTABLE STRICT;

Why is it using upper()?


To make translate() work case-insensitively, it does two translates:  
One lowercase and one uppercase. This allows the translated value to  
be returned with its original casing in tact. No, this isn't ideal,  
but it was simple to do.


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] Prototype: In-place upgrade v02

2008-09-05 Thread Heikki Linnakangas

Zdenek Kotala wrote:

Heikki Linnakangas napsal(a):

The patch seems to be missing the new htup.c file.


I'm sorry. I attached new version which is synchronized with current
head. I would like to say more comments as well.

1) The patch contains also changes which was discussed during July 
commit fest. - PageGetTempPage modification suggested by Tom

- another hash.h backward compatible cleanup


It might be a good idea to split that into a separate patch. The sheer 
size of this patch is quite daunting, even though the bulk of it is 
straightforward search&replace.


2) I add tuplimits.h header file which contains tuple limits for 
different access method. It is not finished yet, but idea is to keep all 
limits in one file and easily add limits for different page layout 
version - for example replace static computing with dynamic based on 
relation (maxtuplesize could be store in pg_class for each relation).


I need this header also because I fallen in a cycle in header dependency.

3) I already sent Page API performance result in 
http://archives.postgresql.org/pgsql-hackers/2008-08/msg00398.php


I replaced call sequence PagetGetItemId, PageGetItemId with 
PageGetIndexTuple and PageGetHeapTuple function. It is main difference 
in this patch. PAgeGetHeap Tuple fills t_ver in HeapTuple to identify 
correct tupleheader version.


It would be good to mention that PageAPI (and tuple API) implementation 
is only prototype without any performance optimization.


You mentioned 5% performance degradation in that thread. What test case 
was that? What would be a worst-case scanario, and how bad is it?


5% is a pretty hefty price, especially when it's paid by not only 
upgraded installations, but also freshly initialized clusters. I think 
you'll need to pursue those performance optimizations.


4) This patch contains more topics for decision. First is general if 
this approach is acceptable.


I don't like the invasiveness of this approach. It's pretty invasive 
already, and ISTM you'll need similar switch-case handling of all data 
types that have changed the internal representation as well.


We've talked about this before, so you'll remember that I favor teh 
approach is to convert the page format, page at a time, when the pages 
are read in. I grant you that there's non-trivial issues with that as 
well, like if the converted data takes more space and don't fit in the 
page anymore.


I wonder if we could go with some sort of a hybrid approach? Convert the 
 whole page when it's read in, but if it doesn't fit, fall back to 
tricks like loosening the alignment requirements on platforms that can 
handle non-aligned data, or support a special truncated page header, 
without pd_tli and pd_prune_xid fields. Just a thought, not sure how 
feasible those particular tricks are, but something along those lines..


All in all, though. I find it a bit hard to see the big picture. For 
upgrade-in-place, what are all the pieces that we need? To keep this 
concrete, let's focus on PG 8.2 -> PG 8.3 (or are you focusing on PG 8.3 
-> 8.4? That's fine with me as well, but let's pick one) and forget 
about hypothetical changes that might occur in a future version. I can see:

1. Handling page layout changes (pd_prune_xid, pd_flags)
2. Handling tuple header changes (infomask2, HOT bits, combocid)
3. Handling changes in data type representation (packed varlens)
4. Toast chunk size
5. Catalogs

After putting all those together, how large a patch are we talking 
about, and what's the performance penalty then? How much of all that 
needs to be in core, and how much can live in a pgfoundry project or an 
extra binary in src/bin or contrib? I realize that none of us have a 
crystal ball, and one has to start somewhere, but I feel uneasy 
committing to an approach until we have a full plan.


--
  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] [Review] Tests citext casts by David Wheeler.

2008-09-05 Thread Tom Lane
"Ryan Bradetich" <[EMAIL PROTECTED]> writes:
> Here is my review of the Test citext casts written by David Wheeler:

Thanks for reviewing.  I've committed this with your suggestions and
one additional non-cosmetic change: schema-qualify names in the
bodies of the SQL functions so that they are not search_path dependent.

One thing that didn't make a lot of sense to me was the last new
function:

CREATE OR REPLACE FUNCTION translate( citext, citext, text ) RETURNS TEXT AS $$
SELECT pg_catalog.translate( pg_catalog.translate( $1::pg_catalog.text, 
pg_catalog.lower($2::pg_catalog.text), $3), 
pg_catalog.upper($2::pg_catalog.text), $3);
$$ LANGUAGE SQL IMMUTABLE STRICT;

Why is it using upper()?

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] Verbosity of Function Return Type Checks

2008-09-05 Thread Volkan YAZICI
On Fri, 05 Sep 2008, Tom Lane <[EMAIL PROTECTED]> writes:
> I think the best way is to use
>
>   subroutine(..., gettext_noop("special error message here"))
>
> at the call sites, and then
>
>   errmsg("%s", _(msg))
>
> when throwing the error.  gettext_noop() is needed to have the string
> be put into the message catalog, but it doesn't represent any run-time
> effort.  The _() macro is what actually makes the translation lookup
> happen.  We don't want to incur that cost in the normal no-error case,
> which is why you shouldn't just do _() at the call site and pass an
> already-translated string to the subroutine.

Modified as you suggested. BTW, will there be a similar i18n scenario
for "dropped column" you mentioned below?

>>   if (td1->attrs[i]->atttypid &&
>>   td2->attrs[i]->atttypid &&
>>   td1->attrs[i]->atttypid != td2->attrs[i]->atttypid)
>
>> expression to fix this?
>
> No, that's weakening the compatibility check.  (There's a separate issue
> here of teaching plpgsql to actually cope nicely with rowtypes
> containing dropped columns, but that's well beyond the scope of this
> patch.)
>
> What I'm on about is protecting format_type_be() from being passed
> a zero and then failing.  Perhaps it would be good enough to do
> something like
>
>   OidIsValid(td1->attrs[i]->atttypid) ?
>  format_type_with_typemod(td1->attrs[i]->atttypid,
>   td1->attrs[i]->atttypmod) :
>  "dropped column"
>
> while throwing the error.
>
> BTW, since what's actually being looked at is just the type OID and not
> the typmod, it seems inappropriate to me to show the typmod in the
> error.  I'd go with just format_type_be(td1->attrs[i]->atttypid) here
> I think.

Done with format_type_be() usage.

BTW, Alvaro fixed my string concatenations which yielded in lines
exceeding 80 characters width, but I'd want to ask twice if you're sure
with this. Because, IMHO, PostgreSQL is also famous with the quality and
readability of its source code -- that I'm quite proud of as a user,
kudos to developers -- and I think it'd be better to stick with 80
characters width convention as much as one can.


Regards.

Index: src/pl/plpgsql/src/pl_exec.c
===
RCS file: /projects/cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v
retrieving revision 1.219
diff -c -r1.219 pl_exec.c
*** src/pl/plpgsql/src/pl_exec.c	1 Sep 2008 22:30:33 -	1.219
--- src/pl/plpgsql/src/pl_exec.c	5 Sep 2008 18:19:50 -
***
*** 188,194 
  	   Oid reqtype, int32 reqtypmod,
  	   bool isnull);
  static void exec_init_tuple_store(PLpgSQL_execstate *estate);
! static bool compatible_tupdesc(TupleDesc td1, TupleDesc td2);
  static void exec_set_found(PLpgSQL_execstate *estate, bool state);
  static void plpgsql_create_econtext(PLpgSQL_execstate *estate);
  static void free_var(PLpgSQL_var *var);
--- 188,195 
  	   Oid reqtype, int32 reqtypmod,
  	   bool isnull);
  static void exec_init_tuple_store(PLpgSQL_execstate *estate);
! static void validate_tupdesc_compat(TupleDesc expected, TupleDesc returned,
! 	const char *msg);
  static void exec_set_found(PLpgSQL_execstate *estate, bool state);
  static void plpgsql_create_econtext(PLpgSQL_execstate *estate);
  static void free_var(PLpgSQL_var *var);
***
*** 384,394 
  			{
  case TYPEFUNC_COMPOSITE:
  	/* got the expected result rowtype, now check it */
! 	if (estate.rettupdesc == NULL ||
! 		!compatible_tupdesc(estate.rettupdesc, tupdesc))
! 		ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
!  errmsg("returned record type does not match expected record type")));
  	break;
  case TYPEFUNC_RECORD:
  
--- 385,392 
  			{
  case TYPEFUNC_COMPOSITE:
  	/* got the expected result rowtype, now check it */
! 	validate_tupdesc_compat(tupdesc, estate.rettupdesc,
! 			gettext_noop("returned record type does not match expected record type"));
  	break;
  case TYPEFUNC_RECORD:
  
***
*** 705,715 
  		rettup = NULL;
  	else
  	{
! 		if (!compatible_tupdesc(estate.rettupdesc,
! trigdata->tg_relation->rd_att))
! 			ereport(ERROR,
! 	(errcode(ERRCODE_DATATYPE_MISMATCH),
! 	 errmsg("returned tuple structure does not match table of trigger event")));
  		/* Copy tuple to upper executor memory */
  		rettup = SPI_copytuple((HeapTuple) DatumGetPointer(estate.retval));
  	}
--- 703,711 
  		rettup = NULL;
  	else
  	{
! 		validate_tupdesc_compat(trigdata->tg_relation->rd_att,
! estate.rettupdesc,
! gettext_noop("returned tuple structure does not match table of trigger event"));
  		/* Copy tuple to upper executor memory */
  		rettup = SPI_copytuple((HeapTuple) DatumGetPointer(estate.retval));
  	}
***
*** 2199,2209 
  		  (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
  		  

[HACKERS] Planner question

2008-09-05 Thread Tom Raney

Why does the planner consider both input variations of each symmetric merge join?  The 
README says "there is not a lot of difference" between the two options.  When 
are there any differences?

-Tom Raney

--
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] Withdraw PL/Proxy from commitfest

2008-09-05 Thread Asko Oja
On Fri, Sep 5, 2008 at 7:37 PM, Heikki Linnakangas <
[EMAIL PROTECTED]> wrote:

> So, you'll implement the part of SQL-MED that deals with specifying remote
> connections, e.g something like "CREATE CONNECTION" (no, I haven't looked at
> what the syntax actually is)?
>
> Yeah, that sounds like a good idea. We should get that into core, and
> modify contrib/dblink to use it as well. It's just a small part of SQL-MED,
> but it's a start, and it's useful for these other projects.
>

Yes that's the plan.

>
>
> Marko Kreen wrote:
>
>> In the previous discussion there was mentioned that Postgres should
>> move to the SQL-MED direction in remote connection handling.
>>
>> SQL-MED specifies that connections should have names and referenced
>> everywhere using names.  PL/Proxy currently does not conform to that
>> standard - it uses connection strings directly.  Although it could
>> made work with SQL-MED backend, it would look ugly.
>>
>> So I'd like to withdraw PL/Proxy from commitfest and rework it's
>> connection handling scheme to be also name->connstr based.  Idea will
>> be that it will have user-definable connection handling backend,
>> which operates on named connections.  And in the future we can
>> plug in a backend that reuses connection info from builtin SQL-MED store.
>>
>> Although the current connection handling works and is secure it has
>> a deficiency that it's bit hard to hide the password that is used
>> for connecting.  User can either play with table/function permissions
>> and SECURITY DEFINER functions but that's complex.  Or he can put
>> passwords into .pgpass - this is easy and secure but has the problem
>> that the file is not manageable from inside database.
>>
>> So PL/Proxy needs new SQL-MED based scheme that fixes it.  When this
>> is ready we can re-discuss the builtin vs. PL-based remote functions.
>> As I don't plan to work on it near-term there is no point polluting
>> the commitfest page with it.
>>
>> [ There was a attempt to paint the .pgpass based password handling
>>  insecure because dblink makes the file world-readable.  I still
>>  fail to see how this any way points to flaws of the scheme... ]
>>
>>
>
> --
>  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] 8.4devel out of memory

2008-09-05 Thread Tom Lane
"Kevin Grittner" <[EMAIL PROTECTED]> writes:
> The tables and views aren't that hard; finding a way to generate
> enough fake data may be a challenge.  (I assume that since it took
> over a half hour to run out of memory, the volume of data needs to be
> sufficient to get there.)

We don't really need 2GB of leakage to find the problem ... a query
that generates a couple hundred meg of bloat would be plenty.

Since we don't know how much space the query would have needed to run to
completion, it's likely that something involving much less than a tenth
as much data would still be enough to make the leak obvious.

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] CVS head has broken make

2008-09-05 Thread Tom Lane
Stefan Kaltenbrunner <[EMAIL PROTECTED]> writes:
> yeah the "code coverage" changes broke it - the buildfarm dashboard is 
> pretty telling:

Yes --- it looks like the configure.in patch is designed to look for
gcov AND lcov (do we really need both?) AND genhtml, and error out
if they're not present, even if you didn't say --enable-coverage.
Please fix.

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] 8.4devel out of memory

2008-09-05 Thread Kevin Grittner
>>> Tom Lane <[EMAIL PROTECTED]> wrote: 
> "Kevin Grittner" <[EMAIL PROTECTED]> writes:
>> PortalHeapMemory: 1620992 total in 200 blocks; 5856 free (8
chunks); 
> 1615136 used
>>   ExecutorState: 2787288448 total in 364 blocks; 328 free (5
chunks); 
> 2787288120 used
> 
> Ouch.  We have created a memory leak someplace, but it's hard to
tell
> where from this info.  Can you boil this down into a self-contained
test
> case?  I doubt it depends at all on the specific data, so the table
&
> view definitions plus some dummy data would probably be enough to
> reproduce it.
 
The tables and views aren't that hard; finding a way to generate
enough fake data may be a challenge.  (I assume that since it took
over a half hour to run out of memory, the volume of data needs to be
sufficient to get there.)
 
> Is this a 32-bit or 64-bit build?
 
32-bit PostgreSQL on 32-bit Linux.
 
> Also, does the leak still occur if
> you just run the query as-is rather than EXPLAIN ANALYZE it?
 
I will find out.
 
-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] CVS head has broken make

2008-09-05 Thread Stefan Kaltenbrunner

Andrew Chernow wrote:

Getting this, my cvs copy is as of 1:00PM EST.

[EMAIL PROTECTED] pgsql]# ./configure --prefix=/usr
checking build system type... i686-pc-linux-gnu
checking host system type... i686-pc-linux-gnu
checking which template to use... linux
checking whether to build with 64-bit integer date/time support... yes
checking whether NLS is wanted... no
checking for default port number... 5432
checking for gcov... gcov
checking for lcov... no
configure: error: lcov not found

I get the same error trying to make libpq.  Another box with a copy from 
yesterday afternoon builds fine.


Things updated that may be related.


yeah the "code coverage" changes broke it - the buildfarm dashboard is 
pretty telling:


http://buildfarm.postgresql.org/cgi-bin/show_status.pl


Stefan

--
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] plpgsql is not translate-aware

2008-09-05 Thread Tom Lane
Alvaro Herrera <[EMAIL PROTECTED]> writes:
> The vices in the error message are not the translator's fault: missing
> quotes and "plpgsql" instead of "PL/pgSQL":

It's been message style policy for quite some time to not quote the
output of format_type.  I think this is because format_type sometimes
puts quotes into its output, and it'd look weird.

> I'd even go a bit further and say that the original should not include
> the language name in the string, so that (say) plpython and plperl can
> use the same translation:

That'd only be useful if they all share a common message catalog, which
does not seem like a good design direction to me.  How would a non-core
PL hope to get localized if it can't have its own catalog?

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


[HACKERS] CVS head has broken make

2008-09-05 Thread Andrew Chernow

Getting this, my cvs copy is as of 1:00PM EST.

[EMAIL PROTECTED] pgsql]# ./configure --prefix=/usr
checking build system type... i686-pc-linux-gnu
checking host system type... i686-pc-linux-gnu
checking which template to use... linux
checking whether to build with 64-bit integer date/time support... yes
checking whether NLS is wanted... no
checking for default port number... 5432
checking for gcov... gcov
checking for lcov... no
configure: error: lcov not found

I get the same error trying to make libpq.  Another box with a copy from 
yesterday afternoon builds fine.


Things updated that may be related.

P GNUmakefile.in
P configure
P configure.in
P src/Makefile.global.in
P src/backend/common.mk

--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.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] 8.4devel out of memory

2008-09-05 Thread Tom Lane
"Kevin Grittner" <[EMAIL PROTECTED]> writes:
> PortalHeapMemory: 1620992 total in 200 blocks; 5856 free (8 chunks); 
> 1615136 used
>   ExecutorState: 2787288448 total in 364 blocks; 328 free (5 chunks); 
> 2787288120 used

Ouch.  We have created a memory leak someplace, but it's hard to tell
where from this info.  Can you boil this down into a self-contained test
case?  I doubt it depends at all on the specific data, so the table &
view definitions plus some dummy data would probably be enough to
reproduce it.

Is this a 32-bit or 64-bit build?  Also, does the leak still occur if
you just run the query as-is rather than EXPLAIN ANALYZE it?

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] plpgsql is not translate-aware

2008-09-05 Thread Alvaro Herrera
Tom Lane wrote:
> Alvaro Herrera <[EMAIL PROTECTED]> writes:
> > Actually this is wrong -- since the library is going to run with
> > "postgres" text domain, we need to add the files to the backend's
> > nls.mk:
> 
> Can't we give it its own text domain?  It seems fundamentally wrong
> for a plug-in language to require core support for its messages.
> (Now that I think about it, that may have been the reason we don't have
> localization for it already.)  I suppose this must be possible,
> since e.g. glibc manages to have its own messages separate from
> whatever app it's linked with.

I'm not sure how this'd work.  I think this would require plpgsql using
dgettext (passing a domain) instead of plain gettext(), but since it
uses ereport() just like the backend, I don't see a good way to make
that work.

Maybe another idea would be to call textdomain() just before calling
anything that would raise an error, and reset it on exit.  But since
backend errors can happen at any time too, this doesn't seem possible.

Not sure how glibc does it.  Maybe they just use dgettext().

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

-- 
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] Withdraw PL/Proxy from commitfest

2008-09-05 Thread Heikki Linnakangas
So, you'll implement the part of SQL-MED that deals with specifying 
remote connections, e.g something like "CREATE CONNECTION" (no, I 
haven't looked at what the syntax actually is)?


Yeah, that sounds like a good idea. We should get that into core, and 
modify contrib/dblink to use it as well. It's just a small part of 
SQL-MED, but it's a start, and it's useful for these other projects.


Marko Kreen wrote:

In the previous discussion there was mentioned that Postgres should
move to the SQL-MED direction in remote connection handling.

SQL-MED specifies that connections should have names and referenced
everywhere using names.  PL/Proxy currently does not conform to that
standard - it uses connection strings directly.  Although it could
made work with SQL-MED backend, it would look ugly.

So I'd like to withdraw PL/Proxy from commitfest and rework it's
connection handling scheme to be also name->connstr based.  Idea will
be that it will have user-definable connection handling backend,
which operates on named connections.  And in the future we can
plug in a backend that reuses connection info from builtin SQL-MED store.

Although the current connection handling works and is secure it has
a deficiency that it's bit hard to hide the password that is used
for connecting.  User can either play with table/function permissions
and SECURITY DEFINER functions but that's complex.  Or he can put
passwords into .pgpass - this is easy and secure but has the problem
that the file is not manageable from inside database.

So PL/Proxy needs new SQL-MED based scheme that fixes it.  When this
is ready we can re-discuss the builtin vs. PL-based remote functions.
As I don't plan to work on it near-term there is no point polluting
the commitfest page with it.

[ There was a attempt to paint the .pgpass based password handling
  insecure because dblink makes the file world-readable.  I still
  fail to see how this any way points to flaws of the scheme... ]




--
  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] Need more reviewers!

2008-09-05 Thread Simon Riggs

On Fri, 2008-09-05 at 09:19 -0400, Andrew Dunstan wrote:
> 
> Simon Riggs wrote:
> > On Thu, 2008-09-04 at 10:45 -0700, Josh Berkus wrote:
> >
> >   
> >> If you are a postgresql hacker at all, or even want to be one, we need 
> >> your 
> >> help reviewing patches!  There are several "easy" patches in the list, so 
> >> I can assign them to beginners.  
> >> 
> >
> > It would be a reasonable rule that all patch submitters also have to do
> > patch reviews. If we made it a strict rule, then sponsoring companies
> > would know that they *must* provide money/time for that aspect also.
> > Otherwise it is almost impossible to get formal approval to do that.

> All this would do is to deter people from submitting patches. Hard rules 
> like this don't work in FOSS communities. I know it's like herding cats, 
> but persuasion is really our only tool.

I don't *want* the rule, I just think we *need* the rule because
otherwise sponsors/managers/etc make business decisions to exclude that
aspect of the software dev process.

Otherwise we have a patch-and-dump culture that is unsustainable because
a few people's benevolence as reviewers turns everything into a
bottleneck. It doesn't need to mean loss of control for core and
committers.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and 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] plpgsql is not translate-aware

2008-09-05 Thread Tom Lane
Alvaro Herrera <[EMAIL PROTECTED]> writes:
> Actually this is wrong -- since the library is going to run with
> "postgres" text domain, we need to add the files to the backend's
> nls.mk:

Can't we give it its own text domain?  It seems fundamentally wrong
for a plug-in language to require core support for its messages.
(Now that I think about it, that may have been the reason we don't have
localization for it already.)  I suppose this must be possible,
since e.g. glibc manages to have its own messages separate from
whatever app it's linked with.

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] plpgsql is not translate-aware

2008-09-05 Thread Alvaro Herrera
Tom Lane wrote:
> Alvaro Herrera <[EMAIL PROTECTED]> writes:
> > In reviewing Volkan Yazici's (sorry for the dots) patch to improve
> > plpgsql's error messages, I noticed that we have no PO files for plpgsql
> > at all!
> 
> Ugh.  Yeah, we should fix that.  Does it actually just work, seeing
> that plpgsql is a loadable library?

Well, it didn't, but I just tested what I posted in the followup and it
does work:

alvherre=# create function aa (internal) returns int language plpgsql as $$ 
begin; select 1; end; $$;
ERROR:  las funciones plpgsql no pueden tener el tipo internal como argumento

The vices in the error message are not the translator's fault: missing
quotes and "plpgsql" instead of "PL/pgSQL":

alvherre=# set lc_messages to 'C';
SET
alvherre=# create function aa (internal) returns int language plpgsql as $$ 
begin; select 1; end; $$;
ERROR:  plpgsql functions cannot take type internal

I'd even go a bit further and say that the original should not include
the language name in the string, so that (say) plpython and plperl can
use the same translation:

"%s functions cannot take type \"%s\"", "PL/pgSQL", type_name

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

-- 
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] Need more reviewers!

2008-09-05 Thread Marko Kreen
On 9/5/08, Marko Kreen <[EMAIL PROTECTED]> wrote:
> The list is correct but too verbose.  And it does not attack the core
>  of the problem.  I think the problem is not:
>
>   What can/should I do?
>
>  but instead:
>
>   Can I take the responsibility?

To clarify it - that was the problem I faced last commitfest.

Basically, I'm not a newbie, but I'm not deeply familiar with most of the
components in Postgres.  I'm not afraid to patch any part of code,
because I know somebody who *is* familiar with the part will later
review it.  But if I'm supposed to answer "Is this patch commitable?"
then this is too much for me.

But when I convinced myself that my only task is to decrease the amount
problems a patch has, so that committers job will be easier, then I felt
at ease to take stab on several of them.

So I suppose this works for other too and maybe this is worth accepting
as official policy - the reviewers job is not to guarantee some level
of quality, but just to report any problems they are able to find,
so that committer's job will be easier and he can concentrate on the
in-depth review.

All this assumes you want relatively nobodies doing the reviews.  If you
want guaranteed quality, then this will scare away lightweights or make
them look only one aspect of the patch.

This leaves the heavyweights, but as you know, they are busy..

>  Lets say reviewer would like look on coding style or performance.
>  ATM it seems to him he well be now fully responsible for that aspect.
>
>  I think we have better results and more relaxed atmospere if we
>  use following task description for reviewers:
>
>   The committer will do in-depth review.  You task as a reviewer
>   is to take off load from committers by catching simple problems.
>   Your task is done if you think the patch is ready for in-depth
>   review from committer.
>
>  Note1 - Yes, the trick is to emphasize that all responsibility
>  lies on committer.
>
>  Note2 - detailed lists of areas to look at and reviewer types are not
>  useful as each patch is different and each revier is different.
>  Long lists just confuse people.  The simpler the better.
>
>  The main thing is to make easy for reviewer to take the first look.


-- 
marko

-- 
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] Verbosity of Function Return Type Checks

2008-09-05 Thread Tom Lane
Volkan YAZICI <[EMAIL PROTECTED]> writes:
> On Thu, 04 Sep 2008, Tom Lane <[EMAIL PROTECTED]> writes:
>> This is not ready to go: you've lost the ability to localize most of
>> the error message strings.

> How can I make this available? What's your suggestion?

I think the best way is to use

subroutine(..., gettext_noop("special error message here"))

at the call sites, and then

errmsg("%s", _(msg))

when throwing the error.  gettext_noop() is needed to have the string
be put into the message catalog, but it doesn't represent any run-time
effort.  The _() macro is what actually makes the translation lookup
happen.  We don't want to incur that cost in the normal no-error case,
which is why you shouldn't just do _() at the call site and pass an
already-translated string to the subroutine.

>> Another problem with it is it's likely going to fail completely on
>> dropped columns (which will have atttypid = 0).

> Is it ok if I'd replace

>   if (td1->attrs[i]->atttypid != td2->attrs[i]->atttypid)

> line in validate_tupdesc_compat with

>   if (td1->attrs[i]->atttypid &&
>   td2->attrs[i]->atttypid &&
>   td1->attrs[i]->atttypid != td2->attrs[i]->atttypid)

> expression to fix this?

No, that's weakening the compatibility check.  (There's a separate issue
here of teaching plpgsql to actually cope nicely with rowtypes
containing dropped columns, but that's well beyond the scope of this
patch.)

What I'm on about is protecting format_type_be() from being passed
a zero and then failing.  Perhaps it would be good enough to do
something like

OidIsValid(td1->attrs[i]->atttypid) ?
   format_type_with_typemod(td1->attrs[i]->atttypid,
td1->attrs[i]->atttypmod) :
   "dropped column"

while throwing the error.

BTW, since what's actually being looked at is just the type OID and not
the typmod, it seems inappropriate to me to show the typmod in the
error.  I'd go with just format_type_be(td1->attrs[i]->atttypid) here
I think.

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] plpgsql is not translate-aware

2008-09-05 Thread Tom Lane
Alvaro Herrera <[EMAIL PROTECTED]> writes:
> In reviewing Volkan Yazici's (sorry for the dots) patch to improve
> plpgsql's error messages, I noticed that we have no PO files for plpgsql
> at all!

Ugh.  Yeah, we should fix that.  Does it actually just work, seeing
that plpgsql is a loadable library?

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] plpgsql is not translate-aware

2008-09-05 Thread Alvaro Herrera
Alvaro Herrera wrote:

> It doesn't seem hard to add; I just had to create a nls.mk file and
> things seem ready to go.  Obviously, we'll need to add plpgsql to the
> pgtranslation files in pgfoundry.

Actually this is wrong -- since the library is going to run with
"postgres" text domain, we need to add the files to the backend's
nls.mk:


Index: nls.mk
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/nls.mk,v
retrieving revision 1.22
diff -c -p -u -r1.22 nls.mk
--- nls.mk  24 Mar 2008 18:08:47 -  1.22
+++ nls.mk  5 Sep 2008 16:00:18 -
@@ -7,7 +7,7 @@ GETTEXT_FILES   := + gettext-files
 GETTEXT_TRIGGERS:= _ errmsg errdetail errdetail_log errhint errcontext 
write_stderr yyerror
 
 gettext-files: distprep
-   find $(srcdir)/ $(srcdir)/../port/ -name '*.c' -print >$@
+   find $(srcdir)/ $(srcdir)/../port/ $(srcdir)/../pl/ -name '*.c' -print 
>$@
 
 my-maintainer-clean:
rm -f gettext-files

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

-- 
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] [Review] pgbench duration option

2008-09-05 Thread Brendan Jurd
Hello again,

I received the following email from a helpful fellow off-list,
pointing out an error in my review:

On Fri, Sep 5, 2008 at 7:03 PM, Ragnar <[EMAIL PROTECTED]> wrote:
> On fös, 2008-09-05 at 15:07 +1000, Brendan Jurd wrote:
>> Wouldn't this be better written as:
>>
>>   if ((duration > 0 && timer_exceeded) || st->cnt >= 
>> nxacts)
>>   {
>>   
>>   }
>
> sorry, but these do not lok as the same thing to me.
>
> in the first variant there will not be a stop if
>  (duration > 0) and NOT (timer_exceeded) and (st->cnt >= nxacts)
> but in the second variant there will.
>
> admittedly, i have no idea if that situation can occur.
>
> gnari
>

gnari is right.  Looking closer I see that nxacts defaults to 10 in
the absence of a -t option, so my version of the code would end up
stopping when the run reaches 10 transactions, even if the user has
specified a -T option.

Sorry for the error.  The (duration > 0) test does in fact need to be separate.

Thanks for the catch, gnari.

Cheers,
BJ

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


[HACKERS] plpgsql is not translate-aware

2008-09-05 Thread Alvaro Herrera
Hi,

In reviewing Volkan Yazici's (sorry for the dots) patch to improve
plpgsql's error messages, I noticed that we have no PO files for plpgsql
at all!

It doesn't seem hard to add; I just had to create a nls.mk file and
things seem ready to go.  Obviously, we'll need to add plpgsql to the
pgtranslation files in pgfoundry.

There are 141 new strings to translate, and from spanish I get 71
fuzzies, so it seems an easy project.

Should I go ahead and commit the initial files?

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

-- 
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] Verbosity of Function Return Type Checks

2008-09-05 Thread Volkan YAZICI
On Fri, 5 Sep 2008, Alvaro Herrera <[EMAIL PROTECTED]> writes:
> Please use the patch I posted yesterday, as it had all the issues I
> found fixed.  There were other changes in that patch too.

My bad. Patch is modified with respect to suggestions[1][2] from
Tom. (All 115 tests passed in cvs tip.)


Regards.

[1] "char *msg" is replaced with "const char *msg".

[2] "errmsg(msg)" is replaced with 'errmsg("%s", msg)'.

Index: src/pl/plpgsql/src/pl_exec.c
===
RCS file: /projects/cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v
retrieving revision 1.219
diff -c -r1.219 pl_exec.c
*** src/pl/plpgsql/src/pl_exec.c	1 Sep 2008 22:30:33 -	1.219
--- src/pl/plpgsql/src/pl_exec.c	5 Sep 2008 13:47:07 -
***
*** 188,194 
  	   Oid reqtype, int32 reqtypmod,
  	   bool isnull);
  static void exec_init_tuple_store(PLpgSQL_execstate *estate);
! static bool compatible_tupdesc(TupleDesc td1, TupleDesc td2);
  static void exec_set_found(PLpgSQL_execstate *estate, bool state);
  static void plpgsql_create_econtext(PLpgSQL_execstate *estate);
  static void free_var(PLpgSQL_var *var);
--- 188,195 
  	   Oid reqtype, int32 reqtypmod,
  	   bool isnull);
  static void exec_init_tuple_store(PLpgSQL_execstate *estate);
! static void validate_tupdesc_compat(TupleDesc expected, TupleDesc returned,
! 	const char *msg);
  static void exec_set_found(PLpgSQL_execstate *estate, bool state);
  static void plpgsql_create_econtext(PLpgSQL_execstate *estate);
  static void free_var(PLpgSQL_var *var);
***
*** 384,394 
  			{
  case TYPEFUNC_COMPOSITE:
  	/* got the expected result rowtype, now check it */
! 	if (estate.rettupdesc == NULL ||
! 		!compatible_tupdesc(estate.rettupdesc, tupdesc))
! 		ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
!  errmsg("returned record type does not match expected record type")));
  	break;
  case TYPEFUNC_RECORD:
  
--- 385,392 
  			{
  case TYPEFUNC_COMPOSITE:
  	/* got the expected result rowtype, now check it */
! 	validate_tupdesc_compat(tupdesc, estate.rettupdesc,
! 			"returned record type does not match expected record type");
  	break;
  case TYPEFUNC_RECORD:
  
***
*** 705,715 
  		rettup = NULL;
  	else
  	{
! 		if (!compatible_tupdesc(estate.rettupdesc,
! trigdata->tg_relation->rd_att))
! 			ereport(ERROR,
! 	(errcode(ERRCODE_DATATYPE_MISMATCH),
! 	 errmsg("returned tuple structure does not match table of trigger event")));
  		/* Copy tuple to upper executor memory */
  		rettup = SPI_copytuple((HeapTuple) DatumGetPointer(estate.retval));
  	}
--- 703,711 
  		rettup = NULL;
  	else
  	{
! 		validate_tupdesc_compat(trigdata->tg_relation->rd_att,
! estate.rettupdesc,
! "returned tuple structure does not match table of trigger event");
  		/* Copy tuple to upper executor memory */
  		rettup = SPI_copytuple((HeapTuple) DatumGetPointer(estate.retval));
  	}
***
*** 2199,2209 
  		  (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
  		   errmsg("record \"%s\" is not assigned yet",
    rec->refname),
! 		   errdetail("The tuple structure of a not-yet-assigned record is indeterminate.")));
! 	if (!compatible_tupdesc(tupdesc, rec->tupdesc))
! 		ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! 		errmsg("wrong record type supplied in RETURN NEXT")));
  	tuple = rec->tup;
  }
  break;
--- 2195,2204 
  		  (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
  		   errmsg("record \"%s\" is not assigned yet",
    rec->refname),
! 		   errdetail("The tuple structure of a not-yet-assigned"
! 	 " record is indeterminate.")));
! 	validate_tupdesc_compat(tupdesc, rec->tupdesc,
! 		"wrong record type supplied in RETURN NEXT");
  	tuple = rec->tup;
  }
  break;
***
*** 2309,2318 
  		   stmt->params);
  	}
  
! 	if (!compatible_tupdesc(estate->rettupdesc, portal->tupDesc))
! 		ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! 		  errmsg("structure of query does not match function result type")));
  
  	while (true)
  	{
--- 2304,2311 
  		   stmt->params);
  	}
  
! 	validate_tupdesc_compat(estate->rettupdesc, portal->tupDesc,
! 			"structure of query does not match function result type");
  
  	while (true)
  	{
***
*** 5145,5167 
  }
  
  /*
!  * Check two tupledescs have matching number and types of attributes
   */
! static bool
! compatible_tupdesc(TupleDesc td1, TupleDesc td2)
  {
! 	int			i;
  
! 	if (td1->natts != td2->natts)
! 		return false;
  
! 	for (i = 0; i < td1->natts; i++)
! 	{
! 		if (td1->attrs[i]->atttypid != td2->attrs[i]->atttypid)
! 			return false;
! 	}
  
! 	return true;
  }
  
  /* --
--- 5138,5174 
  }
  
  /*
!

Re: [HACKERS] Need more reviewers!

2008-09-05 Thread Simon Riggs

On Fri, 2008-09-05 at 17:19 +0300, Marko Kreen wrote:
> >
> >  I think this should be organised with different kinds of reviewer:
> 
> The list is correct but too verbose.  And it does not attack the core
> of the problem.  I think the problem is not:
> 
>   What can/should I do?
> 
> but instead:
> 
>   Can I take the responsibility?

Completely agree. The list was really an example of the different styles
of review that are possible, not a rigid categorisation that must be
followed.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and 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] Need more reviewers!

2008-09-05 Thread Marko Kreen
On 9/5/08, Simon Riggs <[EMAIL PROTECTED]> wrote:
>  On Fri, 2008-09-05 at 16:03 +0200, Markus Wanner wrote:
>  > > I don't *want* the rule, I just think we *need* the rule because
>  > > otherwise sponsors/managers/etc make business decisions to exclude that
>  > > aspect of the software dev process.
>  >
>  > I agree that making sponsors/managers/etc aware of that aspect of the
>  > dev process is necessary and worthwhile. However, I don't think a rule
>  > for *patch submitters* helps with that. There must be other ways to
>  > convince managers to encourage reviewers.
>
> Such as? You might think those arguments exist and work, but I would say
>  they manifestly do not. Almost all people doing reviews are people that
>  have considerable control over their own time, or are directed by people
>  that understand the Postgres review process and wish to contribute to it
>  for commercial reasons.

Well, the number of companies who are *interested* their patches getting
in is rather small...  I think it's more common for companies to think
they are already donating to Postgres by encouraging their staff to
write patches and publish them.

So such applying such strict policy for everyone seems bad idea.
Although I quite agree on strongly encouraging patch submitters to review.
And those 3-4 companies who have direct commercial interests in Postgres
development should probably internally rethink their time allocation.

Note also we are only on our 2nd commitfest so its quite normal that
people are not used to the process .

We need to work few political aspects:

* Making reviewers to more at ease.
* Encouraging patch submitters to review.

And technical aspects:

* The (hopefully short and relaxed) rules for reviewers should be
  more visible.  Best would be on (every) Commitfest page.
* Wiki editing rules should be visible.

Well, and then:

* Although the wiki looks nice it's pain to operate.

-- 
marko

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


[HACKERS] 8.4devel out of memory

2008-09-05 Thread Kevin Grittner
I was testing a very complex statistical query, with (among other
things) many EXISTS and NOT EXISTS tests against a build of the source
snapshot from 3 September.  (The query looks pretty innocent, but
those aren't tables, they're complicated views.)  Under 8.3.3 this
query runs successfully, but takes a few hours.  I started it last
night before leaving, on the same machine where 8.3.3 has been
running, and in the morning found this:

olr=# explain analyze
SELECT
  "MS"."sMatterNo",
  CAST(COUNT(*) AS int) AS "count"
  FROM
   "MatterSearch" "MS"
   JOIN "MatterDateStat" "S" ON
 (
  "S"."matterNo" = "MS"."sMatterNo" AND
   "S"."isOnHold" = FALSE
   )
  WHERE
  (
   "MS"."matterStatusCode" IN ('OP', 'RO')
  )
  GROUP BY "MS"."sMatterNo"
;
ERROR:  out of memory
DETAIL:  Failed on request of size 8.
 
It was running for about half an hour before I left, and I didn't
notice the error, so I'm pretty sure it took longer than that for this
error to appear.
 
[EMAIL PROTECTED]:~> df -h
FilesystemSize  Used Avail Use% Mounted on
/dev/sda2  20G  8.0G   11G  43% /
tmpfs 2.0G   16K  2.0G   1% /dev/shm
/dev/sda3 253G  7.9G  245G   4% /var/pgsql/data
[EMAIL PROTECTED]:~> free -m
 total   used   free sharedbuffers
cached
Mem:  4049   2239   1809  0 94  
1083
-/+ buffers/cache:   1061   2987
Swap: 1027561466
 
There are several development databases on this machine, all fairly
small, but enough that there's usually no significant free memory --
it gets used as cache.  The 1.8 GB free this morning suggests that
something allocated and free a lot of memory.
 
[EMAIL PROTECTED]:~/postgresql-snapshot> uname -a
Linux OLR-DEV-PG 2.6.5-7.286-bigsmp #1 SMP Thu May 31 10:12:58 UTC 2007
i686 i686 i386 GNU/Linux
[EMAIL PROTECTED]:~/postgresql-snapshot> cat /proc/version
Linux version 2.6.5-7.286-bigsmp ([EMAIL PROTECTED]) (gcc version 3.3.3
(SuSE Linux)) #1 SMP Thu May 31 10:12:58 UTC 2007
[EMAIL PROTECTED]:~/postgresql-snapshot> cat /etc/SuSE-release
SUSE LINUX Enterprise Server 9 (i586)
VERSION = 9
PATCHLEVEL = 3
 
Attached are the plans from 8.3.3 and 8.4devel.  Also attached are the
non-default 8.3.3 postgresql.conf settings; the file is the same for
8.4devel except for the port number.  I don't know if the specifics of
the views and tables would be useful here, or just noise, so I'll omit
them unless someone asks for them.
 
What would be the reasonable next step here?
 
-Kevin
 
[EMAIL PROTECTED]:~> /usr/local/pgsql-8.4dev/bin/pg_config
BINDIR = /usr/local/pgsql-8.4dev/bin
DOCDIR = /usr/local/pgsql-8.4dev/share/doc
HTMLDIR = /usr/local/pgsql-8.4dev/share/doc
INCLUDEDIR = /usr/local/pgsql-8.4dev/include
PKGINCLUDEDIR = /usr/local/pgsql-8.4dev/include
INCLUDEDIR-SERVER = /usr/local/pgsql-8.4dev/include/server
LIBDIR = /usr/local/pgsql-8.4dev/lib
PKGLIBDIR = /usr/local/pgsql-8.4dev/lib
LOCALEDIR = /usr/local/pgsql-8.4dev/share/locale
MANDIR = /usr/local/pgsql-8.4dev/share/man
SHAREDIR = /usr/local/pgsql-8.4dev/share
SYSCONFDIR = /usr/local/pgsql-8.4dev/etc
PGXS = /usr/local/pgsql-8.4dev/lib/pgxs/src/makefiles/pgxs.mk
CONFIGURE = '--prefix=/usr/local/pgsql-8.4dev'
'--enable-integer-datetimes' '--enable-debug' '--disable-nls'
CC = gcc
CPPFLAGS = -D_GNU_SOURCE
CFLAGS = -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Wendif-labels
-fno-strict-aliasing -g
CFLAGS_SL = -fpic
LDFLAGS = -Wl,-rpath,'/usr/local/pgsql-8.4dev/lib'
LDFLAGS_SL =
LIBS = -lpgport -lz -lreadline -lcrypt -ldl -lm
VERSION = PostgreSQL 8.4devel
[EMAIL PROTECTED]:~> /usr/local/pgsql-8.4dev/bin/pg_controldata
/var/pgsql/data/kgrittn
pg_control version number:842
Catalog version number:   200808311
Database system identifier:   5242286260647024629
Database cluster state:   in production
pg_control last modified: Thu 04 Sep 2008 05:17:28 PM CDT
Latest checkpoint location:   0/26E7A718
Prior checkpoint location:0/26E7A6D4
Latest checkpoint's REDO location:0/26E7A718
Latest checkpoint's TimeLineID:   1
Latest checkpoint's NextXID:  0/3561
Latest checkpoint's NextOID:  49152
Latest checkpoint's NextMultiXactId:  1
Latest checkpoint's NextMultiOffset:  0
Time of latest checkpoint:Thu 04 Sep 2008 05:17:28 PM CDT
Minimum recovery ending location: 0/0
Maximum data alignment:   4
Database block size:  8192
Blocks per segment of large relation: 131072
WAL block size:   8192
Bytes per WAL segment:16777216
Maximum length of identifiers:64
Maximum columns in an index:  32
Maximum size of a TOAST chunk:2000
Date/time type storage:   64-bit integers
Float4 argument passing:  by value
Float8 argument passing:  by reference
Maximum length of locale name:128
LC_COLLATE:

Re: [HACKERS] code coverage patch

2008-09-05 Thread Alvaro Herrera
Gregory Stark wrote:
> Peter Eisentraut <[EMAIL PROTECTED]> writes:
> 
> > I have uploaded an example run here:
> > http://developer.postgresql.org/~petere/coverage/
> >
> > Current test coverage is about 66% overall.
> 
> With some pretty glaring gaps: 0% coverage of geqo, 0% coverage of logtape
> which implies no tuplesorts are spilling to disk, no coverage of mark/restore
> on index scans...

Yah, that kinda shocked me too.  Clearly we should spend some effort to
expand the regression tests a bit.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

-- 
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] Need more reviewers!

2008-09-05 Thread Markus Wanner

Hi,

Simon Riggs wrote:

Such as?


Dunno. Rules for sponsors? It would probably make sense to not only pay 
a single developer to create and submit a patch, but instead plan for 
paying others to review the code as well.



You might think those arguments exist and work, but I would say
they manifestly do not.


Most managers - especially within software companies I'd say - are 
pretty much aware of how costly quality assurance (or the lack thereof) 
can be, no?


What do you respond to potential sponsors who request that a new feature 
must be accepted into Postgres itself?


Let's tell *them* that review is costly. Encourage them to pay others to 
review your work, for example. Let's coopete ;-)  (or whatever the verb 
for coopetition is)


Maybe we can do more WRT organizing this reviewing process, including 
payment. Some sort of bounty system or something. Dunno, this is just 
some brainstorming.



Almost all people doing reviews are people that
have considerable control over their own time, or are directed by people
that understand the Postgres review process and wish to contribute to it
for commercial reasons.


Sure. I don't quite get where you are going with this argument, sorry.

Regards

Markus Wanner

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


[HACKERS] Synchronous Log Shipping Replication

2008-09-05 Thread Fujii Masao
Hi,

In PGCon 2008, I proposed synchronous log shipping replication.
Sorry for late posting, but I'd like to start the discussion
about its implementation from now.
http://www.pgcon.org/2008/schedule/track/Horizontal%20Scaling/76.en.html

First of all, I'm not planning to put the prototype which I demoed
in PGCon into core directly.

- Portability issues (using message queue, multi-threaded ...)
- Have too much dependency on Heartbeat

Yes, since the prototype is useful reference of implementation,
I plan to open it ASAP. But, I'm sorry - it still takes a month
to open it.

Pavan re-designed the sync replication based on the prototype
and I posted that design doc on wiki. Please check it if you
are interested in it.
http://wiki.postgresql.org/wiki/NTT%27s_Development_Projects

This design is too huge. In order to enhance the extensibility
of postgres, I'd like to divide the sync replication into
minimum hooks and some plugins and to develop it, respectively.
Plugins for the sync replication plan to be available at the
time of 8.4 release.

In my design, WAL sending is achieved as follow by WALSender.
WALSender is a new process which I introduce.

  1) On COMMIT, backend requests WALSender to send WAL.
  2) WALSender reads WAL from walbuffers and send it to slave.
  3) WALSender waits for the response from slave and replies
 backend.

I propose two hooks for WAL sending.

WAL-writing hook

This hook is for backend to communicate with WALSender.
WAL-writing hook intercepts write system call in XLogWrite.
That is, backend requests WAL sending whenever write is called.

WAL-writing hook is available also for other uses e.g.
Software RAID (writes WAL into two files for durability).

Hook for WALSender
--
This hook is for introducing WALSender. There are the following
three ideas of how to introduce WALSender. A required hook
differs by which idea is adopted.

a) Use WALWriter as WALSender

   This idea needs WALWriter hook which intercepts WALWriter
   literally. WALWriter stops the local WAL write and focuses on
   WAL sending. This idea is very simple, but I don't think of
   the use of WALWriter hook other than WAL sending.

b) Use new background process as WALSender

   This idea needs background-process hook which enables users
   to define new background processes. I think the design of this
   hook resembles that of rmgr hook proposed by Simon. I define
   the table like RmgrTable. It's for registering some functions
   (e.g. main function and exit...) for operating a background
   process. Postmaster calls the function from the table suitably,
   and manages a start and end of background process. ISTM that
   there are many uses in this hook, e.g. performance monitoring
   process like statspack.

c) Use one backend as WALSender

   In this idea, slave calls the user-defined function which
   takes charge of WAL sending via SQL e.g. "SELECT pg_walsender()".
   Compared with other ideas, it's easy to implement WALSender
   because postmater handles the establishment and authentication
   of connection. But, this SQL causes a long transaction which
   prevents vacuum. So, this idea needs idle-state hook which
   executes plugin before transaction starts. I don't think of
   the use of this hook other than WAL sending either.

Which idea should we adopt?

Comments welcome.

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

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


Re: [HACKERS] Need more reviewers!

2008-09-05 Thread Marko Kreen
On 9/4/08, Simon Riggs <[EMAIL PROTECTED]> wrote:
>  On Thu, 2008-09-04 at 10:45 -0700, Josh Berkus wrote:
>  > We currently have 38 patches pending, and only nine people reviewing them.
>  > At this rate, the September commitfest will take three months.
>  >
>  > If you are a postgresql hacker at all, or even want to be one, we need your
>  > help reviewing patches!  There are several "easy" patches in the list, so
>  > I can assign them to beginners.
>  >
>  > Please volunteer now!
>
>
> Everybody is stuck in "I'm not good enough to do a full review". They're
>  right (myself included), so that just means we're organising it wrongly.
>  We can't expect to grow more supermen, but we probably can do more
>  teamwork and delegation.
>
>  I think this should be organised with different kinds of reviewer:

The list is correct but too verbose.  And it does not attack the core
of the problem.  I think the problem is not:

  What can/should I do?

but instead:

  Can I take the responsibility?

Lets say reviewer would like look on coding style or performance.
ATM it seems to him he well be now fully responsible for that aspect.

I think we have better results and more relaxed atmospere if we
use following task description for reviewers:

  The committer will do in-depth review.  You task as a reviewer
  is to take off load from committers by catching simple problems.
  Your task is done if you think the patch is ready for in-depth
  review from committer.

Note1 - Yes, the trick is to emphasize that all responsibility
lies on committer.

Note2 - detailed lists of areas to look at and reviewer types are not
useful as each patch is different and each revier is different.
Long lists just confuse people.  The simpler the better.

The main thing is to make easy for reviewer to take the first look.

-- 
marko

-- 
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] Need more reviewers!

2008-09-05 Thread Simon Riggs

On Fri, 2008-09-05 at 16:03 +0200, Markus Wanner wrote:

> > I don't *want* the rule, I just think we *need* the rule because
> > otherwise sponsors/managers/etc make business decisions to exclude that
> > aspect of the software dev process.
> 
> I agree that making sponsors/managers/etc aware of that aspect of the 
> dev process is necessary and worthwhile. However, I don't think a rule 
> for *patch submitters* helps with that. There must be other ways to 
> convince managers to encourage reviewers.

Such as? You might think those arguments exist and work, but I would say
they manifestly do not. Almost all people doing reviews are people that
have considerable control over their own time, or are directed by people
that understand the Postgres review process and wish to contribute to it
for commercial reasons. 

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and 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] code coverage patch

2008-09-05 Thread Gregory Stark
Peter Eisentraut <[EMAIL PROTECTED]> writes:

> I have uploaded an example run here:
> http://developer.postgresql.org/~petere/coverage/
>
> Current test coverage is about 66% overall.

With some pretty glaring gaps: 0% coverage of geqo, 0% coverage of logtape
which implies no tuplesorts are spilling to disk, no coverage of mark/restore
on index scans...

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL 
training!

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


[HACKERS] PostgreSQL SSL problem

2008-09-05 Thread Andriy Bakay

Hi Bruce and Team,

I have problems to setup SSL for PostgreSQL server. I did all the steps
which described in the documentation (17.8. Secure TCP/IP Connections
with SSL), but when I try to start the PostgreSQL server the pg_ctl gave
me: "could not start server". And nothing in the logs (I enabled all of
them). I googled around but did not find much.

After I disable SSL option in postgresql.conf the server is starting
successfully.

I have all certificates with proper CA signature, rest of applications
(Postfix, Apache, etc.) work with this certificates very well. I am
using OpenSSL from ports.

Please, advise.

My spec:

FreeBSD 7.0-RELEASE-p3 amd64

PostgreSQL 8.3.3 (installed from ports):

WITH_NLS=true
WITHOUT_PAM=true
WITHOUT_LDAP=true
WITHOUT_MIT_KRB5=true
WITHOUT_HEIMDAL_KRB5=true
WITHOUT_OPTIMIZED_CFLAGS=true
WITH_XML=true
WITHOUT_TZDATA=true
WITHOUT_DEBUG=true
WITH_ICU=true
WITH_INTDATE=true

$ pg_config
BINDIR = /usr/local/bin
DOCDIR = /usr/local/share/doc/postgresql
INCLUDEDIR = /usr/local/include
PKGINCLUDEDIR = /usr/local/include/postgresql
INCLUDEDIR-SERVER = /usr/local/include/postgresql/server
LIBDIR = /usr/local/lib
PKGLIBDIR = /usr/local/lib/postgresql
LOCALEDIR = /usr/local/share/locale
MANDIR = /usr/local/man
SHAREDIR = /usr/local/share/postgresql
SYSCONFDIR = /usr/local/etc/postgresql
PGXS = /usr/local/lib/postgresql/pgxs/src/makefiles/pgxs.mk
CONFIGURE = '--with-libraries=/usr/local/lib'
'--with-includes=/usr/local/include' '--enable-thread-safety'
'--with-docdir=/usr/local/share/doc/postgresql' '--with-openssl'
'--with-system-tzdata=/usr/share/zoneinfo' '--enable-integer-datetimes'
'--enable-nls' '--prefix=/usr/local' '--mandir=/usr/local/man'
'--infodir=/usr/local/info/' '--build=amd64-portbld-freebsd7.0' 'CC=cc'
'CFLAGS=-O2 -fno-strict-aliasing -pipe ' 'LDFLAGS= -pthread
-rpath=/usr/local/lib' 'build_alias=amd64-portbld-freebsd7.0'
CC = cc
CPPFLAGS = -I/usr/local/include
CFLAGS = -O2 -fno-strict-aliasing -pipe  -Wall -Wmissing-prototypes
-Wpointer-arith -Winline -Wdeclaration-after-statement -Wendif-labels
-fno-strict-aliasing -fwrapv
CFLAGS_SL = -fPIC -DPIC
LDFLAGS = -pthread -rpath=/usr/local/lib -L/usr/local/lib
-Wl,-R'/usr/local/lib'
LDFLAGS_SL =
LIBS = -lpgport -lintl -lssl -lcrypto -lz -lreadline -lcrypt -lm
VERSION = PostgreSQL 8.3.3

Thanks,
Andriy


--
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] Need more reviewers!

2008-09-05 Thread Markus Wanner

Hi,

Simon Riggs wrote:

On Fri, 2008-09-05 at 09:19 -0400, Andrew Dunstan wrote:
All this would do is to deter people from submitting patches. Hard rules 
like this don't work in FOSS communities. I know it's like herding cats, 
but persuasion is really our only tool.


+1


I don't *want* the rule, I just think we *need* the rule because
otherwise sponsors/managers/etc make business decisions to exclude that
aspect of the software dev process.


I agree that making sponsors/managers/etc aware of that aspect of the 
dev process is necessary and worthwhile. However, I don't think a rule 
for *patch submitters* helps with that. There must be other ways to 
convince managers to encourage reviewers.


Regards

Markus Wanner


--
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] Need more reviewers!

2008-09-05 Thread Greg Sabino Mullane

-BEGIN PGP SIGNED MESSAGE-
Hash: RIPEMD160


> I don't *want* the rule, I just think we *need* the rule because
> otherwise sponsors/managers/etc make business decisions to exclude that
> aspect of the software dev process.

How exactly would you even begin to enforce such a rule? Retroactively
pull otherwise vali patches from the queue? Ban people from sending
email to the -patches list?

> Otherwise we have a patch-and-dump culture that is unsustainable because
> a few people's benevolence as reviewers turns everything into a
> bottleneck. It doesn't need to mean loss of control for core and
> committers.

That problem needs a solution, but not the one you proposed.

- --
Greg Sabino Mullane [EMAIL PROTECTED]
PGP Key: 0x14964AC8 200809050953
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-BEGIN PGP SIGNATURE-

iEYEAREDAAYFAkjBOdIACgkQvJuQZxSWSsiFoACgoqOgumuuZq6z2HBPSAPZUWHd
kS0An2TgFmOLTgdFWuLkpazFbECY4nnz
=ZrYl
-END PGP SIGNATURE-



-- 
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 events update

2008-09-05 Thread Andrew Chernow

Andrew Chernow wrote:


I think it got confused with the instanceData feature, which has nothing 
to do with the event system and requires public functions.  libpqtypes 
happens to use the instanceData functions within its eventproc, but this 
is not a requirement.




I forgot to mention that the instanceData functions should be moved from 
libpq-events.h to libpq-fe.h because they are not part of the event 
system.  I plan on making this change as well, so let me know if you 
hate it.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

--
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 events update

2008-09-05 Thread Andrew Chernow
I would like to remove the PQpassThroughData and PQresultPassThroughData 
functions.The passThrough pointer should be added as a 3rd argument 
to the PGEventProc:


typedef int (*PGEventProc)(PGEventId evtId, void *evtInfo,
  void *passThrough);

Having a public accessor function for the passThrough. doesn't seem 
helpful.  Its purpose is to be available to the eventproc, which doesn't 
require a public function.


I think it got confused with the instanceData feature, which has nothing 
to do with the event system and requires public functions.  libpqtypes 
happens to use the instanceData functions within its eventproc, but this 
is not a requirement.


All those who oppose any of the above, speak now or forever hold your 
peace.  An updated patch with full sgml documentation is coming.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.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] pg_regress inputdir

2008-09-05 Thread Peter Eisentraut

Jorgen Austvik - Sun Norway wrote:

Alvaro Herrera wrote:

In my opinion, the need
for running tests outside the test dir is not very strong (or we would
have heard complaints before), and thus the solution is to remove
--inputdir and --outputdir.


Attached is a patch that removes --inputdir and --outputdir. I still 
prefere the first patch (that fixed my problem), but removing them is 
probably better than having them when they don't work.


There is interest among packagers to run the regression tests or other 
tests after the build.  The Red Hat RPMs have shipped a postgresql-test 
package for years with a hacked-up makefile that will probably overwrite 
random files that it shouldn't in /usr/lib.  So I would rather be in 
favor of coming up with a solution that would make this work rather than 
removing the options.  The solution would probably be adding another 
option to place the generated files, but the exact behavior would need 
to be worked out.



--
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] Need more reviewers!

2008-09-05 Thread Andrew Dunstan



Simon Riggs wrote:

On Thu, 2008-09-04 at 10:45 -0700, Josh Berkus wrote:

  
If you are a postgresql hacker at all, or even want to be one, we need your 
help reviewing patches!  There are several "easy" patches in the list, so 
I can assign them to beginners.  



It would be a reasonable rule that all patch submitters also have to do
patch reviews. If we made it a strict rule, then sponsoring companies
would know that they *must* provide money/time for that aspect also.
Otherwise it is almost impossible to get formal approval to do that.
  



All this would do is to deter people from submitting patches. Hard rules 
like this don't work in FOSS communities. I know it's like herding cats, 
but persuasion is really our only tool.



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] code coverage patch

2008-09-05 Thread Peter Eisentraut

Michelle Caisse wrote:

Thanks, I'll take a look at these issues.


I have committed your patch with some rework that mainly addresses the 
concerns also found by Alvaro with regard to cleaning and dependency 
handling.  I have renamed the out target to coverage-html, to be more in 
line with our other targets.  So the workflow is now


./configure --enable-coverage
make
make check
make coverage-html
$BROWSER coverage/index.html

There are a couple of platform-specific problems that I have come across:

* Linux (Debian) works OK
* FreeBSD build fails, apparently because libgcov.a was not compiled 
with -fPIC
* Mac OS X builds and runs OK but the server does not shut down in 
finite time at the end of the regression tests; it has to be killed 
manually.


I have uploaded an example run here: 
http://developer.postgresql.org/~petere/coverage/


Current test coverage is about 66% overall.

--
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] Verbosity of Function Return Type Checks

2008-09-05 Thread Alvaro Herrera
Volkan YAZICI wrote:
> On Thu, 4 Sep 2008, Alvaro Herrera <[EMAIL PROTECTED]> writes:
> > Cool, thanks.  I had a look and you had some of the expected vs.
> > returned reversed.
> 
> I'll happy to fix the reversed ones if you can report them in more
> details.

Please use the patch I posted yesterday, as it had all the issues I
found fixed.  There were other changes in that patch too.

-- 
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: propose to include 3 new functions into intarray and intagg

2008-09-05 Thread Markus Wanner

Hi,

Gregory Stark wrote:

I definitely like the int_array_append_aggregate function but I don't see
anything int[] specific about it. We should be able to have a generic
array_union() aggregate which uses the same IsA(fcinfo->context, AggState)
trick to scribble on its state variable. It don't even see any reason it
couldn't work for arrays of varlenas, though it would take a bit of
restructuring.


I've just noticed that this already is a todo item:

  "Add array_accum() and array_to_set() functions for arrays

The standards specify array_agg() and UNNEST."

See also:
http://archives.postgresql.org/pgsql-hackers/2007-08/msg00464.php

Regards

Markus Wanner

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


[HACKERS] Withdraw PL/Proxy from commitfest

2008-09-05 Thread Marko Kreen
In the previous discussion there was mentioned that Postgres should
move to the SQL-MED direction in remote connection handling.

SQL-MED specifies that connections should have names and referenced
everywhere using names.  PL/Proxy currently does not conform to that
standard - it uses connection strings directly.  Although it could
made work with SQL-MED backend, it would look ugly.

So I'd like to withdraw PL/Proxy from commitfest and rework it's
connection handling scheme to be also name->connstr based.  Idea will
be that it will have user-definable connection handling backend,
which operates on named connections.  And in the future we can
plug in a backend that reuses connection info from builtin SQL-MED store.

Although the current connection handling works and is secure it has
a deficiency that it's bit hard to hide the password that is used
for connecting.  User can either play with table/function permissions
and SECURITY DEFINER functions but that's complex.  Or he can put
passwords into .pgpass - this is easy and secure but has the problem
that the file is not manageable from inside database.

So PL/Proxy needs new SQL-MED based scheme that fixes it.  When this
is ready we can re-discuss the builtin vs. PL-based remote functions.
As I don't plan to work on it near-term there is no point polluting
the commitfest page with it.

[ There was a attempt to paint the .pgpass based password handling
  insecure because dblink makes the file world-readable.  I still
  fail to see how this any way points to flaws of the scheme... ]

-- 
marko

-- 
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: propose to include 3 new functions into intarray and intagg

2008-09-05 Thread Markus Wanner

Hi,

Gregory Stark wrote:

The naming 'bidx' seems a bit weired to me, but otherwise I'm also optimistic
about it.


If we wanted to put that in core


Uh.. ATM it's just a patch against contrib. I don't think 'bidx' needs 
to go into core. Otherwise we'd have to move the whole intarr contrib 
module as well, no?



it would make more sense to have a flag on
the array indicating whether it's sorted or not which is maintained whenever
we construct or alter an array. Then just have the regular _int_contains()
(which is an operator @>) take advantage of it if it's set and the data type
is fixed-size.


Yeah, that sounds like a good thing. Currently, the intarr module 
doesn't provide the optimized functions for the outside world 
(_int_inter_inner() and such.. the _inner appendix really means "inside 
intarr module only). I've ended up copy'n'pasting that code into my own 
module, where I take care about ordering myself. But still want to 
maintain the optimization.


However, that's probably not within the scope of this patch.

Regards

Markus Wanner



--
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: propose to include 3 new functions into intarray and intagg

2008-09-05 Thread Gregory Stark
Markus Wanner <[EMAIL PROTECTED]> writes:

> Hi,
>
> Gregory Stark wrote:
>> Regarding the patch listed on the commitfest "3 new functions into intarray
>> and intagg" (which I just noticed has a reviewer listed -- doh):
>
> ..well, just add your name as well, no?

Yeah, people should feel free to comment on items even if other people have or
are as well. It would have just been more useful for me to pick one that
didn't already have someone reading it is all.

>> As far as detailed code commentary the only thing which jumps out at me is
>> that it's using MemoryContextAlloc to grow the array instead of repalloc 
>> which
>> seems like a waste. This isn't a new thing though, it was how intagg was
>> written and this patch just didn't change it.
>
> Oh, good catch.

Actually on further thought what's going on is that it's resizing the array by
doubling its size when necessary. palloc/repalloc does that as well, so you're
getting two layers of extra space to handle reallocations. I think it's
simpler and cleaner to just repalloc just enough space at each growth and let
repalloc handle allocating extra space to handle future growth. I think that
would be necessary for handling varlenas also.

> The naming 'bidx' seems a bit weired to me, but otherwise I'm also optimistic
> about it.

If we wanted to put that in core it would make more sense to have a flag on
the array indicating whether it's sorted or not which is maintained whenever
we construct or alter an array. Then just have the regular _int_contains()
(which is an operator @>) take advantage of it if it's set and the data type
is fixed-size.

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's On-Demand Production Tuning

-- 
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] Need more reviewers!

2008-09-05 Thread Ibrar Ahmed

Josh Berkus wrote:

Hackers,

We currently have 38 patches pending, and only nine people reviewing them.  
At this rate, the September commitfest will take three months.  

If you are a postgresql hacker at all, or even want to be one, we need your 
help reviewing patches!  There are several "easy" patches in the list, so 
I can assign them to beginners.  


Please volunteer now!

  

Please assign me one; any of the "easy" ones.


--
 Ibrar Ahmed
 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] Need more reviewers!

2008-09-05 Thread Devrim GÜNDÜZ
On Fri, 2008-09-05 at 12:18 +0100, Simon Riggs wrote:
> 
> It would be a reasonable rule that all patch submitters also have to
> do patch reviews.

This is almost the only way to be accepted as a contributor to Fedora --
and I like it. 
-- 
Devrim GÜNDÜZ, RHCE
devrim~gunduz.org, devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr
   http://www.gunduz.org


signature.asc
Description: This is a digitally signed message part


Re: [HACKERS] Need more reviewers!

2008-09-05 Thread Robert Haas
> That way, instead of just an appeal to the masses to volunteer for
> $NEBULOUS_TASK, we can say something like "Please volunteer to review
> patches.  Doing an initial patch review is easy, please see our guide
>  to learn more."

+1.  I'll review a patch if you like, but the patch I have in this
'fest is only one of two I've ever submitted, and my understanding of
PG guts is very weak, so I confidently predict that me looking at it
has maybe 5% of the value of a committer looking at it, so I'm not
sure that really gets us anywhere.  Even if I think the patch is
great, my opinion doesn't and shouldn't carry much weight compared to
someone like Simon or Zdenek who are probably perceived by the
committers as much more likely to have a clue, so the committer is
just going to end up reviewing it all over again anyway.

...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] Need more reviewers!

2008-09-05 Thread Ibrar Ahmed

Josh Berkus wrote:

Hackers,

We currently have 38 patches pending, and only nine people reviewing them.  
At this rate, the September commitfest will take three months.  

If you are a postgresql hacker at all, or even want to be one, we need your 
help reviewing patches!  There are several "easy" patches in the list, so 
I can assign them to beginners.  


Please volunteer now!

  

Please assign me one; any of the "easy" ones.

--
 Ibrar Ahmed
 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] Patch: propose to include 3 new functions into intarray and intagg

2008-09-05 Thread Markus Wanner

Hi,

Gregory Stark wrote:

Regarding the patch listed on the commitfest "3 new functions into intarray
and intagg" (which I just noticed has a reviewer listed -- doh):


..well, just add your name as well, no?


I definitely like the int_array_append_aggregate function but I don't see
anything int[] specific about it. We should be able to have a generic
array_union() aggregate which uses the same IsA(fcinfo->context, AggState)
trick to scribble on its state variable. It don't even see any reason it
couldn't work for arrays of varlenas, though it would take a bit of
restructuring.


Yeah, the same idea was bugging me. Doesn't such code already exist?


So I would be definitely for a adding this to core if it were rewritten to
work with generic arrays which, unless there are problems I'm not seeing, I
don't think would be very hard.

As far as detailed code commentary the only thing which jumps out at me is
that it's using MemoryContextAlloc to grow the array instead of repalloc which
seems like a waste. This isn't a new thing though, it was how intagg was
written and this patch just didn't change it.


Oh, good catch.


I'm not against putting more functions into intagg and intarray and bidx and
the grouping/counting thing seem like they might be useful functionality. but
I have a feeling others might feel differently.


The naming 'bidx' seems a bit weired to me, but otherwise I'm also 
optimistic about it.


Regards

Markus Wanner

--
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] Need more reviewers!

2008-09-05 Thread Simon Riggs

On Thu, 2008-09-04 at 10:45 -0700, Josh Berkus wrote:

> If you are a postgresql hacker at all, or even want to be one, we need your 
> help reviewing patches!  There are several "easy" patches in the list, so 
> I can assign them to beginners.  

It would be a reasonable rule that all patch submitters also have to do
patch reviews. If we made it a strict rule, then sponsoring companies
would know that they *must* provide money/time for that aspect also.
Otherwise it is almost impossible to get formal approval to do that.

I count more than 20 patch authors that are not reviewing, including
myself. Of that, there are at least 5 people on their second or
subsequent patch (figure probably wildly inaccurate, since don't keep
track of that).

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and 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: propose to include 3 new functions into intarray and intagg

2008-09-05 Thread Gregory Stark

Regarding the patch listed on the commitfest "3 new functions into intarray
and intagg" (which I just noticed has a reviewer listed -- doh):

http://archives.postgresql.org/message-id/[EMAIL PROTECTED]

I definitely like the int_array_append_aggregate function but I don't see
anything int[] specific about it. We should be able to have a generic
array_union() aggregate which uses the same IsA(fcinfo->context, AggState)
trick to scribble on its state variable. It don't even see any reason it
couldn't work for arrays of varlenas, though it would take a bit of
restructuring.

So I would be definitely for a adding this to core if it were rewritten to
work with generic arrays which, unless there are problems I'm not seeing, I
don't think would be very hard.

As far as detailed code commentary the only thing which jumps out at me is
that it's using MemoryContextAlloc to grow the array instead of repalloc which
seems like a waste. This isn't a new thing though, it was how intagg was
written and this patch just didn't change it.

I'm not against putting more functions into intagg and intarray and bidx and
the grouping/counting thing seem like they might be useful functionality. but
I have a feeling others might feel differently.

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's RemoteDBA 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] WIP: Column-level Privileges

2008-09-05 Thread Markus Wanner

Hi,

Stephen Frost wrote:

I would suggest you review the updated patch (linked off the wiki page)
here:
http://archives.postgresql.org/message-id/[EMAIL PROTECTED]


That's the patch I've been talking about: file 
colprivs_wip.20080902.diff.gz, dated Sept, 2nd.



It includes my comments about what's done and what's not.  I reiterated
much of it here as well:
http://archives.postgresql.org/pgsql-hackers/2008-09/msg00263.php


Uh.. I've read that message as well, but didn't find it overly clear on 
what you were referring to.



As mentioned in above, regression tests, documentation updates,
dependency handling, and actually implementing the permission checks all
remain.  What I'm looking for feedback on are the changes to the
grammer, parser, catalog changes, psql output, etc.


Aha, good. So I'm going to (try to) check these things and comment.

Regards

Markus Wanner


--
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: Column-level Privileges

2008-09-05 Thread Stephen Frost
Hi Markus,

* Markus Wanner ([EMAIL PROTECTED]) wrote:
> Stephen Frost wrote:
>>   Comments welcome, apologies for it not being ready by 9/1.  I'm
>>   planning to continue working on it tomorrow, and throughout September
>>   as opportunity allows (ie: when Josh isn't keeping me busy).
>
> I'm trying to review this patch. I could at least compile it  
> successfully for now.

That's a start at least. :)

> There have already been some comments from Tom [1]. You've mostly  
> answered that you are going to fix these issues, but I'm pretty unclear  
> on what the current status is (of patch 09/02). Can you please elaborate  
> on what's done and what not?

I would suggest you review the updated patch (linked off the wiki page)
here:
http://archives.postgresql.org/message-id/[EMAIL PROTECTED]

It includes my comments about what's done and what's not.  I reiterated
much of it here as well:
http://archives.postgresql.org/pgsql-hackers/2008-09/msg00263.php

> As this is still marked as WIP on the wiki page: what needs to be done  
> until you consider it done?

As mentioned in above, regression tests, documentation updates,
dependency handling, and actually implementing the permission checks all
remain.  What I'm looking for feedback on are the changes to the
grammer, parser, catalog changes, psql output, etc.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Page layout footprint

2008-09-05 Thread Heikki Linnakangas

Zdenek Kotala wrote:
OK. You convinced me that information could be collected from other 
sources.


Great, I'm glad we're in agreement.

--
  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] Page layout footprint

2008-09-05 Thread Zdenek Kotala

Heikki Linnakangas napsal(a):

Zdenek Kotala wrote:

Heikki Linnakangas napsal(a):




AFAICS you can get all the same information from pg_controldata. We have 
a pretty good idea of the alignments of all the usual platforms anyway. 
If someone says in a bug report that they're running on x86_64 or 32-bit 
Sparc, we know what the alignments on those platforms are.


And if you're working at such a low level, it's not that hard to 
calculate the offsets within the struct manually.


I'm not sure if it is so easy. Are you able do it for SPARC, PPC or 
other non x86 CPUs?


Not off the top of my head. But I am able to do it on x86 and x86_64 
which are the platforms I work and debug on.


OK. You convinced me that information could be collected from other sources.




It could also report changes in alignment.


Huh? I would be pretty surprised if the alignment changed randomly on 
some platform.


I thought if somebody change compiler switches - for example 32/64 compilation. 
But it is probably rare case.


Zdenek


--
Zdenek Kotala  Sun Microsystems
Prague, Czech Republic http://sun.com/postgresql


--
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] For what should pg_stop_backup wait?

2008-09-05 Thread Simon Riggs

On Fri, 2008-08-08 at 11:47 +0900, Fujii Masao wrote:
> On Thu, Aug 7, 2008 at 11:34 PM, Simon Riggs <[EMAIL PROTECTED]> wrote:
> >
> > On Thu, 2008-08-07 at 14:59 +0100, Simon Riggs wrote:
> >
> >> I'll do a patch. Thanks for your input.
> >
> > Please review attached patch.
> 
> Thank you for your patch!

Would you mind if I asked you to be the official reviewer of this patch
for the latest CommitFest? It would help greatly, since you understand
the issue and clearly the code also. Thanks, if possible.

 http://wiki.postgresql.org/wiki/CommitFest:2008-09

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and 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] Page layout footprint

2008-09-05 Thread Heikki Linnakangas

Zdenek Kotala wrote:

Heikki Linnakangas napsal(a):
I'm afraid I still fail to see the usefulness of this. gdb knows how 
to deal with structs,


If I correct that GDB knows structure only if you have debug version. 
But usually you don't have debug version on production system. 


Using gdb without debug systems is pretty much a lost cause anyway.

And 
another small advantage is that --footprint switch is easy to use. It is 
easier that work with gdb and you can easy get info from users who are 
not familiar with gdb.


AFAICS you can get all the same information from pg_controldata. We have 
a pretty good idea of the alignments of all the usual platforms anyway. 
If someone says in a bug report that they're running on x86_64 or 32-bit 
Sparc, we know what the alignments on those platforms are.


And if you're working at such a low level, it's not that hard to 
calculate the offsets within the struct manually.


I'm not sure if it is so easy. Are you able do it for SPARC, PPC or 
other non x86 CPUs?


Not off the top of my head. But I am able to do it on x86 and x86_64 
which are the platforms I work and debug on.


If we needed more information about the architectures, we could just 
collect the output of pg_controldata. But I think the configure logs 
already contains all the useful information.


It seems to be good idea. Only what we need is extend buildfarm to parse 
config.log and shows this data for each build machine. 


Well, the information is already there. I'm not convinced it's such an 
important issue that it's worth the effort to add special handling to 
extract that information from the log. Of course, if someone feels 
otherwise and does it, I won't object.



It could also report changes in alignment.


Huh? I would be pretty surprised if the alignment changed randomly on 
some platform.


--
  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] Page layout footprint

2008-09-05 Thread Zdenek Kotala

Gregory Stark napsal(a):

Zdenek Kotala <[EMAIL PROTECTED]> writes:


Hmm, good question.  For example ZFS is platform independent, you can take disk
from SPARC machine and plug it into x86 and ZFS works perfectly. 


FWIW as far as I know *all* filesystems are platform independent. (Of course
now someone is surely going to find some counter-example) Doesn't really
change the argument though.


Yeah, of course. I selected bad word. ZFS write data structures in native endian 
format. It does not need convert data to correct byte order. Only if you switch 
harddisk from SPARC to x86 you need convert bytes order. Other systems has a 
penalty for endian conversion on oposit platform or conversion is not supported. 
However, I'm not sure if ext3 filesystem which is created on x86 machine is 
readable under linux on SPARC machine? FAT32 works fine :-) everywhere.


Maybe optimized platform independence is better term.

Zdenek


--
Zdenek Kotala  Sun Microsystems
Prague, Czech Republic http://sun.com/postgresql


--
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] Page layout footprint

2008-09-05 Thread Gregory Stark

Zdenek Kotala <[EMAIL PROTECTED]> writes:

> Hmm, good question.  For example ZFS is platform independent, you can take 
> disk
> from SPARC machine and plug it into x86 and ZFS works perfectly. 

FWIW as far as I know *all* filesystems are platform independent. (Of course
now someone is surely going to find some counter-example) Doesn't really
change the argument though.

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's Slony Replication 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] Prototype: In-place upgrade

2008-09-05 Thread Zdenek Kotala

Heikki Linnakangas napsal(a):

The patch seems to be missing the new htup.c file.


Upps, I'm sorry I'm going to fix it and I will send new version asap.

Zdenek


--
Zdenek Kotala  Sun Microsystems
Prague, Czech Republic http://sun.com/postgresql


--
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] Page layout footprint

2008-09-05 Thread Zdenek Kotala

Heikki Linnakangas napsal(a):

Zdenek Kotala wrote:
The original proposal 
(http://archives.postgresql.org/message-id/[EMAIL PROTECTED])
contains two parts. First part is implementation of --footprint cmd 
line switch which shows you  page layout structures footprint. It is 
useful for development (mostly for in-place upgrade) and also for 
manual data recovery when you need to know exact structures.


I'm afraid I still fail to see the usefulness of this. gdb knows how to 
deal with structs,


If I correct that GDB knows structure only if you have debug version. But 
usually you don't have debug version on production system. And another small 
advantage is that --footprint switch is easy to use. It is easier that work with 
gdb and you can easy get info from users who are not familiar with gdb.


and for manual data recovery you need so much more 
than the page header structure. 


Yeah, I know, but I didn't want to spend several days with full coding without 
idea approval. There are special data, meta pages and so on which have to be added.


And if you're working at such a low 
level, it's not that hard to calculate the offsets within the struct 
manually.


I'm not sure if it is so easy. Are you able do it for SPARC, PPC or other non 
x86 CPUs?


BTW, this makes me wonder if it would be possible to use the 
upgrade-in-place machinery to convert a data directory from one 
architecture to another? Just a thought..


Hmm, good question.  For example ZFS is platform independent, you can take disk 
from SPARC machine and plug it into x86 and ZFS works perfectly. ZFS converts 
its own data during a read and any new block is written in a new format. You are 
able read all binary platform independent data like MP3, JPEG and so on. But 
PostgreSQL will not work, because PostgreSQL fails during pg_control file 
reading, because endianes are different.


Convert data structures like PageHeader and so on could be possible but you 
don't have control over user data types.


I think in this case is better to develop platform independent replication and 
use this mechanism for data transfer.



However, there is still --footprint switch which is useful and it is 
reason why I put it on wiki for review and feedback. The switch could 
also use in build farm for collecting footprints from build farm members.


If we needed more information about the architectures, we could just 
collect the output of pg_controldata. But I think the configure logs 
already contains all the useful information.




It seems to be good idea. Only what we need is extend buildfarm to parse 
config.log and shows this data for each build machine. It could also report 
changes in alignment.


Zdenek


--
Zdenek Kotala  Sun Microsystems
Prague, Czech Republic http://sun.com/postgresql


--
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] New FSM patch

2008-09-05 Thread Heikki Linnakangas

Simon Riggs wrote:

On Thu, 2008-09-04 at 11:07 +0300, Heikki Linnakangas wrote:
Scenario: The binary tree on a page is corrupt, so that the value of an 
upper node is > Max(leftchild, rightchild).
Consequence: Searchers will notice the corruption while trying to 
traverse down that path, and throw an elog(WARNING) in search_avail(). 
fsm_search will retry the search, and will in worst case go into an 
infinite loop. That's obviously not good. We could automatically fix the 
upper nodes of the tree, but that would wipe evidence that would be 
useful in debugging.


We probably need to break out of infinite loops, especially ones that
output warning messages on each loop. :-)


Yep. I turned that warning into an error in the latest patch I just 
posted elsewhere in this thread.


--
  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] WIP: Column-level Privileges

2008-09-05 Thread Markus Wanner

Hello Stephen,

Stephen Frost wrote:

  Comments welcome, apologies for it not being ready by 9/1.  I'm
  planning to continue working on it tomorrow, and throughout September
  as opportunity allows (ie: when Josh isn't keeping me busy).


I'm trying to review this patch. I could at least compile it 
successfully for now.


There have already been some comments from Tom [1]. You've mostly 
answered that you are going to fix these issues, but I'm pretty unclear 
on what the current status is (of patch 09/02). Can you please elaborate 
on what's done and what not?


As this is still marked as WIP on the wiki page: what needs to be done 
until you consider it done?


Regards

Markus Wanner

[1]: Comments from Tom:
http://archives.postgresql.org/pgsql-patches/2008-05/msg00111.php


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