Re: [HACKERS] plperl and inline functions -- first draft

2009-11-29 Thread Alexey Klyukin

On Nov 29, 2009, at 4:40 AM, Tom Lane wrote:

 Alexey Klyukin al...@commandprompt.com writes:
 
 Isn't it also the case with the existing plperl code ? I've noticed that 
 free(prodesc) is called when it's no longer used (i.e. in 
 plperl_compile_callback:1636), but refcount of desc-reference is never 
 decremented.
 
 I've been experimenting with this and confirmed that there is a leak;
 not only in the DO patch but in the pre-existing code, if a plperl
 function is redefined repeatedly.
 
 Is this the correct way to release the SV* reference?
 
   if (reference)
   SvREFCNT_dec(reference);


Yes. In fact this only decreases the reference count, making the interpreter 
free the memory referred to when it becomes 0, but since prodesc-reference has 
refcount of 1 this would do the right thing.

--
Alexey Klyukin  http://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] Timezones (in 8.5?)

2009-11-29 Thread Andrew Gierth
 Bruce == Bruce Momjian br...@momjian.us writes:

 Bruce I think there is general agreement that we should have a
 Bruce timezone data type which validates against
 Bruce pg_timezone_names().name.

What happens when pg_timezone_names output changes? (which it can do,
especially if the install is using the OS tzdata; even if not using OS
tzdata, it's not expected to be stable even between point releases)

-- 
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] compile error with -DOPTIMIZER_DEBUG

2009-11-29 Thread Bruce Momjian
Tom Lane wrote:
 Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
  Jan Urba??ski wrote:
  ISTM that there's a superfluous curly brace in print_path (which only
  gets compiled with -DOPTIMIZER_DEBUG.
 
  Thanks, committed.
 
 You know, the last couple of times I've touched that code, I've been
 wondering why we bother to maintain it.  Personally I always use pprint()
 when I'm interested in a printout of a plan tree.  Is anyone actually
 using the printout code in allpaths.c?

I thought OPTIMIZER_DEBUG showed us all the possible paths, not just the
final plan.

-- 
  Bruce Momjian  br...@momjian.ushttp://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] LDAP where DN does not include UID attribute

2009-11-29 Thread Magnus Hagander
On Fri, Sep 18, 2009 at 02:24, Robert Fleming flemi...@gmail.com wrote:
 On Thu, Sep 17, 2009 at 11:15 AM, Magnus Hagander mag...@hagander.net
 wrote:

 On Thu, Sep 17, 2009 at 18:02, Robert Fleming flemi...@gmail.com wrote:
  Following a discussion on the pgsql-admin list
  http://archives.postgresql.org/pgsql-admin/2009-09/msg00075.php, I
  have
  created a patch to (optionally) allow PostgreSQL to do a LDAP search to
  determine the user's DN (as is done in Apache, MediaWiki, Bugzilla, et
  al.)
  instead of building the DN from a prefix and suffix.
  This is necessary for schemas where the login attribute is not in the
  DN,
  such as is described here
  http://www.ldapman.org/articles/intro_to_ldap.html#individual (look
  for
  name-based).  This patch is against PostgreSQL 8.4.0 from Debian
  Lenny-backports.  If this would be a welcome addition, I can port it
  forward
  to the latest from postgresql.org.
  Thanks in advance for your feedback.

 This sounds like a very useful feature, and one that I can then remove
 from my personal TODO list without having to do much work :-)

 A couple of comments:

 First of all, please read up on the PostgreSQL coding style, or at
 least look at the code around yours. This doesn't look anything like
 our standards.

 Sorry, the 8.1 manual said all contributions go through pgindent so I didn't
 spend too much time on that.  I see that the 8.4 manual clarifies that
 pgindent won't get run till release time.  In any case, I've re-formatted
 the patch using the Emacs settings from the 8.1 manual (why are they gone
 from the 8.4 manual)?  It seems like most of the rest of the Coding
 Conventions have to do with error reporting.  Do please let me know if
 there's something else I can do.

 Second, this appears to require an anonymous bind to the directory,
 which is something we should not encourage people to enable on their
 LDAP servers. I think we need to also take parameters with a DN and a
 password to bind with in order to do the search, and then re-bind as
 the user when found.

 The new patch implements the initial bind using new configuration parameters
 ldapbinddn and ldapbindpasswd.  I've also added a ldapsearchattribute
 just to be complete.

I've finally had the time to look at this patch some more, for the
current commitfest - sorry about the delay.

Other than minor style changes (make the indentation be the same as
the code around it), I noticed one fairly large issue.

The way that LDAP is re-initialized both breaks Windows (in the error
reporting part) and not as obvious but even more importantly, it
breaks TLS. If you enable TLS for LDAP it will only be used for the
search, not for the actual password check - which really removes the
point of it :-)

I assume we need the second ldap_init() because we want to do a new
ldap_simple_bind()? In that case, we realliy need to re-initialize the
whole connection, including TLS.

I also notice that it's hardcoded to retrieve the uid attribute,
while the one being searched for can be configured. ISTM it's better
to set both of these to the hba-ldapsearchattribute value - so we
won't fail if uid does not exist.

Also, as I'm sure you're aware there are no documentation updates included.

I'll be happy to work on this to get it ready for commit, or do you
want to do the updates?

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

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


Re: [HACKERS] cvs chapters in our docs

2009-11-29 Thread Bruce Momjian
Robert Haas wrote:
 I have to say I'm not really impressed by the idea of removing things
 from our documentation and replacing them with pages on the wiki.  The
 documentation is better-written and easier to navigate.  Yeah, the
 part about 28K modems is pretty silly, but we can fix that without
 throwing the baby out with the bathwater...

Wow, we mention 28k modems --- we are legacy software:  ;-)

 This initial checkout is a little slower than simply downloading
 a filenametar.gz/filename file; expect it to take 40 minutes
 or so if you have a 28.8K modem.

-- 
  Bruce Momjian  br...@momjian.ushttp://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] Timezones (in 8.5?)

2009-11-29 Thread Bruce Momjian
Andrew Gierth wrote:
  Bruce == Bruce Momjian br...@momjian.us writes:
 
  Bruce I think there is general agreement that we should have a
  Bruce timezone data type which validates against
  Bruce pg_timezone_names().name.
 
 What happens when pg_timezone_names output changes? (which it can do,
 especially if the install is using the OS tzdata; even if not using OS
 tzdata, it's not expected to be stable even between point releases)

Uh, wow, yea, that would invalidate stored data --- yuck.

-- 
  Bruce Momjian  br...@momjian.ushttp://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


[HACKERS] A small issue in CommentCast function

2009-11-29 Thread Gokulakannan Somasundaram
Hi,
   I think this is more of code uniformity issue. Nevertheless, i just
thought of putting it here.
   In the CommentCast function, i think both sourcetype and targettype
should be in the arguments, whereas in the code, source type is made as the
name and the target type is passed in as argument. So both of them should
get stored in the objargs member of the CommentStmt struct.

Thanks,
Gokul.


Re: [HACKERS] compile error with -DOPTIMIZER_DEBUG

2009-11-29 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 Tom Lane wrote:
 You know, the last couple of times I've touched that code, I've been
 wondering why we bother to maintain it.  Personally I always use pprint()
 when I'm interested in a printout of a plan tree.  Is anyone actually
 using the printout code in allpaths.c?

 I thought OPTIMIZER_DEBUG showed us all the possible paths, not just the
 final plan.

Yeah, but we could repoint that code at pprint.

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 chapters in our docs

2009-11-29 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 Robert Haas wrote:
 I have to say I'm not really impressed by the idea of removing things
 from our documentation and replacing them with pages on the wiki.  The
 documentation is better-written and easier to navigate.  Yeah, the
 part about 28K modems is pretty silly, but we can fix that without
 throwing the baby out with the bathwater...

 Wow, we mention 28k modems --- we are legacy software:  ;-)

The depressing thing is we can't even blame that on Berkeley ...
if memory serves, I wrote 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] Application name patch - v4

2009-11-29 Thread Dave Page
On Sat, Nov 28, 2009 at 11:47 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Dave Page dp...@pgadmin.org writes:
 Updated application name patch, including a GUC assign hook to clean
 the application name of any unsafe characters, per discussion.

 Applied with assorted editorialization.  There were a couple of
 definitional issues that I don't recall if we had consensus on:

 1. The patch prevents non-superusers from seeing other users'
 application names in pg_stat_activity.  This seems at best pretty
 debatable to me.  Yes, it supports usages in which you want to put
 security-sensitive information into the appname, but at the cost of
 disabling (perfectly reasonable) usages where you don't.  If we made
 the app name universally visible, people simply wouldn't put security
 sensitive info in it, the same as they don't put it on the command line.
 Should we change this?

Uh, yeah, I guess. That wasn't a concious decision, more a copy n
paste inherited 'feature'.

 (While I'm looking at it, I wonder why client_addr and client_port
 are similarly hidden.)

 2. I am wondering if we should mark application_name as
 GUC_NO_RESET_ALL.  As-is, the value sent at libpq initialization
 will be lost during RESET ALL, which would probably surprise people.
 On the other hand, not resetting it might surprise other people.
 If we were able to send it in the startup packet then this wouldn't
 be a problem, but we are far from being able to do that.

In the use cases I envisage for this, the appname is more a property
of the connection than the session, thus I wouldn't expect it to
change following a RESET ALL. That said, one could then argue that it
should RESET to the connection-time value...

I think we should use GUC_NO_RESET_ALL.

-- 
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

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


Re: [HACKERS] LDAP where DN does not include UID attribute

2009-11-29 Thread Magnus Hagander
On Sun, Nov 29, 2009 at 13:05, Magnus Hagander mag...@hagander.net wrote:
 On Fri, Sep 18, 2009 at 02:24, Robert Fleming flemi...@gmail.com wrote:
 On Thu, Sep 17, 2009 at 11:15 AM, Magnus Hagander mag...@hagander.net
 wrote:

 On Thu, Sep 17, 2009 at 18:02, Robert Fleming flemi...@gmail.com wrote:
  Following a discussion on the pgsql-admin list
  http://archives.postgresql.org/pgsql-admin/2009-09/msg00075.php, I
  have
  created a patch to (optionally) allow PostgreSQL to do a LDAP search to
  determine the user's DN (as is done in Apache, MediaWiki, Bugzilla, et
  al.)
  instead of building the DN from a prefix and suffix.
  This is necessary for schemas where the login attribute is not in the
  DN,
  such as is described here
  http://www.ldapman.org/articles/intro_to_ldap.html#individual (look
  for
  name-based).  This patch is against PostgreSQL 8.4.0 from Debian
  Lenny-backports.  If this would be a welcome addition, I can port it
  forward
  to the latest from postgresql.org.
  Thanks in advance for your feedback.

 This sounds like a very useful feature, and one that I can then remove
 from my personal TODO list without having to do much work :-)

 A couple of comments:

 First of all, please read up on the PostgreSQL coding style, or at
 least look at the code around yours. This doesn't look anything like
 our standards.

 Sorry, the 8.1 manual said all contributions go through pgindent so I didn't
 spend too much time on that.  I see that the 8.4 manual clarifies that
 pgindent won't get run till release time.  In any case, I've re-formatted
 the patch using the Emacs settings from the 8.1 manual (why are they gone
 from the 8.4 manual)?  It seems like most of the rest of the Coding
 Conventions have to do with error reporting.  Do please let me know if
 there's something else I can do.

 Second, this appears to require an anonymous bind to the directory,
 which is something we should not encourage people to enable on their
 LDAP servers. I think we need to also take parameters with a DN and a
 password to bind with in order to do the search, and then re-bind as
 the user when found.

 The new patch implements the initial bind using new configuration parameters
 ldapbinddn and ldapbindpasswd.  I've also added a ldapsearchattribute
 just to be complete.

 I've finally had the time to look at this patch some more, for the
 current commitfest - sorry about the delay.

 Other than minor style changes (make the indentation be the same as
 the code around it), I noticed one fairly large issue.

 The way that LDAP is re-initialized both breaks Windows (in the error
 reporting part) and not as obvious but even more importantly, it
 breaks TLS. If you enable TLS for LDAP it will only be used for the
 search, not for the actual password check - which really removes the
 point of it :-)

 I assume we need the second ldap_init() because we want to do a new
 ldap_simple_bind()? In that case, we realliy need to re-initialize the
 whole connection, including TLS.

 I also notice that it's hardcoded to retrieve the uid attribute,
 while the one being searched for can be configured. ISTM it's better
 to set both of these to the hba-ldapsearchattribute value - so we
 won't fail if uid does not exist.

 Also, as I'm sure you're aware there are no documentation updates included.

 I'll be happy to work on this to get it ready for commit, or do you
 want to do the updates?

Here's a patch with my work so far. Major points missing is the
sanitizing of the username and the docs.

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


ldap_search.patch
Description: Binary data

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


Re: [HACKERS] Application name patch - v4

2009-11-29 Thread Tom Lane
Dave Page dp...@pgadmin.org writes:
 On Sat, Nov 28, 2009 at 11:47 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 1. The patch prevents non-superusers from seeing other users'
 application names in pg_stat_activity.  This seems at best pretty
 debatable to me.  Yes, it supports usages in which you want to put
 security-sensitive information into the appname, but at the cost of
 disabling (perfectly reasonable) usages where you don't.  If we made
 the app name universally visible, people simply wouldn't put security
 sensitive info in it, the same as they don't put it on the command line.
 Should we change this?

 Uh, yeah, I guess. That wasn't a concious decision, more a copy n
 paste inherited 'feature'.

OK.  Everybody seems to agree it should not be hidden, so I'll go change
that.

 2. I am wondering if we should mark application_name as
 GUC_NO_RESET_ALL.

 I think we should use GUC_NO_RESET_ALL.

I agree with you, but it seems we have at least as many votes to not do
that.  Any other votes out there?

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 chapters in our docs

2009-11-29 Thread Bruce Momjian
Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  Robert Haas wrote:
  I have to say I'm not really impressed by the idea of removing things
  from our documentation and replacing them with pages on the wiki.  The
  documentation is better-written and easier to navigate.  Yeah, the
  part about 28K modems is pretty silly, but we can fix that without
  throwing the baby out with the bathwater...
 
  Wow, we mention 28k modems --- we are legacy software:  ;-)
 
 The depressing thing is we can't even blame that on Berkeley ...
 if memory serves, I wrote it :-(

There is no mention of paper tape or punch cards in our docs.  :-)

-- 
  Bruce Momjian  br...@momjian.ushttp://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] Timezones (in 8.5?)

2009-11-29 Thread David E. Wheeler
On Nov 28, 2009, at 5:40 PM, Bruce Momjian wrote:

 I think there is general agreement that we should have a timezone data
 type which validates against pg_timezone_names().name.  It might be
 enough to just document how users can create such a domain data type,
 but I don't know of a way to do that.  Is this a TODO?

From 
http://justatheory.com/computers/databases/postgresql/citext-patch-submitted.html

CREATE OR REPLACE FUNCTION is_timezone( tz TEXT ) RETURNS BOOLEAN as $$
BEGIN
  PERFORM now() AT TIME ZONE tz;
  RETURN TRUE;
EXCEPTION WHEN invalid_parameter_value THEN
  RETURN FALSE;
END;
$$ language plpgsql STABLE;

CREATE DOMAIN timezone AS CITEXT
CHECK ( is_timezone( value ) );

It could also be TEXT I suppose, but America/Los_Angeles and 
america/los_angeles should be considered the same.

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] Timezones (in 8.5?)

2009-11-29 Thread Pavel Stehule
2009/11/29 David E. Wheeler da...@kineticode.com:
 On Nov 28, 2009, at 5:40 PM, Bruce Momjian wrote:

 I think there is general agreement that we should have a timezone data
 type which validates against pg_timezone_names().name.  It might be
 enough to just document how users can create such a domain data type,
 but I don't know of a way to do that.  Is this a TODO?

 From 
 http://justatheory.com/computers/databases/postgresql/citext-patch-submitted.html

 CREATE OR REPLACE FUNCTION is_timezone( tz TEXT ) RETURNS BOOLEAN as $$
 BEGIN
  PERFORM now() AT TIME ZONE tz;
  RETURN TRUE;
 EXCEPTION WHEN invalid_parameter_value THEN
  RETURN FALSE;
 END;
 $$ language plpgsql STABLE;

 CREATE DOMAIN timezone AS CITEXT
 CHECK ( is_timezone( value ) );

 It could also be TEXT I suppose, but America/Los_Angeles and 
 america/los_angeles should be considered the same.

nice :)

Pavel


 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


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


[HACKERS] Patch: Remove gcc dependency in definition of inline functions

2009-11-29 Thread Kurt Harriman

Hi,

The attached patch is offered for the 2010-01 commitfest.
It's also available in my git repository in the submitted branch:
  
http://git.postgresql.org/gitweb?p=users/harriman/share.git;a=shortlog;h=refs/heads/submitted

In palloc.h and pg_list.h, some inline functions are defined if
allowed by the compiler.  Previously the test was gcc-specific.
This patch enables inlining on more platforms, relying on the
Autoconf-generated configure script to determine whether
the compiler supports inline functions.

Depending on the compiler, the keyword for defining an inline
function might be spelled as inline, __inline, or __inline__,
or none of these.  configure finds out, and except in the
first case, puts a suitable #define inline into pg_config.h.
This hasn't changed.

What's new is that configure will add #define HAVE_INLINE 1
into pg_config.h if it detects that the compiler accepts inline
function definitions.  palloc.h and pg_list.h will condition
their inline function definitions on HAVE_INLINE instead of
the gcc-specific __GNUC__.

After applying the patch, run these commands to update
the configure script and pg_config.h.in:
autoconf; autoheader

(Does anybody still use a C compiler that doesn't support
inline functions?)

Regards,
... kurt


Remove gcc dependency in definition of inline functions.

In palloc.h and pg_list.h, some inline functions are defined if
allowed by the compiler.  Previously the test was gcc-specific.
Now the test is platform independent, relying on the Autoconf-
generated configure script to determine whether the compiler
supports inline functions.

Depending on the compiler, the keyword for defining an inline
function might be spelled as inline, __inline, or __inline__,
or none of these.  configure finds out, and except in the
first case, puts a suitable #define inline into pg_config.h.
This hasn't changed.

What's new is that configure now adds #define HAVE_INLINE 1
into pg_config.h upon finding that the compiler accepts inline
function definitions.  palloc.h and pg_list.h now condition
their inline function definitions on HAVE_INLINE instead of
the gcc-specific __GNUC__.

---
 configure.in  |4 
 src/backend/nodes/list.c  |   10 --
 src/backend/utils/mmgr/mcxt.c |9 -
 src/include/nodes/pg_list.h   |4 ++--
 src/include/utils/palloc.h|8 
 5 files changed, 18 insertions(+), 17 deletions(-)
--- Kurt Harriman harri...@acm.org 2009-11-29 08:22:05 -0800

diff --git a/configure.in b/configure.in
index 612e843..467f40d 100644
--- a/configure.in
+++ b/configure.in
@@ -1105,6 +1105,10 @@ PGAC_STRUCT_SOCKADDR_STORAGE
 PGAC_STRUCT_SOCKADDR_STORAGE_MEMBERS
 PGAC_STRUCT_ADDRINFO
 
+if test $ac_cv_c_inline != no; then
+  AC_DEFINE(HAVE_INLINE, 1, [Define to 1 if inline functions are allowed in C])
+fi
+
 AC_CHECK_TYPES([struct cmsgcred, struct fcred, struct sockcred], [], [],
 [#include sys/param.h
 #include sys/types.h
diff --git a/src/backend/nodes/list.c b/src/backend/nodes/list.c
index 1ba85f4..66fa3d6 100644
--- a/src/backend/nodes/list.c
+++ b/src/backend/nodes/list.c
@@ -1224,12 +1224,10 @@ list_copy_tail(List *oldlist, int nskip)
 }
 
 /*
- * When using non-GCC compilers, we can't define these as inline
- * functions in pg_list.h, so they are defined here.
- *
- * TODO: investigate supporting inlining for some non-GCC compilers.
+ * pg_list.h defines inline versions of this function if allowed by the 
+ * compiler; in which case the definitions below are skipped.
  */
-#ifndef __GNUC__
+#ifndef HAVE_INLINE
 
 ListCell *
 list_head(List *l)
@@ -1248,7 +1246,7 @@ list_length(List *l)
 {
return l ? l-length : 0;
 }
-#endif   /* ! __GNUC__ */
+#endif   /* ! HAVE_INLINE */
 
 /*
  * Temporary compatibility functions
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 4939046..2c5ddbb 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -628,11 +628,10 @@ repalloc(void *pointer, Size size)
  * MemoryContextSwitchTo
  * Returns the current context; installs the given context.
  *
- * This is inlined when using GCC.
- *
- * TODO: investigate supporting inlining for some non-GCC compilers.
+ * palloc.h defines an inline version of this function if allowed by the 
+ * compiler; in which case the definition below is skipped.
  */
-#ifndef __GNUC__
+#ifndef HAVE_INLINE
 
 MemoryContext
 MemoryContextSwitchTo(MemoryContext context)
@@ -645,7 +644,7 @@ MemoryContextSwitchTo(MemoryContext context)
CurrentMemoryContext = context;
return old;
 }
-#endif   /* ! __GNUC__ */
+#endif   /* ! HAVE_INLINE */
 
 /*
  * MemoryContextStrdup
diff --git a/src/include/nodes/pg_list.h b/src/include/nodes/pg_list.h
index 04da862..4e3e08d 100644
--- a/src/include/nodes/pg_list.h
+++ b/src/include/nodes/pg_list.h
@@ -74,7 +74,7 @@ struct ListCell
  * arguments. Therefore, we implement them using GCC inline functions,
  * and as regular 

Re: [HACKERS] draft RFC: concept for partial, wal-based replication

2009-11-29 Thread Boszormenyi Zoltan
Hi,

we tried to discuss on a lower level what should be needed
for a partial replication based on streaming replication.

a. transferring base data after a slave got added a relation/index/...
(and initial setup)
b. communicating the the slaves which relations they currently should
have available
c. splitting wal into different replication sets
d. some configuration frontend. Possibly directly via sql or via functions

I don't think its reasonable trying to discuss and implement this all
in one huge patch so I propose implementing at least 1) as a seperate
patch.

a) is very useful outside the context of this specific feature and
kind of a requisite so i suggest tackling that first.

Plan:
M: checkpoint, full page writes, access share lock on the relation
S: stop receiving wal
SM: Using $method transfer required base data for every required segment/fork
M: normal writes
S: restart wal replay

Do you see any fundamental problem with this?

Now there unforatunately are two different schools of thought how to implement 
transfering base data.
1. archive_command like transfer command taking a source path/host and target 
path/host
   - very flexible (think e.g. not transferring the data for multiple slaves 
over the whole country)
   - harder to setup
   - more in style of classical wal archiving
2. add the capability to the WAL Streaming patch's libpq based protocol
   - no additional configuration needed
   - inflexible
   - makes usage from non streaming replication is impossible

I favor 1. but only lightly so.

Detail Questions:
- How to deal with multiple transfer requests at the same time?
  There would be a need for multiple full backup requests for
  individual tables by several clients at once.
  Currently pg_start_backup() isn't allowed from
  two clients in parallel, the second one gets an error.
  We thought that pg_start_backup() and pg_stop_backup()
  can turn into simple reference counts. IIRC, WALs
  are still generated and _shipped to slaves_ during
  a full backup, they are simply not yet applied to
  base table files. So, in this case a pg_stop_backup()
  issued from a slave decreases refcount of the base backups
  and the slave can simply resume applying its newly
  received WALs to base files.
- Keep track of current number of transfers
  We would also need a way to query the refcount of the
  base backup, so if a slave dies, the master can be
  recovered manually, so it can also resume applying leftover WALs.

b) The slave needs to know whether a relation got added to itself in
order to request a base backup of the relevant files.

I would suggest adding a new wal record type for this:
  - DROP_RELATION_SLAVE(node_id, relation)
  - ADD_RELATION_SLAVE(node_id, relation)

We thought about two ways of administering the replication set:
- slaves with full replication, optionally and explicitely excluding
relations
- slaves with minimal replication, explicitely included relations
The above WAL record types will be needed both cases,
but we also need two new catalog tables:
- the slave nodes, indicating the type of the slave above
- explicit table indications (treated depending on the type of the slave)

Any out of band communication has severe problems with
crashes/unavailability of the slave or not allowing classic, non
streaming, wal replication.

Questions:
- How to deal with the fact that the slave may be unavailable during adding 
something to
  its replication set?
  - Possibly forbid all DDL to the table until the slave got the update

c)
What to filter:
Every slave gets a node_id which is assigned in a system catalog on the master
pg_node(nodeoid) (per cluster)
And a catalog contains all replicated relations.
pg_replication_set(nodeoid, classoid, acknowledged_on_slave) (per database)

How to filter:
Heap2, Heap, Btree, Gin, Gist, Storage, Sequence need to be filtered by 
database/relation.
I am by far not yet familiar enough with the relevant code to see if it is 
feasible and worthwile at all to filter clog, transaction and multixact per 
database.

Where to filter:
I propose doing so in the walsender. While this would prohibit using
classical wal based standby I do not see a big problem in that.

If done via wal streaming it would be a simple addition of a node_id in 
PQstartXLogStreaming. This id obviously should not be resettable to something 
else...

Questions:
- How to deal with access to the different database-wide catalogs?
  - Storing that data cluster wide seems really ugly.
  - read the code...

d) I do not have any strong or even moderate opinions about this. I think its 
sensible to get something prototypish, function based done before deciding 
about the real interface and getting into syntax wars.


Steps:

1:
- Transfer of Relations during runtime
  - needed to use wal-splitting
  - internally:
- Possibility 1:
  - transfer_command = ... %filename%
  - should not error out if data changes beneath it.
  - called for every file (i.e. 

Re: [HACKERS] Application name patch - v4

2009-11-29 Thread Dimitri Fontaine
Hi,

Le 29 nov. 2009 à 18:22, Tom Lane a écrit :
 I think we should use GUC_NO_RESET_ALL.
 
 I agree with you, but it seems we have at least as many votes to not do
 that.  Any other votes out there?

Driven by the pooler use case (pgbouncer, even), I'd say RESET ALL should reset 
also the application name. And the connection value is not tied any more to 
something sensible as soon as you have pooling in there...

Regards,
-- 
dim

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


Re: [HACKERS] cvs chapters in our docs

2009-11-29 Thread Brendan Jurd
2009/11/29 Bruce Momjian br...@momjian.us:
 Wow, we mention 28k modems --- we are legacy software:  ;-)

     This initial checkout is a little slower than simply downloading
     a filenametar.gz/filename file; expect it to take 40 minutes
     or so if you have a 28.8K modem.

Yes, and what about all the people using carrier pidgeon to download
Postgres?  I think our documentation is neglecting this substantial
and vital portion of our user community.

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] Patch: Remove gcc dependency in definition of inline functions

2009-11-29 Thread Tom Lane
Kurt Harriman harri...@acm.org writes:
 (Does anybody still use a C compiler that doesn't support
 inline functions?)

The question isn't so much that, it's whether the compiler supports
inline functions with the same behavior as gcc.  At minimum that
would require
* not generating extra copies of the function
* not whining about unreferenced static functions
How many compilers have you tested this patch against?  Which ones
does it actually offer any benefit for?

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] plperl and inline functions -- first draft

2009-11-29 Thread Joshua Tolley
On Sat, Nov 28, 2009 at 10:15:40PM -0500, Tom Lane wrote:
 Joshua Tolley eggyk...@gmail.com writes:
  Makes sense on both counts. Thanks for the help. How does the attached look?
 
 Applied with minor corrections, mainly around the state save/restore
 logic.  I also put in some code to fix the memory leak noted by Tim Bunce,
 but am waiting for some confirmation that it's right before
 back-patching the pre-existing bug of the same ilk.
 
   regards, tom lane

Yay, and thanks. For the record, I'm can't claim to know whether your fix is
the Right Thing or not, so I'm witholding comment.

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


signature.asc
Description: Digital signature


Re: [HACKERS] Writeable CTE patch

2009-11-29 Thread Alex Hunsaker
On Sat, Nov 28, 2009 at 11:59, Tom Lane t...@sss.pgh.pa.us wrote:
 1. I thought we'd agreed at
 http://archives.postgresql.org/pgsql-hackers/2009-10/msg00558.php
 that the patch should support WITH on DML statements, eg
        with (some-query) insert into foo ...
 This might not take much more than grammar additions, but it's
 definitely lacking at the moment.

Hrm ? A few messages down you say SELECT should be a good start

http://archives.postgresql.org/pgsql-hackers/2009-10/msg01081.php

 2. The handling of rules on DML WITH queries is far short of sufficient.

Ick.

 Perhaps it would be acceptable to just throw ERROR_FEATURE_NOT_SUPPORTED
 when there are DO ALSO or conditional DO INSTEAD rules applying to the
 target of a DML WITH query.

+1

 3. I'm pretty unimpressed with the code added to ExecutePlan.
 I wonder whether it would be practical to fix both #2 and #3 by having the
 representation of DML WITH queries look more like the representation of
 rule rewrite output

Interesting...  This seems like the best solution ( assuming its
workable ).  It also looks like it might make #1 easier as well.

However, I think the current approach does have some virtue in that I
was surprised how little the patch was.  Granted that is partly due to
ExecutePlan knowing to much.

-- 
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] Application name patch - v4

2009-11-29 Thread Tom Lane
Dimitri Fontaine dfonta...@hi-media.com writes:
 Le 29 nov. 2009 à 18:22, Tom Lane a écrit :
 I think we should use GUC_NO_RESET_ALL.
 
 I agree with you, but it seems we have at least as many votes to not do
 that.  Any other votes out there?

 Driven by the pooler use case (pgbouncer, even), I'd say RESET ALL should 
 reset also the application name. And the connection value is not tied any 
 more to something sensible as soon as you have pooling in there...

The thing is that the libpq API treats application_name as a *property
of the connection*.  You shouldn't expect it to go away on RESET ALL,
any more than you'd expect RESET ALL to cause you to be reconnected to
some other database.

If a pooler wants application_name to be cleared when it issues RESET
ALL, I think it ought to be setting the name via SET, not via the libpq
connection option.

But it's certainly true that using GUC_NO_RESET_ALL would be a quick
kluge rather than a proper solution.  Andres Freund suggested upthread
that we should fix this by extending SET:

: One possibility would be to make it possible to issue SETs that behave
: as if set in a startup packet - imho its an implementation detail that
: SET currently is used.

I think there's a good deal of merit in this, and it would't be hard at
all to implement, seeing that we already have SET LOCAL and SET SESSION.
We could add a third keyword, say SET DEFAULT, that would have the
behavior of setting the value in a fashion that would persist across
resets.  I'm not sure that DEFAULT is exactly le mot juste here, but
agreeing on a keyword would probably be the hardest part of making it
happen.

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] Application name patch - v4

2009-11-29 Thread Florian G. Pflug

Tom Lane wrote:

: One possibility would be to make it possible to issue SETs that
behave : as if set in a startup packet - imho its an implementation
detail that : SET currently is used.

I think there's a good deal of merit in this, and it would't be hard
at all to implement, seeing that we already have SET LOCAL and SET
SESSION. We could add a third keyword, say SET DEFAULT, that would
have the behavior of setting the value in a fashion that would
persist across resets.  I'm not sure that DEFAULT is exactly le mot
juste here, but agreeing on a keyword would probably be the hardest
part of making it happen.


Hm, but without a way to prevent the users of a connection pool from
issuing SET DEFAULT, that leaves a connection pool with no way to
revert a connection to a known state.

How about SET CONNECTION, with an additional GUC called
connection_setup which can only be set to true, never back to false.
Once connection_setup is set to true, further SET CONNECTION attempts
would fail.

In a way, this mimics startup-packet SETs without actually doing things
in the startup packet.

best regards,
Florian Pflug



smime.p7s
Description: S/MIME Cryptographic Signature


Re: [HACKERS] cvs chapters in our docs

2009-11-29 Thread Ron Mayer
Brendan Jurd wrote:
 2009/11/29 Bruce Momjian br...@momjian.us:
 Wow, we mention 28k modems --- we are legacy software:  ;-)

 This initial checkout is a little slower than simply downloading
 a filenametar.gz/filename file; expect it to take 40 minutes
 or so if you have a 28.8K modem.
 
 Yes, and what about all the people using carrier pidgeon to download
 Postgres?  I think our documentation is neglecting this substantial
 and vital portion of our user community.

Never underestimate the bandwidth of a carrier pigeon with a flash
card tied to his leg. [1]

   11-month-old bird armed with a 4GB memory stick... the carrier pigeon
   delivered 4GB of data 60 miles in a little over an hour


[1] 
http://www.dslreports.com/shownews/Carrier-Pigeon-Officially-Beats-DSL-104393


-- 
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] Application name patch - v4

2009-11-29 Thread Tom Lane
Fujii Masao masao.fu...@gmail.com writes:
 Why doesn't application_name appear in postgresql.conf.sample?
 That is expected to be set from only libpq?

It would seem pretty silly to set it in the conf file.  You *can*,
if you want, but I see no reason to list it there.

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] Application name patch - v4

2009-11-29 Thread Fujii Masao
On Mon, Nov 30, 2009 at 10:20 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Fujii Masao masao.fu...@gmail.com writes:
 Why doesn't application_name appear in postgresql.conf.sample?
 That is expected to be set from only libpq?

 It would seem pretty silly to set it in the conf file.  You *can*,
 if you want, but I see no reason to list it there.

Yeah, I see your point. But, is it a policy not to put such parameter
(other than that for debug) on postgresql.conf.sample?

Regards,

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

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


Re: [HACKERS] [PATCH 4/4] Add tests to dblink covering use of COPY TO FUNCTION

2009-11-29 Thread Jeff Davis
On Fri, 2009-11-27 at 20:28 -0500, Greg Smith wrote:
 In the context of the read case, I'm not as sure it's so black and 
 white.  While the current situation does map better to a function that 
 produces a stream of bytes, that's not necessarily the optimal approach 
 for all situations.  It's easy to imagine a function intended for 
 accelerating bulk loading that is internally going to produce a stream 
 of already processed records. 

The binary COPY mode is one of the closest things I can think of to
already-processed records. Is binary COPY slow? If so, the only thing
faster would have to be machine-specific, I think.

 I think there's a very valid use-case for both approaches.
...
 COPY target FROM FUNCTION foo() WITH RECORDS;

In what format would the records be?

Also, this still doesn't really answer why INSERT ... SELECT isn't good
enough. If the records really are in their internal format, then
INSERT ... SELECT seems like the way to go.

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] [PATCH 4/4] Add tests to dblink covering use of COPY TO FUNCTION

2009-11-29 Thread Jeff Davis
On Thu, 2009-11-26 at 18:30 -0800, Daniel Farina wrote:
 Okay, so this thread sort of wandered into how we could refactor other
 elements of COPY.  Do we have a good sense on what we should do to the
 current patch (or at least the idea represented by it) to get it into
 a committable state within finite time?

We're in the middle of a commitfest, so a lot of hackers are
concentrating on other patches. In a week or two, when it winds down,
people will be more willing to make decisions on new proposals and
designs. I still think this thread has been productive.

 I think adding a bytea and/or text mode is once such improvement...I
 am still reluctant to give up on INTERNAL because the string buffer
 passed in the INTERNAL scenario is ideal for C programmers -- the
 interface is even simpler than dealing with varlena types.  But I
 agree that auxiliary modes should exist to enable easier hacking.

I like the idea of an internal mode as well. We may need some kind of
performance numbers to justify avoiding the extra memcpy, though.

 The thorniest issue in my mind is how state can be initialized
 retained and/or modified between calls to the bytestream-acceptance
 function.
 
 Arguably it is already in a state where it is no worse than dblink,
 which itself has a global hash table to manage state.

The idea of using a separate type of object (e.g. CREATE COPYSTREAM)
to bundle the init/read/write/end functions together might work. That
also allows room to specify what the functions should accept
(internal/bytea/text).

I think that's the most elegant solution (at least it sounds good to
me), but others might not like the idea of a new object type just for
this feature. Perhaps if it fits nicely within an overall SQL/MED-like
infrastructure, it will be easier to justify.

 Also, if you look carefully at the dblink test suite I submitted,
 you'll see an interesting trick: one can COPY from multiple sources
 consecutively to a single COPY on a remote node when in text mode
 (binary mode has a header that cannot be so neatly catenated).  This
 is something that's pretty hard to enable with any automatic
 startup-work-cleanup approach.

What if the network buffer is flushed in the middle of a line? Is that
possible, or is there a guard against that somewhere?

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] [PATCH 4/4] Add tests to dblink covering use of COPY TO FUNCTION

2009-11-29 Thread Daniel Farina
On Sun, Nov 29, 2009 at 6:35 PM, Jeff Davis pg...@j-davis.com wrote:
 What if the network buffer is flushed in the middle of a line? Is that
 possible, or is there a guard against that somewhere?

What do you mean?  They both catenate onto one stream of bytes, it
shouldn't matter where the flush boundaries are...

It so happens as a convenient property of the textual modes is that
adding more payload is purely concatenative (not true for binary,
where there's a header that would cause confusion to the receiving
side)

fdr

-- 
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] draft RFC: concept for partial, wal-based replication

2009-11-29 Thread Itagaki Takahiro

Boszormenyi Zoltan z...@cybertec.at wrote:

 we tried to discuss on a lower level what should be needed
 for a partial replication based on streaming replication.

We need to discuss a partial recovery before the partial replication.

There are some related items in out ToDo list and previous discussions:

http://wiki.postgresql.org/wiki/Todo
- Allow WAL logging to be turned off for a table,
  but the table might be dropped or truncated during crash recovery
- Allow WAL logging to be turned off for a table,
  but the table would avoid being truncated/dropped

- rmgr_hook
http://archives.postgresql.org/pgsql-hackers/2008-12/msg01361.php

Could you try them first as a preparation for the partial replication?

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



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


Re: [HACKERS] [PATCH 4/4] Add tests to dblink covering use of COPY TO FUNCTION

2009-11-29 Thread Jeff Davis
On Sun, 2009-11-29 at 18:53 -0800, Daniel Farina wrote:
 On Sun, Nov 29, 2009 at 6:35 PM, Jeff Davis pg...@j-davis.com wrote:
  What if the network buffer is flushed in the middle of a line? Is that
  possible, or is there a guard against that somewhere?
 
 What do you mean?  They both catenate onto one stream of bytes, it
 shouldn't matter where the flush boundaries are...

Nevermind, for some reason I thought you were talking about interleaving
the data rather than concatenating.

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] draft RFC: concept for partial, wal-based replication

2009-11-29 Thread Fujii Masao
On Mon, Nov 30, 2009 at 4:56 AM, Boszormenyi Zoltan z...@cybertec.at wrote:
 I don't think its reasonable trying to discuss and implement this all
 in one huge patch so I propose implementing at least 1) as a seperate
 patch.

I agree with this development plan.

 Now there unforatunately are two different schools of thought how to 
 implement transfering base data.
 1. archive_command like transfer command taking a source path/host and target 
 path/host
   - very flexible (think e.g. not transferring the data for multiple slaves 
 over the whole country)
   - harder to setup
   - more in style of classical wal archiving
 2. add the capability to the WAL Streaming patch's libpq based protocol
   - no additional configuration needed
   - inflexible
   - makes usage from non streaming replication is impossible

 I favor 1. but only lightly so.

I favor 2 ;) Because I think that it's too hard for users to set up
a transfer command. One of streaming replication's merits is that
users no longer need to specify a transfer command for log-shipping.
So users can configure and use replication without complex settings.
But, #1 would spoil this merit.

 Detail Questions:
 - How to deal with multiple transfer requests at the same time?
  There would be a need for multiple full backup requests for
  individual tables by several clients at once.
  Currently pg_start_backup() isn't allowed from
  two clients in parallel, the second one gets an error.
  We thought that pg_start_backup() and pg_stop_backup()
  can turn into simple reference counts. IIRC, WALs
  are still generated and _shipped to slaves_ during
  a full backup, they are simply not yet applied to
  base table files. So, in this case a pg_stop_backup()
  issued from a slave decreases refcount of the base backups
  and the slave can simply resume applying its newly
  received WALs to base files.

I'm not sure how. But at first multiple online-backup feature
rather than backup-shipping itself might have to be addressed.

Regards,

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

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


Re: [HACKERS] Application name patch - v4

2009-11-29 Thread Fujii Masao
On Mon, Nov 30, 2009 at 11:21 AM, Fujii Masao masao.fu...@gmail.com wrote:
 On Mon, Nov 30, 2009 at 10:20 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Fujii Masao masao.fu...@gmail.com writes:
 Why doesn't application_name appear in postgresql.conf.sample?
 That is expected to be set from only libpq?

 It would seem pretty silly to set it in the conf file.  You *can*,
 if you want, but I see no reason to list it there.

 Yeah, I see your point. But, is it a policy not to put such parameter
 (other than that for debug) on postgresql.conf.sample?

Ooops! I missed GUC_NOT_IN_SAMPLE parameters. Sorry for noise.

Regards,

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

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


Re: [HACKERS] Add YAML option to explain

2009-11-29 Thread Itagaki Takahiro

Itagaki Takahiro itagaki.takah...@oss.ntt.co.jp wrote:

 My rewrite is relatively large. Please reversely-review the patch.

I rethink the code cleanup should be done with another patch even if needed.
Here is a lite version of yaml YAML explan patch.

All of the logic for indent is done in ExplainYAMLLineStarting().
It can remove undesirable linebreaks from the item list and end of output.

Please check whether the v3.1 patch works as your expectation.
If ok, we can move it to Ready for Committer.

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



yaml.explain.v3.1.patch
Description: Binary data

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


Re: [HACKERS] Listen / Notify - what to do when the queue is full

2009-11-29 Thread Jeff Davis
On Fri, 2009-11-20 at 10:35 +0100, Joachim Wieland wrote:
 On Thu, Nov 19, 2009 at 11:04 PM, Joachim Wieland j...@mcknight.de wrote:
  Given your example, what I am proposing now is to stop reading from
  the queue once we see a not-yet-committed notification but once the
  queue is full, read the uncommitted notifications, effectively copying
  them over into the backend's own memory... Once the transaction
  commits and sends a signal, we can process, send and discard the
  previously copied notifications. In the above example, at some point
  one, two or all three backends would see that the queue is full and
  everybody would read the uncommitted notifications of the other one,
  copy them into the own memory and space will be freed in the queue.
 
 Attached is the patch that implements the described modifications.

This is a first-pass review.

Comments:

 * Why don't we read all notifications into backend-local memory at
every opportunity? It looks like sometimes it's only reading the
committed ones, and I don't see the advantage of leaving it in the SLRU.

Code comments:

 * I see a compiler warning:
ipci.c: In function ‘CreateSharedMemoryAndSemaphores’:
ipci.c:222: warning: implicit declaration of function ‘AsyncShmemInit’
 * sanity_check test fails, needs updating
 * guc test fails, needs updating
 * no docs

The overall design looks pretty good to me. From my very brief pass over
the code, it looks like:
 * When the queue is full, the transaction waits until there is room
before it's committed. This may have an effect on 2PC, but the consensus
seemed to be that we could restrict the combination of 2PC + NOTIFY.
This also introduces the possibility of starvation, I suppose, but that
seems remote.
 * When the queue is full, the inserter tries to signal the listening
backends, and tries to make room in the queue.
 * Backends read the notifications when signaled, or when inserting (in
case the inserting backend is also the one preventing the queue from
shrinking).

I haven't looked at everything yet, but this seems like it's in
reasonable shape from a high level. Joachim, can you clean the patch up,
include docs, and fix the tests? If so, I'll do a full review.

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] New VACUUM FULL

2009-11-29 Thread Itagaki Takahiro
Thanks for review!

Jeff Davis pg...@j-davis.com wrote:

  * VACUUM (FULL REPLACE) pg_class should be rejected, not silently
 turned into VACUUM (FULL INPLACE) pg_class.

Hmmm, it requires to remember whether REPLACE is specified or not
for the non-INPLACE vacuum, but I don't want to add VACOPT_REPLACE
only for the purpose.

I just removed FULL REPLACE syntax; Only FULL and FULL INPLACE are
available. VACUUM FULL without INPLACE behaves as cluster-like rewrites
for non-system tables. FULL INPLACE is a traditional vacuum full.
System catalogs are always vacuumed with INPLACE version.
  - VACUUM FULL / VACUUM (FULL) = rewritten version
  - VACUUM (FULL INPLACE)   = traditional version

  * In vacuumdb, the code for handling INPLACE is a little ugly. Perhaps
 it should be changed to always use your new options syntax? That might
 be more code, but it would be a little more readable.

It might make the code cleaner, but I want vacuumdb in 8.5 to work on older
versions of servers unless we use the new feature. Older servers can only
accept older syntax, so I avoided using the new syntax if not needed.
(The new patch still uses two versions of syntax.)

  * The patch should be merged with CVS HEAD. The changes required are
 minor; but notice that there is a minor style difference in the assert
 in vacuum().
  * vacuumdb should reject -i without -f
  * The replace or inplace option is a magical default, because VACUUM
 FULL defaults to replace for user tables and inplace for system
 tables. I tried to make that more clear in my documentation suggestions.
  * There are some windows line endings in the patch, which should be
 removed.

Done.

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



vacuum-full_20091130.patch
Description: Binary data

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


Re: [HACKERS] SE-PgSQL patch review

2009-11-29 Thread KaiGai Kohei
Itagaki Takahiro wrote:
 KaiGai Kohei kai...@kaigai.gr.jp wrote:
   -- keep it smaller, and step-by-step enhancement
 
 I'd prefer smaller concept rather than smaller patch.

For the last a few days, I've talked with Itagaki-san off list to make clear
where is the point of his suggestion.

In summary, it was similar approach with what I already proposed in the CF#2,
but rejected.

During the first commit-fest of v8.5 development cycle, Stephen Frost suggested
to rework the default PG's access controls to host other optional security
features, not only the default one.
Then, I submitted a large patch titled as Reworks for Access Controls,
but it contained 3.5KL of changeset on the core routines, and 4KL of new codes
into src/backend/security/* except for documentations and testcases.
Then, this approach was rejected (not returned with feedback) due to the scale
and complexity.

After the fest, we discussed the direction to implement SE-PgSQL.
Basically, it needs to keep the changeset small, and the rest of features (such
as row-level granurality, access control decision cache, ...) shoule be added
step-by-step consistently, according to the suggestion in the v8.4 development
cycle. Heikki Linnakangas also suggested we need developer documentation which
introduces SE-PgSQL compliant permission checks and specification of security
hooks, after the reworks are rejected.

So, I boldly removed most of the features from SE-PgSQL except for its core
functionalities, and added developer documentation (README) and widespread
source code comments to introduce the implementations instead.
In the result, the current proposal is near to naked one.
 - No access controls except for database, schema, table and column.
 - No row-level granularity in access controls.
 - No access control decision chache.
 - No security OID within HeapTupleHeader.

I believe the current patch is designed according to the past suggestions.
However, his suggestion seems to me backing to the rejected approach again.

I've been torn between the past suggestions and his suggestion.
So, I asked him to get off reviewing the patch, because of the deadlock in
the development process. At the current moment, I'd like to respect
suggestions in the past discussions more.

Thanks for paying your efforts in spite of differences in opinions.
-- 
OSS Platform Development Division, NEC
KaiGai Kohei kai...@ak.jp.nec.com

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


Re: [HACKERS] draft RFC: concept for partial, wal-based replication

2009-11-29 Thread Craig Ringer
Boszormenyi Zoltan wrote:

 c. splitting wal into different replication sets

Just a side note: in addition to its use for partial replication, this
might have potential for performance-prioritizing databases or tablespaces.

Being able to separate WAL logging so that different DBs, tablespaces,
etc went to different sets of WAL logs would allow a DBA to give some
databases or tablespaces dedicated WAL logging space on faster storage.
If partial recovery is implemented, it might also permit less important
databases to be logged to fast-but-unsafe storage such as a non-BBU disk
controller with write cache enabled, without putting more important
databases in the same cluster in danger.

More importantly, if the WAL writing was done in different wal writer
backends, the admin could also use nice and ionice to encourage the OS
to favour WAL logging for some DBs over others.

Currently all these things require splitting the install into multiple
clusters, incurring config management and backup overhead and most
importantly partitioning shared memory.

OTOH, even with split WAL logging, you still have the shared bgwriter to
contend with, and the effects of an unimportant query pushing data
related to more performance-critical DBs out of shm or OS cache. So
perhaps splitting the cluster is actually the best answer, and a
complete implementation of DB prioritization would land up looking a lot
like multiple Pg clusters multiplexed on one port anyway...

In any case, I thought it worth mentioning as something that may be
worth keeping in mind - or considering and disregarding - while looking
at the WAL changes involved in partial replication.

--
Craig Ringer

-- 
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] Application name patch - v4

2009-11-29 Thread Andres Freund
Hi,

On Monday 30 November 2009 01:16:43 Florian G. Pflug wrote:
 Tom Lane wrote:
  : One possibility would be to make it possible to issue SETs that
 
  behave : as if set in a startup packet - imho its an implementation
  detail that : SET currently is used.
 
  I think there's a good deal of merit in this, and it would't be hard
  at all to implement, seeing that we already have SET LOCAL and SET
  SESSION. We could add a third keyword, say SET DEFAULT, that would
  have the behavior of setting the value in a fashion that would
  persist across resets.  I'm not sure that DEFAULT is exactly le mot
  juste here, but agreeing on a keyword would probably be the hardest
  part of making it happen.
 Hm, but without a way to prevent the users of a connection pool from
 issuing SET DEFAULT, that leaves a connection pool with no way to
 revert a connection to a known state.
Perhaps we should only allow a few parameters to be SET as a connection 
default - then the pooler would have to issue those just as it has to do for 
actual connection defaults.

 How about SET CONNECTION, with an additional GUC called
 connection_setup which can only be set to true, never back to false.
 Once connection_setup is set to true, further SET CONNECTION attempts
 would fail.
How would that help the pooler case? The next connection to it might be from a 
different application.

Andres


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


Re: [HACKERS] ProcessUtility_hook

2009-11-29 Thread Itagaki Takahiro

Euler Taveira de Oliveira eu...@timbira.com wrote:

 The functionality is divided in two parts. The first part is a hook in the
 utility module. The idea is capture the commands that doesn't pass through
 executor. I'm afraid that that hook will be used only for capturing non-DML
 queries. If so, why don't we hack the tcop/postgres.c and grab those queries
 from the same point we log statements?

DDLs can be used from user defined functions. It has the same reason
why we have executor hooks instead of tcop hooks.


 The second part is to use that hook to capture non-DML commands for
 pg_stat_statements module.

- I fixed a bug that it should handle only isTopLevel command.
- A new parameter pg_stat_statements.track_ddl (boolean) is
  added to enable or disable the feature.

 Do we need to have rows = 0 for non-DML commands?
 Maybe NULL could be an appropriate value.

Yes, but it requires additional management to handle 0 and NULL
separately. I don't think it is worth doing.

 The PREPARE command stopped to count
  the number of rows. Should we count the rows in EXECUTE command or in the
 PREPARE command?

It requires major rewrites of EXECUTE command to pass the number of
affected rows to caller. I doubt it is worth fixing because almost all
drivers use protocol-level prepared statements instead of PREPARE+EXECUTE.

 The other command that doesn't count properly is COPY. Could
 you fix that?

I added codes for it.

 I'm concerned about storing some commands that expose passwords
 like CREATE ROLE foo PASSWORD 'secret'; I know that the queries are only
 showed to superusers but maybe we should add this information to documentation
 or provide a mechanism to exclude those commands.

I think there is no additonal problem there because we can see the 'secret'
command using pg_stat_activity even now.


 I don't know if it is worth the trouble adding some code to capture VACUUM and
 ANALYZE commands called inside autovacuum.

I'd like to exclude VACUUM and ANALYZE by autovacuum because
pg_stat_statements should not filled with those commands.


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



ProcessUtility_hook_20091130.patch
Description: Binary data

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


[HACKERS] Fwd: psql+krb5

2009-11-29 Thread rahimeh khodadadi
-- Forwarded message --
From: rahimeh khodadadi rahimeh.khodad...@gmail.com
Date: 2009/11/29
Subject: Re: psql+krb5
To: Denis Feklushkin denis.feklush...@gmail.com


These items have added after my sending.

I repeat again my configurations:


*
1) The configuration of  krb5.conf is:
 [realms]
   EXAMPLE.COM http://example.com/ ={

 kdc=star :88
 admin_server=star:749
 default_domain= example.com
}
.*

2) Then, I created principal as*   postgres/s...@example.com * and its
password is saved in* '/usr/local/pgsql/data/postgresql.keytab' .*


(star is localhost IP, but in hosts.conf I configure like: 213.233.169.93
star)

3) I setup *postgresql.conf *as below:

krb_server_keyfile = '/usr/local/pgsql/data/
postgresql.keytab'
krb_srvname = 'postgres/s...@example.com'

krb_server_hostname = 'star' # empty string matches any keytab entry
krb_caseins_users = off

4) I *create user frank*  in Psql .

5) Then I set up* hba.conf :*

hostall all 0.0.0.0/0  krb5
hostall all 127.0.0.1/32   krb5


When I want to connect to Postgresql, it gives error.

# *kinit frank*

[r...@star bin]# *./psql -h star  -U frank  -d test*

psql: *krb5_sendauth: Bad application version was sent (via sendauth)*

I should mention that * both postgresql server and krb-server are in same
system* and* my IP is acquring from dhcp server  of university*.  Where is
wrong.

2009/11/29 Denis Feklushkin denis.feklush...@gmail.com

 On Sun, 29 Nov 2009 14:23:52 +0330
 rahimeh khodadadi rahimeh.khodad...@gmail.com wrote:

  Thanks for your replying. My detail of configuration is:
 
  I try to setup kerberos authentication in Postgresql 8.1.18 on centos.
 
  But I have some problem.
 
  1) The configuration of  krb5.conf is:
   [realms]
EXAMPLE.COM http://example.com/http://EXAMPLE.COM
  http://example.com/ ={
 
  kdc=star :88
  admin_server=star:749
  default_domain= example.comhttp://example.com
  
   
   }
   .
  
   2) Then, I created principal as   postgres/s...@example.commailto:
   s...@example.com  and its password is saved in
   '/usr/local/pgsql/data/postgresql.keytab' .
  
  
   (star is localhost IP, but in hosts.conf I configure like:
   213.233.169.93 star)
  
   3) I setup postgresql.conf as below:
  
   krb_server_keyfile = '/usr/local/pgsql/data/
   postgresql.keytab'
   krb_srvname = 'postgres/s...@example.commailto:s...@example.com'
  
   krb_server_hostname = 'star' # empty string matches any
   keytab entry
   krb_caseins_users = off
  
   4) I create user frank  in Psql .
  
   5) Then I set up hba.conf :
  
   hostall all 0.0.0.0/0http://0.0.0.0/0
krb5
   hostall all 127.0.0.1/32http://127.0.0.1/32
 krb5
  
  
   When I want to connect to Postgresql, it gives error.
  
   # kinit frank
  
   [r...@star bin]# ./psql -h star  -U frank  -d test
  
   psql: krb5_sendauth: Bad application version was sent (via sendauth)
  
 
  some changes in users gives below error :
  [r...@www bin]# ./psql -h 213.233.168.249  -U postgres
psql: Kerberos 5 authentication rejected:  Wrong principal in
  request
 
 
   I should mention that  both postgresql server and krb-server are in
   same system and my IP is acquring from dhcp server  of university.
   Where is wrong.
  
 
 
 
  2009/11/29 Denis Feklushkin denis.feklush...@gmail.com
 
   On Sun, 29 Nov 2009 10:48:30 +0330
   rahimeh khodadadi rahimeh.khodad...@gmail.com wrote:
  
Hi,
   
When I want to connect to psql via krb5 in Linux, it gives me
error like: [r...@www bin]# ./psql -h 213.233.168.249  -U
postgres psql: Kerberos 5 authentication rejected:  Wrong
principal in request
  
   Что в логах KDC?
  !!!

 И ещё, в тексте который Вы дали встречаются пробелы в именах
 принципалов и странные записи mailto:s...@example.com

 При настройке важно чтобы ничего этого небыло




-- 
With Best Regards
Miss.KHodadadi



-- 
With Best Regards
Miss.KHodadadi


Re: [HACKERS] Patch: Remove gcc dependency in definition of inline functions

2009-11-29 Thread Marko Kreen
On 11/29/09, Tom Lane t...@sss.pgh.pa.us wrote:
 Kurt Harriman harri...@acm.org writes:
   (Does anybody still use a C compiler that doesn't support
   inline functions?)

+1 for modern C.

 The question isn't so much that, it's whether the compiler supports
  inline functions with the same behavior as gcc.  At minimum that
  would require
 * not generating extra copies of the function
 * not whining about unreferenced static functions
  How many compilers have you tested this patch against?  Which ones
  does it actually offer any benefit for?

Those are not correctness problems.  Compilers with non-optimal or
missing 'inline' do belong together with compilers without working int64.
We may spend some effort to be able to compile on them, but they
should not affect our coding style.

'static inline' is superior to macros because of type-safety and
side-effects avoidance.  I'd suggest event removing any HAVE_INLINE
ifdefs and let autoconf undef the 'inline' if needed.  Less duplicated
code to maintain.  The existence of compilers in active use without
working 'inline' seems quite hypothetical these days.

-- 
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] set the cost of an aggregate function

2009-11-29 Thread Jaime Casanova
Hi,

why we can't do $subject? it could have any benefit on the planner?

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

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