Re: [HACKERS] [PATCH] Largeobject access controls

2009-09-28 Thread Jaime Casanova
2009/9/24 KaiGai Kohei kai...@ak.jp.nec.com:
 The attached patch is revised from the previous revision at the following 
 points:

 - The largeobject_compat_acl is renamed to largeobject_check_acl.
  Its default is on, and turning it off means the largeobject stuff
  performs in compatible mode for the v8.4.x or prior releases.
 - Notification messages were eliminated at the compatible mode.
  It always allows to bypass ACL checks without any warnings.


a few minor points:

+ For example, the literallo_import()/literal and
+ literallo_export/literal need superuser privileges independent
+ from this setting, as if the prior version doing.

that should read as prior versions were doing?

and you're still using pg_largeobject_meta in some comments in
src/include/catalog/pg_largeobject_metadata.h

besides that it looks good to me...
i will mark the patch as ready for committer

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


[HACKERS] how to use eclipse when debugging postgreSQL backend

2009-09-28 Thread 노홍찬
Hello hackers,

 

 

I really appreciate Mr. Cecchet’s effort to establish the wiki page
(working_with_eclipse, http://archives.postgresql.org/pgsql-hackers/2008-
10/msg00312.php).

 

I set up my PostgreSQL development environment with Eclipse, following the
page’s instructions.

 

However, I’m stuck to the situation that I cannot debug the modified
backend source of PostgreSQL 

 

since the gdb incorporated with Eclipse doesn’t support the debugging of
the forked child processes.

 

 

When I start debugging process by using the project default, it can only
debug the postmaster process,

 

since the postmaster process forks child processes each of which is
actually postgres backend process responsible for the response to each
client process (psql).

 

What I’m trying to debug is the storage-related part of the backend
source, so the gdb should be able to access the forked processes.

 

I tried several ways like making the default option of gdb to be ‘set
follow-fork-mode child’, but those tries didn’t work.

 

For sure, there is another option, giving up using eclipse when debugging
and using the console-mode gdb, 

 

but I prefer using graphical development environment and that’s the reason
why I chose to use eclipse as a default development tool in the very first
start.

 

(This try might be regarded as silly though, I might be supposed to get
familiar to console development environment,

 

but if someone went through the same situation before and he or she can
help me, it will be very time-saving)

 

 

If someone can let me know how to set Eclipse or several steps needed to
access the forked process while Elipse debugging, 

 

It will be very much helpful to me.

 

Thank you for reading this

 

 

- Best Regards
  Hongchan
  ( mailto:falls...@cs.yonsei.ac.kr falls...@cs.yonsei.ac.kr, (02)2123-
7757) -

 

 



Re: [HACKERS] Lock Wait Statistics (next commitfest)

2009-09-28 Thread Jaime Casanova
On Sat, Aug 8, 2009 at 7:47 PM, Mark Kirkwood mar...@paradise.net.nz wrote:


 Patch with max(wait time).

 Still TODO

 - amalgamate individual transaction lock waits
 - redo (rather ugly) temporary pg_stat_lock_waits in a form more like
 pg_locks

 This version has the individual transaction lock waits amalgamated.

 Still TODO: redo pg_stat_lock_waits ...


it applies with some hunks, compiles fine and seems to work...
i'm still not sure this is what we need, some more comments could be helpful.

what kind of questions are we capable of answer with this and and what
kind of questions are we still missing?

for example, now we know number of locks that had to wait, total
time waiting and max time waiting for a single lock... but still we
can have an inaccurate understanding if we have lots of locks waiting
little time and a few waiting a huge amount of time...

something i have been asked when system starts to slow down is can we
know if there were a lock contention on that period? for now the only
way to answer that is logging locks

about the patch itself:
you haven't documented either. what is the pg_stat_lock_waits view
for? and what are those fieldx it has?

i'll let this patch as needs review for more people to comment on it...

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

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


Re: [HACKERS] GRANT ON ALL IN schema

2009-09-28 Thread Abhijit Menon-Sen
At 2009-09-27 12:54:48 -0400, robertmh...@gmail.com wrote:

 If this patch looks good now, can you mark it Ready for Committer in
 the CommitFest app?

Done.

-- ams

-- 
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] Largeobject access controls

2009-09-28 Thread KaiGai Kohei
Thanks for your comments.

Jaime Casanova wrote:
 2009/9/24 KaiGai Kohei kai...@ak.jp.nec.com:
 The attached patch is revised from the previous revision at the following 
 points:

 - The largeobject_compat_acl is renamed to largeobject_check_acl.
  Its default is on, and turning it off means the largeobject stuff
  performs in compatible mode for the v8.4.x or prior releases.
 - Notification messages were eliminated at the compatible mode.
  It always allows to bypass ACL checks without any warnings.

 
 a few minor points:
 
 + For example, the literallo_import()/literal and
 + literallo_export/literal need superuser privileges independent
 + from this setting, as if the prior version doing.
 
 that should read as prior versions were doing?

Yes.
It seems to me same meanings, but it is unnatural for you, isn't it?

 and you're still using pg_largeobject_meta in some comments in
 src/include/catalog/pg_largeobject_metadata.h

Fixed,

The attached patch is revised based on the comments.

Below is the diffset from the previous revision (r2328).

[kai...@saba ~]$ diff -u r2328.patch r2333.patch
--- r2328.patch 2009-09-28 16:37:19.0 +0900
+++ r2333.patch 2009-09-28 16:36:55.0 +0900
@@ -1,6 +1,6 @@
 diff -Nrpc base/doc/src/sgml/config.sgml blob/doc/src/sgml/config.sgml
 *** base/doc/src/sgml/config.sgml  Thu Sep 24 08:43:31 2009
 blob/doc/src/sgml/config.sgml  Fri Sep 25 09:00:55 2009
+--- blob/doc/src/sgml/config.sgml  Mon Sep 28 16:32:50 2009
 *** dynamic_library_path = 'C:\tools\postgre
 *** 4797,4802 
 --- 4797,4830 
@@ -27,7 +27,7 @@
 + checks corresponding to largeobjects.
 + For example, the literallo_import()/literal and
 + literallo_export/literal need superuser privileges independent
-+ from this setting, as if the prior version doing.
++ from this setting as prior versions were doing.
 +/para
 +para
 + It is literalon/literal by default.
@@ -1990,21 +1990,21 @@
   DECLARE_UNIQUE_INDEX(pg_namespace_oid_index, 2685, on pg_namespace using 
btree(oid oid_ops));
 diff -Nrpc base/src/include/catalog/pg_largeobject_metadata.h 
blob/src/include/catalog/pg_largeobject_metadata.h
 *** base/src/include/catalog/pg_largeobject_metadata.h Thu Jan  1 09:00:00 1970
 blob/src/include/catalog/pg_largeobject_metadata.h Fri Sep 25 09:00:55 2009
+--- blob/src/include/catalog/pg_largeobject_metadata.h Mon Sep 28 16:31:11 2009
 ***
 *** 0 
 --- 1,67 
 + /*-
 +  *
-+  * pg_largeobject_meta.h
-+  * definition of the system largeobject_meta relation 
(pg_largeobject_meta)
++  * pg_largeobject_metadata.h
++  * definition of the system largeobject_metadata relation 
(pg_largeobject_metadata)
 +  * along with the relation's initial contents.
 +  *
 +  *
 +  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
 +  * Portions Copyright (c) 1994, Regents of the University of California
 +  *
-+  * $PostgreSQL: pgsql/src/include/catalog/pg_largeobject_meta.h,v 1.24 
2009/01/01 17:23:57 momjian Exp $
++  * $PostgreSQL: pgsql/src/include/catalog/pg_largeobject_metadata.h,v 1.24 
2009/01/01 17:23:57 momjian Exp $
 +  *
 +  * NOTES
 +  * the genbki.sh script reads this file and generates .bki
@@ -2012,14 +2012,14 @@
 +  *
 +  *-
 +  */
-+ #ifndef PG_LARGEOBJECT_META_H
-+ #define PG_LARGEOBJECT_META_H
++ #ifndef PG_LARGEOBJECT_METADATA_H
++ #define PG_LARGEOBJECT_METADATA_H
 +
 + #include catalog/genbki.h
 +
 + /* 
-+  *   pg_largeobject definition.  cpp turns this into
-+  *   typedef struct FormData_pg_largeobject_meta
++  *   pg_largeobject_metadata definition. cpp turns this into
++  *   typedef struct FormData_pg_largeobject_metadata
 +  * 
 +  */
 + #define LargeObjectMetadataRelationId  2336
@@ -2060,7 +2060,7 @@
 + extern void ac_largeobject_export(Oid loid, const char *filename);
 + extern void ac_largeobject_import(Oid loid, const char *filename);
 +
-+ #endif   /* PG_LARGEOBJECT_META_H */
++ #endif   /* PG_LARGEOBJECT_METADATA_H */
 diff -Nrpc base/src/include/nodes/parsenodes.h 
blob/src/include/nodes/parsenodes.h
 *** base/src/include/nodes/parsenodes.hThu Sep 24 08:43:31 2009
 --- blob/src/include/nodes/parsenodes.hThu Sep 24 09:04:49 2009

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


sepgsql-02-blob-8.5devel-r2333.patch.gz
Description: application/gzip

-- 
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] Hot Standby on git

2009-09-28 Thread Heikki Linnakangas
Heikki Linnakangas wrote:
 Per Simon's request, for the benefit of the archive, here's all the
 changes I've done on the patch since he posted the initial version for
 review for this commitfest as incremental patches. This is extracted
 from my git repository at
 git://git.postgresql.org/git/users/heikki/postgres.git.

Further fixes extracted from above repository attached..

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com


hs-riggs-branch-20090928.tar.gz
Description: GNU Zip compressed 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] syslog_line_prefix

2009-09-28 Thread Magnus Hagander
On Sun, Sep 27, 2009 at 23:03, Robert Haas robertmh...@gmail.com wrote:
 On Sun, Sep 27, 2009 at 4:54 PM, Peter Eisentraut pete...@gmx.net wrote:
 On Sun, 2009-09-27 at 16:15 -0400, Tom Lane wrote:
 Peter Eisentraut pete...@gmx.net writes:
  Then why not send everything to syslog and have syslog filter it to the
  places you want to?  That is what syslog is for, after all.

 We send all syslog output with the same identifier/priority/facility,
 so there's not a lot of hope of getting syslog to do any useful
 filtering (at least not with the versions of syslog I'm familiar with).

 Time to upgrade then. ;-)  For example, the default syslog in Fedora has
 been rsyslog since Fedora 8, and that one can do a lot more than just
 filter by identifier/priority/facility.  syslog-ng is another popular
 example for a more featureful syslog.

 Presumably csvlog would be good for these sorts of things too, no?
 The whole point is it's machine-readable.

If there was a way to pipe the csv log through an external problem,
that would take care of much of the problem.

And I guess if you make that program responsible for *everything* it
would work - you just set your logging level to log very much data,
and let the external process deal with it. If we implemented the
ability to have a different logging level for different destinations
you could keep text logging for other things, or you could just
delegate all that to the external process as well. That would
basically turn the syslogger into a process that reads from the input
and sends the data out to an external process. But it could then
implement things like automatic restart of the external process in
case of crash etc, in perhaps a much easier way than the postmaster
can do for the syslogger itself.


-- 
 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] Issues for named/mixed function notation patch

2009-09-28 Thread Pavel Stehule
 Sorry, I'm having trouble understanding what you're driving at here.
 I think we should just not allow named notation to be combined with
 VARIADIC, at least for a first version of this feature, either when
 defining a function or when calling one.  We can consider relaxing
 that restriction at a later date if we can agree on what the semantics
 should be.

This is maybe too strict. I thing, so safe version is allow variadic
packed parameter with VARIADIC keyword as Jeff proposes.

I'll send actualised patch today.

Pavel


 ...Robert


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


[HACKERS] Rejecting weak passwords

2009-09-28 Thread Albe Laurenz
Dear hackers,

I have been thinking about ways to have PostgreSQL reject
weak passwords.

I think the standard recommendation is use PAM and LDAP,
but that requires the user to change the password outside
of PostgreSQL. And who would want to setup and maintain an
LDAP server just for this?

Since everybody has different ideas what is a good password,
there should be some way to configure that. I've looked at
how Oracle does it, and they simply let you write a
stored procedure that throws an exception if it doesn't
like the password.
Since users are on cluster level and functions live in
databases, that won't work in PostgreSQL.

I have come up with an idea or two and like to hear your
opinion.

1) One could have a set of GUCs like min_password_length,
   min_password_nonchars and similar that everybody
   could configure. This is not extremely flexible though.
2) Another idea would be a GUC that contains a regular
   expression that a password may *not* match.
   Perhaps that's too limiting too.
3) I have also considered a GUC that points to a loadable
   module that performs the password check if set.

Are there better ways?

Yours,
Laurenz Albe

-- 
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] CREATE LIKE INCLUDING COMMENTS and STORAGES

2009-09-28 Thread Itagaki Takahiro
I removed hunks by sql_help.c and fix a typo in documentation.
An updated patch attached.

Brendan Jurd dire...@gmail.com wrote:

 With the sql_help.c changes removed, the patch applied fine and
 testing went well.
 
 I noticed only the following in the new documentation in CREATE TABLE:
 -  literalINCLUDING DEFAULTS INCLUDING CONSTRAINTS INCLUDING INDEXES 
 INCLUDING STORAGES INCLUDING COMMENTS/literal.
 +  literalINCLUDING DEFAULTS INCLUDING CONSTRAINTS INCLUDING INDEXES 
 INCLUDING STORAGE INCLUDING COMMENTS/literal.
 
 Aside from the bogus hunks in the patch, and this one typo, the patch
 looks to be in excellent shape.


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



create-including_20090928a.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] Rejecting weak passwords

2009-09-28 Thread Magnus Hagander


On 28 sep 2009, at 11.54, Albe Laurenz laurenz.a...@wien.gv.at  
wrote:



Dear hackers,

I have been thinking about ways to have PostgreSQL reject
weak passwords.

I think the standard recommendation is use PAM and LDAP,
but that requires the user to change the password outside
of PostgreSQL. And who would want to setup and maintain an
LDAP server just for this?

Since everybody has different ideas what is a good password,
there should be some way to configure that. I've looked at
how Oracle does it, and they simply let you write a
stored procedure that throws an exception if it doesn't
like the password.
Since users are on cluster level and functions live in
databases, that won't work in PostgreSQL.

I have come up with an idea or two and like to hear your
opinion.

1) One could have a set of GUCs like min_password_length,
  min_password_nonchars and similar that everybody
  could configure. This is not extremely flexible though.
2) Another idea would be a GUC that contains a regular
  expression that a password may *not* match.
  Perhaps that's too limiting too.
3) I have also considered a GUC that points to a loadable
  module that performs the password check if set.

Are there better ways?


Isn't there some library we can link with and (conditionally) use? I  
believe windows exposes api function(s) to let you verify password  
complexity - I'm sure there is something similar available on unix,  
hopefully included on most common platforms?


/Magnus


--
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 returning setof foo[]

2009-09-28 Thread Abhijit Menon-Sen
At 2009-09-12 13:17:50 -0400, and...@dunslane.net wrote:

 I have just noticed, somewhat to my chagrin, that while in a plperl
 function that returns an array type you can return a perl arrayref,
 like this:

return [qw(a b c)];

 if the function returns a setof an array type you cannot do this:

return_next [qw(a b c)];

Right. This was an unintentional omission on my part.

 The fix is fairly small (see attached) although I need to check with
 some perlguts guru to see if I need to decrement a refcounter here or
 there.

Slightly simpler patch attached (and tested).

-- ams
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
***
*** 2021,2027  plperl_return_next(SV *sv)
  
  		if (SvOK(sv))
  		{
! 			char	   *val = SvPV(sv, PL_na);
  
  			ret = InputFunctionCall(prodesc-result_in_func, val,
  	prodesc-result_typioparam, -1);
--- 2021,2035 
  
  		if (SvOK(sv))
  		{
! 			char	   *val;
! 
! 			if (prodesc-fn_retisarray  SvROK(sv) 
! SvTYPE(SvRV(sv)) == SVt_PVAV)
! 			{
! sv = plperl_convert_to_pg_array(sv);
! 			}
! 
! 			val = SvPV(sv, PL_na);
  
  			ret = InputFunctionCall(prodesc-result_in_func, val,
  	prodesc-result_typioparam, -1);

-- 
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] syslog_line_prefix

2009-09-28 Thread Robert Haas
On Mon, Sep 28, 2009 at 5:22 AM, Magnus Hagander mag...@hagander.net wrote:
 On Sun, Sep 27, 2009 at 23:03, Robert Haas robertmh...@gmail.com wrote:
 On Sun, Sep 27, 2009 at 4:54 PM, Peter Eisentraut pete...@gmx.net wrote:
 On Sun, 2009-09-27 at 16:15 -0400, Tom Lane wrote:
 Peter Eisentraut pete...@gmx.net writes:
  Then why not send everything to syslog and have syslog filter it to the
  places you want to?  That is what syslog is for, after all.

 We send all syslog output with the same identifier/priority/facility,
 so there's not a lot of hope of getting syslog to do any useful
 filtering (at least not with the versions of syslog I'm familiar with).

 Time to upgrade then. ;-)  For example, the default syslog in Fedora has
 been rsyslog since Fedora 8, and that one can do a lot more than just
 filter by identifier/priority/facility.  syslog-ng is another popular
 example for a more featureful syslog.

 Presumably csvlog would be good for these sorts of things too, no?
 The whole point is it's machine-readable.

 If there was a way to pipe the csv log through an external problem,
 that would take care of much of the problem.

tail -f is probably a bit too fragile for this purpose, but I think it
would be possible to design a utility that would do this.  The idea
would be to maintain a state file that would list the most recent CSV
log file read and the byte offset of the first byte following the last
line processed.  On every iteration, we just open up the last file
read and read beginning at the designated offset through end of file.
Then we check if a newer file is available and, if so, we begin
reading that file.  When we're done reading, we update the state file.

There is the problem of what happens if we read a partial last line of
a file being written, but that seems surmountable: if the last line
read does not end in a newline, and no newer file is present, then
don't include that partial line in the output, and record the offset
of the beginning of that line in the state file.

I'm not sure if this will work on Windows, but it should be OK on
anything UNIX-ish.

 And I guess if you make that program responsible for *everything* it
 would work - you just set your logging level to log very much data,
 and let the external process deal with it. If we implemented the
 ability to have a different logging level for different destinations
 you could keep text logging for other things, or you could just
 delegate all that to the external process as well. That would
 basically turn the syslogger into a process that reads from the input
 and sends the data out to an external process. But it could then
 implement things like automatic restart of the external process in
 case of crash etc, in perhaps a much easier way than the postmaster
 can do for the syslogger itself.

The problem with having the syslogger send the data directly to an
external process is that the external process might be unable to
process the data as fast as syslogger is sending it.  I'm not sure
exactly what will happen in that case, but it will definitely be bad.
I think what will likely happen is that the entire database cluster
will end up waiting on write(2) calls to various places and processing
will grind to a halt.

I think it's better to spool the log messages to files, and then let
the external utility read the files.  The external utility can still
fall behind, but even if it does the cluster will continue running.

...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] syslog_line_prefix

2009-09-28 Thread Magnus Hagander
2009/9/28 Robert Haas robertmh...@gmail.com:
 On Mon, Sep 28, 2009 at 5:22 AM, Magnus Hagander mag...@hagander.net wrote:
 On Sun, Sep 27, 2009 at 23:03, Robert Haas robertmh...@gmail.com wrote:
 On Sun, Sep 27, 2009 at 4:54 PM, Peter Eisentraut pete...@gmx.net wrote:
 On Sun, 2009-09-27 at 16:15 -0400, Tom Lane wrote:
 Peter Eisentraut pete...@gmx.net writes:
  Then why not send everything to syslog and have syslog filter it to the
  places you want to?  That is what syslog is for, after all.

 We send all syslog output with the same identifier/priority/facility,
 so there's not a lot of hope of getting syslog to do any useful
 filtering (at least not with the versions of syslog I'm familiar with).

 Time to upgrade then. ;-)  For example, the default syslog in Fedora has
 been rsyslog since Fedora 8, and that one can do a lot more than just
 filter by identifier/priority/facility.  syslog-ng is another popular
 example for a more featureful syslog.

 Presumably csvlog would be good for these sorts of things too, no?
 The whole point is it's machine-readable.

 If there was a way to pipe the csv log through an external problem,
 that would take care of much of the problem.

 tail -f is probably a bit too fragile for this purpose, but I think it
 would be possible to design a utility that would do this.  The idea
 would be to maintain a state file that would list the most recent CSV
 log file read and the byte offset of the first byte following the last
 line processed.  On every iteration, we just open up the last file
 read and read beginning at the designated offset through end of file.
 Then we check if a newer file is available and, if so, we begin
 reading that file.  When we're done reading, we update the state file.

That would mean we have to write everything to the file, though, so it
would be rather bad for the case where you want to log just a little
but are delegating the decision to the external process. And it
would create double the I/O on disk for the logfile (once to the csv
log, once processed by the external process).


 There is the problem of what happens if we read a partial last line of
 a file being written, but that seems surmountable: if the last line
 read does not end in a newline, and no newer file is present, then
 don't include that partial line in the output, and record the offset
 of the beginning of that line in the state file.

 I'm not sure if this will work on Windows, but it should be OK on
 anything UNIX-ish.

Well, there'll be dealing with the sharing violations and stuff, but
we just need to make sure that the syslogger would open the file with
the proper sharing flags. Which I think it does already, actaully.


 And I guess if you make that program responsible for *everything* it
 would work - you just set your logging level to log very much data,
 and let the external process deal with it. If we implemented the
 ability to have a different logging level for different destinations
 you could keep text logging for other things, or you could just
 delegate all that to the external process as well. That would
 basically turn the syslogger into a process that reads from the input
 and sends the data out to an external process. But it could then
 implement things like automatic restart of the external process in
 case of crash etc, in perhaps a much easier way than the postmaster
 can do for the syslogger itself.

 The problem with having the syslogger send the data directly to an
 external process is that the external process might be unable to
 process the data as fast as syslogger is sending it.  I'm not sure
 exactly what will happen in that case, but it will definitely be bad.
 I think what will likely happen is that the entire database cluster
 will end up waiting on write(2) calls to various places and processing
 will grind to a halt.

We'll have the same issue if we have the syslogger write it to disk,
don't we? In fact, it might even be faster depending on how much
processing is done and what can be thrown away at that step, since it
could decrease the disk I/O needed in favor of CPU work.


 I think it's better to spool the log messages to files, and then let
 the external utility read the files.  The external utility can still
 fall behind, but even if it does the cluster will continue running.

The difficulty there is to make it live enough. But I guess if it
implements the same method as tail -f, it would do that - the only
issue then would be the fact that this would require much more I/O on
disk.


-- 
 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] Rejecting weak passwords

2009-09-28 Thread Andrew Dunstan



Albe Laurenz wrote:

Dear hackers,

I have been thinking about ways to have PostgreSQL reject
weak passwords.

I think the standard recommendation is use PAM and LDAP,
but that requires the user to change the password outside
of PostgreSQL. And who would want to setup and maintain an
LDAP server just for this?

Since everybody has different ideas what is a good password,
there should be some way to configure that. I've looked at
how Oracle does it, and they simply let you write a
stored procedure that throws an exception if it doesn't
like the password.
Since users are on cluster level and functions live in
databases, that won't work in PostgreSQL.

I have come up with an idea or two and like to hear your
opinion.

1) One could have a set of GUCs like min_password_length,
   min_password_nonchars and similar that everybody
   could configure. This is not extremely flexible though.
2) Another idea would be a GUC that contains a regular
   expression that a password may *not* match.
   Perhaps that's too limiting too.
3) I have also considered a GUC that points to a loadable
   module that performs the password check if set.


  


My vote is for #3, if anything.

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] syslog_line_prefix

2009-09-28 Thread Robert Haas
On Mon, Sep 28, 2009 at 6:51 AM, Magnus Hagander mag...@hagander.net wrote:
 I think it's better to spool the log messages to files, and then let
 the external utility read the files.  The external utility can still
 fall behind, but even if it does the cluster will continue running.

 The difficulty there is to make it live enough. But I guess if it
 implements the same method as tail -f, it would do that - the only
 issue then would be the fact that this would require much more I/O on
 disk.

The I/O issue is a tricky one.  If that's an issue, then maybe a pipe
or socket is a better fit.  But if the pipe fills up, then the logging
collector needs to begin spooling the messages to disk so that the
whole system doesn't pile up on the external log analyzer.  Figuring
out the right design here is a bit tricky.

...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] Rejecting weak passwords

2009-09-28 Thread Ing. Marcos L. Ortí­z Valmaseda

Andrew Dunstan escribió:



Albe Laurenz wrote:

Dear hackers,

I have been thinking about ways to have PostgreSQL reject
weak passwords.

I think the standard recommendation is use PAM and LDAP,
but that requires the user to change the password outside
of PostgreSQL. And who would want to setup and maintain an
LDAP server just for this?

Since everybody has different ideas what is a good password,
there should be some way to configure that. I've looked at
how Oracle does it, and they simply let you write a
stored procedure that throws an exception if it doesn't
like the password.
Since users are on cluster level and functions live in
databases, that won't work in PostgreSQL.

I have come up with an idea or two and like to hear your
opinion.

1) One could have a set of GUCs like min_password_length,
   min_password_nonchars and similar that everybody
   could configure. This is not extremely flexible though.
2) Another idea would be a GUC that contains a regular
   expression that a password may *not* match.
   Perhaps that's too limiting too.
3) I have also considered a GUC that points to a loadable
   module that performs the password check if set.


  


My vote is for #3, if anything.

cheers

andrew

You have to analyze all points before to do this. I vote too for the 
third option, but you have to be clear that how do you ´ll check the 
weakness of the password:

1- For example: the length should be greater that 6 char..
2- The password should be have  a combination fo numbers, letters and 
others dots


Things like that you have to think very well, or to do a question to the 
list asking which are the best options.


I think the same about the PAM and LDAP auth

Regards

--
DBAs must implements decisions based on the best fit of the application,DBMS, 
and platform
..for that reason...I use PostgreSQL + Linux

Ing. Marcos L. Ortiz Valmaseda
Línea Soporte y Despliegue
Centro de Tecnologías de Almacenamiento y Análisis de Datos (CENTALAD)

Linux User # 418229
PostgreSQL User
http://www.postgresql.org
http://www.planetpostgresql.org/
http://www.postgresql-es.org/

attachment: mlortiz.vcf
-- 
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] Rejecting weak passwords

2009-09-28 Thread Stephen Frost
* Magnus Hagander (mag...@hagander.net) wrote:
 Are there better ways?

 Isn't there some library we can link with and (conditionally) use? I  
 believe windows exposes api function(s) to let you verify password  
 complexity - I'm sure there is something similar available on unix,  
 hopefully included on most common platforms?

cracklib2.

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Rejecting weak passwords

2009-09-28 Thread Bill Moran
In response to Ing. Marcos L. Ortí­z Valmaseda mlor...@uci.cu:

 Andrew Dunstan escribió:
 
  Albe Laurenz wrote:
  Dear hackers,
 
  I have been thinking about ways to have PostgreSQL reject
  weak passwords.
 
  I think the standard recommendation is use PAM and LDAP,
  but that requires the user to change the password outside
  of PostgreSQL. And who would want to setup and maintain an
  LDAP server just for this?

An option here is to have a way for PG to hook in to LDAP/PAM so
that an ALTER ROLE actually changes the LDAP/PAM password.

  Since everybody has different ideas what is a good password,
  there should be some way to configure that. I've looked at
  how Oracle does it, and they simply let you write a
  stored procedure that throws an exception if it doesn't
  like the password.

[snip]

  3) I have also considered a GUC that points to a loadable
 module that performs the password check if set.
 
 You have to analyze all points before to do this. I vote too for the 
 third option, but you have to be clear that how do you ´ll check the 
 weakness of the password:
 1- For example: the length should be greater that 6 char..
 2- The password should be have  a combination fo numbers, letters and 
 others dots

I think you've missed the point.  If a loadable module is used, then
it can do anything the DBAs need it to.  You can write your own module
if you're not happy with those provided.  At that point, arguing about
what makes a good password is pretty irrelevant.  Note the paragraph
that I didn't snip where Albe points this out.

-- 
Bill Moran
http://www.potentialtech.com
http://people.collaborativefusion.com/~wmoran/

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


Re: [HACKERS] operator exclusion constraints

2009-09-28 Thread Robert Haas
On Sun, Sep 27, 2009 at 11:31 PM, Jeff Davis pg...@j-davis.com wrote:
 On Sun, 2009-09-27 at 22:40 -0400, Robert Haas wrote:
 Apparently, CommitFest
 no longer means a time when people put aside their own patches to
 review those of others; it seems now to mean a time when 87% of the
 patch authors either continue development or ignore the CommitFest
 completely.

 Well, I'm not completely innocent here, either. I also spent time making
 progress on my patch, both in terms of code and discussion so that I
 would at least have enough information to get it ready during the next
 development cycle.

I don't see any problem with that.  As long as everyone is willing to
spend SOME time on their own patch and SOME time reviewing the work of
others, the system works.  After all, the time to review a patch is,
IME, far shorter than the time to develop one.  But right now we have
a large majority of patch authors who are not contributing ANYTHING to
the CommitFest, and that's not so good.

...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] Rejecting weak passwords

2009-09-28 Thread Magnus Hagander
2009/9/28 Stephen Frost sfr...@snowman.net:
 * Magnus Hagander (mag...@hagander.net) wrote:
 Are there better ways?

 Isn't there some library we can link with and (conditionally) use? I
 believe windows exposes api function(s) to let you verify password
 complexity - I'm sure there is something similar available on unix,
 hopefully included on most common platforms?

 cracklib2.

That sounds like the one I thought about :-)

-- 
 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] Rejecting weak passwords

2009-09-28 Thread Andrew Dunstan



Ing. Marcos L. Ortí­z Valmaseda wrote: 


My vote is for #3, if anything.


You have to analyze all points before to do this. I vote too for the 
third option, but you have to be clear that how do you ´ll check the 
weakness of the password:

1- For example: the length should be greater that 6 char..
2- The password should be have  a combination fo numbers, letters and 
others dots


Things like that you have to think very well, or to do a question to 
the list asking which are the best options.


I think the same about the PAM and LDAP auth




I'm voting for #3 precisely so postgres doesn't have to think about it, 
and the module author will do all the work implementing whatever rules 
they want to enforce.


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] Rejecting weak passwords

2009-09-28 Thread Magnus Hagander
2009/9/28 Andrew Dunstan and...@dunslane.net:


 Ing. Marcos L. Ortí­z Valmaseda wrote:

 My vote is for #3, if anything.


 You have to analyze all points before to do this. I vote too for the third 
 option, but you have to be clear that how do you ´ll check the weakness of 
 the password:
 1- For example: the length should be greater that 6 char..
 2- The password should be have  a combination fo numbers, letters and others 
 dots

 Things like that you have to think very well, or to do a question to the 
 list asking which are the best options.

 I think the same about the PAM and LDAP auth



 I'm voting for #3 precisely so postgres doesn't have to think about it, and 
 the module author will do all the work implementing whatever rules they want 
 to enforce.

That makes a lot of sense. Then we could perhaps ship a cracklib2
provider in contrib.

-- 
 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] syslog_line_prefix

2009-09-28 Thread Tom Lane
[ please trim the quoted material a bit, folks ]

Magnus Hagander mag...@hagander.net writes:
 2009/9/28 Robert Haas robertmh...@gmail.com:
 The problem with having the syslogger send the data directly to an
 external process is that the external process might be unable to
 process the data as fast as syslogger is sending it.  I'm not sure
 exactly what will happen in that case, but it will definitely be bad.

This is the same issue already raised with respect to syslog versus
syslogger, ie, some people would rather lose log data than have the
backends block waiting for it to be written.

 That would mean we have to write everything to the file, though, so it
 would be rather bad for the case where you want to log just a little
 but are delegating the decision to the external process. And it
 would create double the I/O on disk for the logfile (once to the csv
 log, once processed by the external process).

Robert's design could be made to work without that, if you dump the
original log output into a ramdisk and let the external process write
whatever it chooses to real disk.  If you have a system crash you lose
any as-yet-unprocessed log output, but hopefully there usually wouldn't
be much.

regards, tom lane

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


Re: [HACKERS] Rejecting weak passwords

2009-09-28 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes:
 Albe Laurenz wrote:
 1) One could have a set of GUCs like min_password_length,
 min_password_nonchars and similar that everybody
 could configure. This is not extremely flexible though.
 2) Another idea would be a GUC that contains a regular
 expression that a password may *not* match.
 Perhaps that's too limiting too.
 3) I have also considered a GUC that points to a loadable
 module that performs the password check if set.

 My vote is for #3, if anything.

Yeah.  I think there is no chance of anything in this vein getting
accepted into core Postgres, if only because everybody will have a
different idea of what it needs to do.  A hook function (no need
for a GUC) would be a reasonable proposal.

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] WIP - syslogger infrastructure changes

2009-09-28 Thread Peter Eisentraut
On Sat, 2009-09-26 at 15:35 -0600, Joshua Tolley wrote:
 On Sat, Sep 26, 2009 at 11:43:46AM -0400, Tom Lane wrote:
  complete but more complex solution.  (dup2 works on Windows, no?)
 
 Unless I'm misreading syslogger.c, dup2() gets called on every platform.
 
 I've started the wiki page in question:
 http://wiki.postgresql.org/wiki/Logging_Brainstorm

Most of the items listed there you can already do with a sufficiently
non-ancient syslog implementation.

Has anyone ever actually tested rsyslog and syslog-ng for performance
and robustness with PostgreSQL?  Half the ideas about on-disk queuing
and checkpointing and so on that have been mentioned recently seem to
come straight from their documentations.


-- 
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] Rejecting weak passwords

2009-09-28 Thread Tom Lane
Actually there's a much bigger problem with asking the backend to reject
weak passwords: what ya gonna do with a pre-MD5'd string?  Which is
exactly what the backend is going to always get, in a security-conscious
environment.

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] Rejecting weak passwords

2009-09-28 Thread Tom Lane
Albe Laurenz laurenz.a...@wien.gv.at writes:
 Tom Lane wrote:
 Actually there's a much bigger problem with asking the backend to reject
 weak passwords: what ya gonna do with a pre-MD5'd string?  Which is
 exactly what the backend is going to always get, in a security-conscious
 environment.

 I'm thinking of the case where somebody changes his or her
 password interactively on the command line, with pgAdmin III,
 or similar. People would hardly use the above in that case,

Really?  If pgAdmin has a password-change function that doesn't use
client-side password encryption then somebody should file a bug against
it.  Sending unencrypted passwords exposes the password at least to the
postmaster logfile.  createuser has been doing encryption, unless
specifically commanded not to, for a long time.

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] Rejecting weak passwords

2009-09-28 Thread Dave Page
On Mon, Sep 28, 2009 at 4:38 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Albe Laurenz laurenz.a...@wien.gv.at writes:
 Tom Lane wrote:
 Actually there's a much bigger problem with asking the backend to reject
 weak passwords: what ya gonna do with a pre-MD5'd string?  Which is
 exactly what the backend is going to always get, in a security-conscious
 environment.

 I'm thinking of the case where somebody changes his or her
 password interactively on the command line, with pgAdmin III,
 or similar. People would hardly use the above in that case,

 Really?  If pgAdmin has a password-change function that doesn't use
 client-side password encryption then somebody should file a bug against
 it.  Sending unencrypted passwords exposes the password at least to the
 postmaster logfile.  createuser has been doing encryption, unless
 specifically commanded not to, for a long time.

pgAdmin MD5's the passwords if you use the GUI to change them, or when
add a user. It doesn't make any attempt to parse the SQL if you enter
it yourself in the query tool though (nor is it going to).

-- 
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] Rejecting weak passwords

2009-09-28 Thread Tom Lane
Dave Page dp...@pgadmin.org writes:
 pgAdmin MD5's the passwords if you use the GUI to change them, or when
 add a user. It doesn't make any attempt to parse the SQL if you enter
 it yourself in the query tool though (nor is it going to).

No, I wouldn't expect it to go that far.  My point is just that
pre-MD5'd passwords are a lot commoner than Albe seems to 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] Rejecting weak passwords

2009-09-28 Thread Albe Laurenz
Tom Lane wrote:
 Actually there's a much bigger problem with asking the backend to reject
 weak passwords: what ya gonna do with a pre-MD5'd string?  Which is
 exactly what the backend is going to always get, in a security-conscious
 environment.

You mean if the password is set with
CREATE/ALTER ROLE x ENCRYPTED PASSWORD 'md5string' ?
That could not be checked of course...

I'm thinking of the case where somebody changes his or her
password interactively on the command line, with pgAdmin III,
or similar. People would hardly use the above in that case,
and you can do it safely over SSL encrypted connections.

Given the feedback by everybody (thanks!) I will investigate
the option of a loadable module or some other kind of hook.

Is there code in PostgreSQL that uses something like that
to get me started?

Yours,
Laurenz Albe

-- 
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] Using results from INSERT ... RETURNING

2009-09-28 Thread Robert Haas
On Mon, Sep 28, 2009 at 11:31 AM, Marko Tiikkaja
marko.tiikk...@cs.helsinki.fi wrote:
 Robert Haas wrote:

 So I think we should at a minimum ask the patch author to (1) fix the
 explain bugs I found and (2) update the README, as well as (3) revert
 needless whitespace changes - there are a couple in execMain.c, from
 the looks of it.

 In the attached patch, I made the changes to explain as you suggested and
 reverted the only whitespace change I could find from execMain.c. However,
 English isn't my first language so I'm not very confident about fixing the
 README.

Can you at least take a stab at it?  We can fix your grammar, but
guessing what's going on without documentation is hard.

...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] Issues for named/mixed function notation patch

2009-09-28 Thread Jeff Davis
On Mon, 2009-09-28 at 11:50 +0200, Pavel Stehule wrote:
 This is maybe too strict. I thing, so safe version is allow variadic
 packed parameter with VARIADIC keyword as Jeff proposes.

The combination of variadic parameters and named call notation is
somewhat strange, on second thought. Can you identify a use case?

If not, then it should probably be blocked in this version of the patch.
Even if it makes sense from a syntax standpoint, it might be confusing
to users.

Robert, did you have a specific concern in mind? Do you see a behavior
there that we might want to change in the future?

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] Issues for named/mixed function notation patch

2009-09-28 Thread Pavel Stehule
2009/9/28 Jeff Davis pg...@j-davis.com:
 On Mon, 2009-09-28 at 11:50 +0200, Pavel Stehule wrote:
 This is maybe too strict. I thing, so safe version is allow variadic
 packed parameter with VARIADIC keyword as Jeff proposes.

 The combination of variadic parameters and named call notation is
 somewhat strange, on second thought. Can you identify a use case?


I have not any use case now. Simply when I have a variadic function,
then I would to allow call it with named notation. Some like

create or replace foo (a int, variadic b int[]) ...

SELECT foo(10 as int, variadic array[10,20] as b)

 If not, then it should probably be blocked in this version of the patch.
 Even if it makes sense from a syntax standpoint, it might be confusing
 to users.


when I though about control, I found so syntax with mandatory VARIADIC
is difficult implementable. So probably the most feasible solution for
this moment is to discard a variadic functions from set of functions
that are callable with named notation. So I thing we are in tune, and
I am going to update patch.

Regards
Pavel Stehule

 Robert, did you have a specific concern in mind? Do you see a behavior
 there that we might want to change in the future?

 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


[HACKERS] ECPG patch views [moved from RRR list]

2009-09-28 Thread Dan Colish
- Forwarded message from Robert Haas robertmh...@gmail.com -

Date: Sun, 27 Sep 2009 12:52:35 -0400
From: Robert Haas robertmh...@gmail.com
To: Boszormenyi Zoltan z...@cybertec.at
Cc: Dan Colish d...@unencrypted.org, pgsql-rrreview...@postgresql.org,
Jeff Janes jeff.ja...@gmail.com,
Hans-Juergen Schoenig h...@cybertec.at,
Michael Meskes mes...@postgresql.org
Subject: Re: CF 2009-09: initial reviewing assignments

2009/9/27 Boszormenyi Zoltan z...@cybertec.at:

 On Sat, Sep 26, 2009 at 11:39:45AM -0700, Jeff Janes wrote:
  Hi Robert,
 
  Is there another patch you'd like me to work on?
 
  Lock wait statistics says it needs review, but the last comment
  suggests it is waiting on author.
 
  Enhancements to COPY (error logging and autopartitioning) says it is
  waiting on author but last comment suggests perhaps it is ready for
  review.
 
  I've taken a look at ECPG, but I couldn't make heads or tails of it.
  I guess I could try harder :)
 
  It looks like the ECPG patches are not independent and need to applied
  in a particular order in order for them to apply cleanly to HEAD.
 
  So I think I need some guidance on what I should be doing.
 
  Thanks,
 
  Jeff
 

 I've been looking at the dynamic cursor names patch, so if you have any
 insights I would really appreciate them. I am having some trouble fully
 reviewing this patch because I am not very familiar with the ecpg code.

 --
 --Dan


 Asketh and you shall be given. :-)

 May I help you in understanding ECPG?

 The dynamic cursorname patch was started on 8.3.7 first
 and we moved to 8.4 (then 8.5) CVS because in 8.4,
 ECPG grammar was rewritten to be auto-generated
 from the core grammar. I had to dive in kneep deep
 before I could modify it...

 Basic thing is that ECPG modifies and extends the core
 grammar in a way that
 1) every token in ECPG is str type. New tokens are
   defined in ecpg.tokens, types are defined in ecpg.type
 2) most tokens from the core grammar are simply converted
   to literals concatenated together to form the SQL string
   passed to the server, this is done by parse.pl.
 3) some rules need side-effects, actions are either added
   or completely overridden (compared to the basic token
   concatenation) for them, these are defined in ecpg.addons,
   the rules for ecpg.addons are explained below.
 4) new grammar rules are needed for ECPG metacommands.
   These are in ecpg.trailer.
 5) ecpg.header contains common functions, etc. used by
   actions for grammar rules.

 In ecpg.addons, every modified rule follows this pattern:
       ECPG: dumpedtokens postfix
 where dumpedtokens is simply tokens from core gram.y's
 rules concatenated together. e.g. if gram.y has this:
       ruleA: tokenA tokenB tokenC {...}
 then dumpedtokens is ruleAtokenAtokenBtokenC.
 postfix above can be:
 a) block - the automatic rule created by parse.pl is completely
    overridden, the code block has to be written completely as
    it were in a plain bison grammar
 b) rule - the automatic rule is extended on, so new syntaxes
    are accepted for ruleA. E.g.:
      ECPG: ruleAtokenAtokenBtokenC rule
          | tokenD tokenE { action_code; }
          ...
    It will be substituted with:
      ruleA: original syntax forms and actions up to and including
                    tokenA tokenB tokenC
             | tokenD tokenE { action_code; }
             ...
 c) addon - the automatic action for the rule (SQL syntax constructed
    from the tokens concatenated together) is prepended with a new
    action code part. This code part is written as is's already inside
    the { ... }

 Multiple addon or block lines may appear together with the
 new code block if the code block is common for those rules, which
 is a very smart thing.

 This was what I gathered from the code. The documentation
 seems to be missing from the rewritten ECPG grammar in 8.4.
 Michael, am I missing something?

 Dan, please, start the review in light of the above.
 If you have any questions, don't hesitate to ask.

Please move this discussion to -hackers, maybe with a link back to
this post.  Good info, wrong place.  :-)

...Robert

- End forwarded message -

I'm bumping this message to pg-hackers.

Here is a link to the archive as well:
http://archives.postgresql.org/pgsql-rrreviewers/2009-09/msg00039.php


--
--Dan

-- 
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] Issues for named/mixed function notation patch

2009-09-28 Thread Jeff Davis
On Mon, 2009-09-28 at 18:23 +0200, Pavel Stehule wrote:
 when I though about control, I found so syntax with mandatory VARIADIC
 is difficult implementable. So probably the most feasible solution for
 this moment is to discard a variadic functions from set of functions
 that are callable with named notation. So I thing we are in tune, and
 I am going to update patch.

Sounds good. I am looking at the code, and there's a part I don't
understand:

In FuncnameGetCandidates():
  /*
   * Wait with apply proargidxs on args. Detection ambigouos needs
   * consistent args (based on proargs). Store proargidxs for later
   * use.
   */
   newResult-proargidxs = proargidxs; 

But after calling FuncnameGetCandidates (the only place where fargnames
is non-NIL), you immediately re-assign to best_candidate-args. What
happens between those two places, and why can't it happen in
FuncnameGetCandidates?

Also, you should consistently pass NIL when you mean an empty list, but
sometimes you pass NULL to FuncnameGetCandidates().

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] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server

2009-09-28 Thread Chris Browne
pete...@gmx.net (Peter Eisentraut) writes:
 On Fri, 2009-09-25 at 16:59 -0400, Tom Lane wrote:
 shakahsha...@gmail.com shakahsha...@gmail.com writes:
  From pg_dump/pg_restore section (9.2 of the Todo page on the
  PostgreSQL Wiki), is the following item
Add comments to output indicating version of pg_dump and of the
  database server
  simply asking for a change to the pg_dump header from:
 
 I think so, but what's not clear is whether this is a good idea to do
 in the default output.  It might only be appropriate in verbose mode,
 so as not to introduce unnecessary diffs between logically identical
 dumps.

 Well, a diff of the same database made by different (major) versions of
 pg_dump will already be different in most situations, so adding the
 pg_dump version number in it is essentially free from this perspective.

 What is the use case for adding the server version?

 I can imagine something like wanting to know exactly where the dump came
 from, but then host name and such would be better.  (And then you can
 infer the server version from that.)

I added this ToDo because we had a case where we were spelunking
through some old pg_dumps, and the provenance was sufficiently distant
that we couldn't readily infer what PostgreSQL version was involved.

If pg_dump reported something like:

-- pg_dump version: 8.5_devel
-- postgres server version: 8.4.17

then it would be trivial to ascertain the information.

Actually, I have no argument with your point; perhaps a whole header
section is the right answer:

-- pg_dump version: 8.5_devel
-- postgres server version: 8.4.17
-- dump began at: 2010-07-01 14:22:27 EDT
-- server name: wolfe
-- more, maybe?


 Another issue is that it's not all that clear what to do or how to do it
 for archive dumps --- do you then want both pg_dump and pg_restore to
 tell you about themselves?

 I don't see a good reason for pg_restore to get involved.

Agreed.  This isn't needed for pg_restore to do anything better; it's
so that humans can do better archaeology.
-- 
let name=cbbrowne and tld=acm.org in name ^ @ ^ tld;;
http://linuxfinances.info/info/languages.html
Rules of the  Evil Overlord #187. I will not  hold lavish banquets in
the middle of  a famine. The good PR among the  guests doesn't make up
for the bad PR among the masses.  http://www.eviloverlord.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] syslog_line_prefix

2009-09-28 Thread Alvaro Herrera
Tom Lane escribió:
 [ please trim the quoted material a bit, folks ]
 
 Magnus Hagander mag...@hagander.net writes:
  2009/9/28 Robert Haas robertmh...@gmail.com:
  The problem with having the syslogger send the data directly to an
  external process is that the external process might be unable to
  process the data as fast as syslogger is sending it. �I'm not sure
  exactly what will happen in that case, but it will definitely be bad.
 
 This is the same issue already raised with respect to syslog versus
 syslogger, ie, some people would rather lose log data than have the
 backends block waiting for it to be written.

That could be made configurable; i.e. let the user choose whether to
lose messages or to make everybody wait.

-- 
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] syslog_line_prefix

2009-09-28 Thread Robert Haas
On Mon, Sep 28, 2009 at 1:07 PM, Alvaro Herrera
alvhe...@commandprompt.com wrote:
 Tom Lane escribió:
 [ please trim the quoted material a bit, folks ]

 Magnus Hagander mag...@hagander.net writes:
  2009/9/28 Robert Haas robertmh...@gmail.com:
  The problem with having the syslogger send the data directly to an
  external process is that the external process might be unable to
  process the data as fast as syslogger is sending it.  I'm not sure
  exactly what will happen in that case, but it will definitely be bad.

 This is the same issue already raised with respect to syslog versus
 syslogger, ie, some people would rather lose log data than have the
 backends block waiting for it to be written.

 That could be made configurable; i.e. let the user choose whether to
 lose messages or to make everybody wait.

I think the behavior I was proposing was neither drop nor wait,
but buffer.  Not sure how people feel about that.

...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] Issues for named/mixed function notation patch

2009-09-28 Thread Pavel Stehule
2009/9/28 Jeff Davis pg...@j-davis.com:
 On Mon, 2009-09-28 at 18:23 +0200, Pavel Stehule wrote:
 when I though about control, I found so syntax with mandatory VARIADIC
 is difficult implementable. So probably the most feasible solution for
 this moment is to discard a variadic functions from set of functions
 that are callable with named notation. So I thing we are in tune, and
 I am going to update patch.

 Sounds good. I am looking at the code, and there's a part I don't
 understand:

 In FuncnameGetCandidates():
  /*
   * Wait with apply proargidxs on args. Detection ambigouos needs
   * consistent args (based on proargs). Store proargidxs for later
   * use.
   */
   newResult-proargidxs = proargidxs;


 But after calling FuncnameGetCandidates (the only place where fargnames
 is non-NIL), you immediately re-assign to best_candidate-args. What
 happens between those two places, and why can't it happen in
 FuncnameGetCandidates?

I am not sure - I have to look to code, but if I remember well, there
are same arrays, with same values, but the field are different order.
One is related to pgproc and second to real params. But I have to
check code again.

 Also, you should consistently pass NIL when you mean an empty list, but
 sometimes you pass NULL to FuncnameGetCandidates().

It's bug, where is it?

Regards
Pavel


 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] DefaultACLs

2009-09-28 Thread Tom Lane
Petr Jelinek pjmo...@pjmodos.net writes:
 [ latest version of DefaultACLs patch ]

I started looking through this patch, but found that it's not nearly
ready to commit :-(.  The big missing piece is that there's no pg_dump
support for default ACLs.  That's a bigger chunk of code than I have
time/interest to write, and I don't think I want to commit the feature
without it.  (I'm willing to commit without tab completion or any
psql \d command to show the defaults, but pg_dump just isn't optional.)

There is another large problem, too.  The patch seems to have
only half-baked support for global defaults (those not tied to a
specific schema) --- it looks like you can put them in, but half
of the code will ignore them or else fail while trying to use them.
This isn't just a matter of a few missed cases while coding, I think.
The generic issue that the code doesn't even think about addressing
is which default should apply when there's potentially more than one
applicable default?  As long as there's only global and per-schema
defaults, it's not too hard to decide that the latter take precedence
over the former; but I have no idea what we're going to do in order
to add any other features.  This seems like a sufficiently big
conceptual issue that it had better be resolved now, even if the first
version of the patch doesn't really need to deal with it.

Also, the GRANT DEFAULT PRIVILEGES thing just seems completely bizarre,
and I'm not convinced it has a sufficient use-case to justify such a
strange wart on GRANT.  I think we should drop it.  Or at least it needs
to be proposed and discussed as a separate feature.  Maybe it would seem
less strange if the syntax was RESET PRIVILEGES ON object.

A couple of minor gripes:

Per recent discussion, names of system catalogs shouldn't be plural.

elog() is not suitable for user-facing errors.  For example in
ExecGrantStmt_Defaults, the grammar does not prohibit the unsupported
target object types, so you need to throw a nicer error there.  Or
else reject them in the grammar.  There seem to be a number of other
places where elog is used but the error is perfectly likely to be caused
by a user mistake.

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] syslog_line_prefix

2009-09-28 Thread Tom Lane
Alvaro Herrera alvhe...@commandprompt.com writes:
 Tom Lane escribió:
 This is the same issue already raised with respect to syslog versus
 syslogger, ie, some people would rather lose log data than have the
 backends block waiting for it to be written.

 That could be made configurable; i.e. let the user choose whether to
 lose messages or to make everybody wait.

Hmm, I guess I missed where you proposed an implementation that would
support that?

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] Review handling of MOVE and FETCH (ToDo)

2009-09-28 Thread John Naylor
(resent to -hackers)

I just applied and tested the new patch. Everything works great.

The only thing I would change now is some of the comments.

1). On line 289, one of the regression test comments got copied:

+   move forward in c;                --should be at '5'

change to:

+   move forward in c;                --should be at '1'

2). Lines 79/80:

+
  errmsg(statement FETCH returns more rows.),
+
  errhint(Multirows fetch are not allowed in PL/pgSQL.)));

This might sound better as statement FETCH returns multiple rows.,
and Multirow FETCH is not allowed in PL/pgSQL.

Everything else looks good to me.
John


 Hi Selena and John,

 Pavel's latest patch seems to address all the issues you raised in
 your initial review.  Do you have any comments on this new revision?
 If you're happy that your issues have been resolved, please mark the
 patch as Ready for Committer.

 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] Issues for named/mixed function notation patch

2009-09-28 Thread Jeff Davis
On Mon, 2009-09-28 at 19:26 +0200, Pavel Stehule wrote:
  Also, you should consistently pass NIL when you mean an empty list, but
  sometimes you pass NULL to FuncnameGetCandidates().
 
 It's bug, where is it?

In regproc.c.

Jeff


-- 
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] Rejecting weak passwords

2009-09-28 Thread Marko Kreen
On 9/28/09, Tom Lane t...@sss.pgh.pa.us wrote:
 Actually there's a much bigger problem with asking the backend to reject
 weak passwords: what ya gonna do with a pre-MD5'd string?  Which is
 exactly what the backend is going to always get, in a security-conscious
 environment.

Hmm...  Looking at the docs, I don't see anywhere mention that the MD5
hashed passwords can still be used to log in, that only thing MD5'ing
protects from is actually knowing what the password was.

Also, although it does protect from sniffing password at login time,
it is as insecure as plaintext password against sniffing on
CREATE/ALTER USER time.

Thus only secure way to change password is over SSL, in which case
it does not make much difference whether it's plaintext or not.
Logfile may seem to be as argument against plaintext, but note
that you still cannot allow unprivileged users to access logfile
that may contain MD5 passwords, as they can use them to log in
as another user.

So promoting the ENCRYPTED 'foo' as secure may lure users into
false sense of security, and be lax against sniffing and logfile
protection.

Better approach seems to be simply refuse to log the password
value into logfile, whether it's MD5 or not, and promote SSL
as only secure way of changing passwords.

IOW, having plaintext password in CREATE/ALTER time which can
then checked for weaknesses is better that MD5 password, which
actually does not increase security.

We just need to fix the logging.

-- 
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] plperl returning setof foo[]

2009-09-28 Thread Andrew Dunstan



Abhijit Menon-Sen wrote:

The fix is fairly small (see attached) although I need to check with
some perlguts guru to see if I need to decrement a refcounter here or
there.



Slightly simpler patch attached (and tested).

  



Thanks. Committed.

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] syslog_line_prefix

2009-09-28 Thread Alvaro Herrera
Robert Haas escribió:
 On Mon, Sep 28, 2009 at 1:07 PM, Alvaro Herrera
 alvhe...@commandprompt.com wrote:
  Tom Lane escribió:
  [ please trim the quoted material a bit, folks ]
 
  Magnus Hagander mag...@hagander.net writes:
   2009/9/28 Robert Haas robertmh...@gmail.com:
   The problem with having the syslogger send the data directly to an
   external process is that the external process might be unable to
   process the data as fast as syslogger is sending it.  I'm not sure
   exactly what will happen in that case, but it will definitely be bad.
 
  This is the same issue already raised with respect to syslog versus
  syslogger, ie, some people would rather lose log data than have the
  backends block waiting for it to be written.
 
  That could be made configurable; i.e. let the user choose whether to
  lose messages or to make everybody wait.
 
 I think the behavior I was proposing was neither drop nor wait,
 but buffer.  Not sure how people feel about that.

Given an arbitrary increase in log rate during an arbitrary length of
time, any buffer you keep will be filled.

-- 
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] [PATCH] DefaultACLs

2009-09-28 Thread Petr Jelinek

Tom Lane wrote:

Petr Jelinek pjmo...@pjmodos.net writes:
  

[ latest version of DefaultACLs patch ]



I started looking through this patch, but found that it's not nearly
ready to commit :-(.  The big missing piece is that there's no pg_dump
support for default ACLs.  That's a bigger chunk of code than I have
time/interest to write, and I don't think I want to commit the feature
without it.  (I'm willing to commit without tab completion or any
psql \d command to show the defaults, but pg_dump just isn't optional.)
  


Yeah I completely forgot about pg_dump just like I did with anonymous 
code blocks :-(



There is another large problem, too.  The patch seems to have
only half-baked support for global defaults (those not tied to a
specific schema) --- it looks like you can put them in, but half
of the code will ignore them or else fail while trying to use them.
This isn't just a matter of a few missed cases while coding, I think.
The generic issue that the code doesn't even think about addressing
is which default should apply when there's potentially more than one
applicable default?  As long as there's only global and per-schema
defaults, it's not too hard to decide that the latter take precedence
over the former; but I have no idea what we're going to do in order
to add any other features.  This seems like a sufficiently big
conceptual issue that it had better be resolved now, even if the first
version of the patch doesn't really need to deal with it.
  


Half of the code will ignore them ? They are ignored if schema specific 
defaults were set.
Yes I haven't tried to solve the problem of having non-hierarchical 
filters for defaults and if we require that then this patch is dead for 
(at least) this commitfest, because at the moment I don't even know 
where to begin solving this.



Also, the GRANT DEFAULT PRIVILEGES thing just seems completely bizarre,
and I'm not convinced it has a sufficient use-case to justify such a
strange wart on GRANT.  I think we should drop it.  Or at least it needs
to be proposed and discussed as a separate feature.  Maybe it would seem
less strange if the syntax was RESET PRIVILEGES ON object.
  


I vote for dropping it then.

--
Regards
Petr Jelinek (PJMODOS)



Re: [HACKERS] syslog_line_prefix

2009-09-28 Thread Robert Haas
On Mon, Sep 28, 2009 at 2:13 PM, Alvaro Herrera
alvhe...@commandprompt.com wrote:
 Robert Haas escribió:
 On Mon, Sep 28, 2009 at 1:07 PM, Alvaro Herrera
 alvhe...@commandprompt.com wrote:
  Tom Lane escribió:
  [ please trim the quoted material a bit, folks ]
 
  Magnus Hagander mag...@hagander.net writes:
   2009/9/28 Robert Haas robertmh...@gmail.com:
   The problem with having the syslogger send the data directly to an
   external process is that the external process might be unable to
   process the data as fast as syslogger is sending it.  I'm not sure
   exactly what will happen in that case, but it will definitely be bad.
 
  This is the same issue already raised with respect to syslog versus
  syslogger, ie, some people would rather lose log data than have the
  backends block waiting for it to be written.
 
  That could be made configurable; i.e. let the user choose whether to
  lose messages or to make everybody wait.

 I think the behavior I was proposing was neither drop nor wait,
 but buffer.  Not sure how people feel about that.

 Given an arbitrary increase in log rate during an arbitrary length of
 time, any buffer you keep will be filled.

True.  But the activity might be bursty.

...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] Rejecting weak passwords

2009-09-28 Thread Tom Lane
Marko Kreen mark...@gmail.com writes:
 So promoting the ENCRYPTED 'foo' as secure may lure users into
 false sense of security, and be lax against sniffing and logfile
 protection.

This argument is entirely irrelevant to the point.  Yes, ENCRYPTED
doesn't fix everything, but it is still good practice to use it
and most well-written tools will.  So having a weak-password detector
that can only work on non-encrypted passwords is going to not be
very helpful.

 IOW, having plaintext password in CREATE/ALTER time which can
 then checked for weaknesses is better that MD5 password, which
 actually does not increase security.

This is not acceptable and will not happen.  The case that ENCRYPTED
protects against is database superusers finding out other users'
original passwords, which is a security issue to the extent that those
users have used the same/similar passwords for other systems.
We're not going to give up protection for that in order to provide
an option to do weak-password checking in a place that simply isn't
the best place to do it anyway.

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] Rejecting weak passwords

2009-09-28 Thread Robert Haas
On Mon, Sep 28, 2009 at 2:00 PM, Marko Kreen mark...@gmail.com wrote:
 So promoting the ENCRYPTED 'foo' as secure may lure users into
 false sense of security, and be lax against sniffing and logfile
 protection.

ENCRYPTED prevents the user's password from being stolen by a modified server.

...Robert

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


Re: [HACKERS] patch: Review handling of MOVE and FETCH (ToDo)

2009-09-28 Thread Pavel Stehule
2009/9/28 John Naylor jcnay...@gmail.com:
 Pavel,

 It looks good. My last email didn't go to -hackers, since I wasn't
 subscribed. I had to resend to -hackers so there will be a link for
 the commitfest page. I think you might have to resend your latest
 patch to the list. Sorry!

nothing, patch attached

Pavel


 In any case, I will say it's ready for commiter.

 Thanks,
 John

 On Mon, Sep 28, 2009 at 2:07 AM, Pavel Stehule pavel.steh...@gmail.com 
 wrote:
 Hello

 I am sending actualised patch as per John comment.

 regards
 Pavel Stehule

 2009/9/26 John Naylor jcnay...@gmail.com:
 Hi,

 Sorry, I didn't notice the attachment on Pavel's email, otherwise I
 would have done this sooner! :)

 I just applied and tested the new patch. Everything works great.

 The only thing I would change now is some of the comments.

 1). On line 289, one of the regression test comments got copied:

 +   move forward in c;                --should be at '5'

 change to:

 +   move forward in c;                --should be at '1'

 2). Lines 79/80:

 +                                                                        
 errmsg(statement FETCH returns more rows.),
 +                                                                        
 errhint(Multirows fetch are not allowed in PL/pgSQL.)));

 This might sound better as statement FETCH returns multiple rows.,
 and Multirow FETCH is not allowed in PL/pgSQL.

 Everything else looks good to me.
 John


 Hi Selena and John,

 Pavel's latest patch seems to address all the issues you raised in
 your initial review.  Do you have any comments on this new revision?
 If you're happy that your issues have been resolved, please mark the
 patch as Ready for Committer.

 Cheers,
 BJ




*** ./doc/src/sgml/plpgsql.sgml.orig	2009-09-28 09:38:55.711469112 +0200
--- ./doc/src/sgml/plpgsql.sgml	2009-09-28 09:39:24.613468923 +0200
***
*** 2656,2670 
  
  para
   The options for the replaceabledirection/replaceable clause are
!  the same as for commandFETCH/, namely
   literalNEXT/,
   literalPRIOR/,
   literalFIRST/,
   literalLAST/,
   literalABSOLUTE/ replaceablecount/replaceable,
   literalRELATIVE/ replaceablecount/replaceable,
!  literalFORWARD/, or
!  literalBACKWARD/.
   Omitting replaceabledirection/replaceable is the same
   as specifying literalNEXT/.
   replaceabledirection/replaceable values that require moving
--- 2656,2670 
  
  para
   The options for the replaceabledirection/replaceable clause are
!  similar to commandFETCH/, namely
   literalNEXT/,
   literalPRIOR/,
   literalFIRST/,
   literalLAST/,
   literalABSOLUTE/ replaceablecount/replaceable,
   literalRELATIVE/ replaceablecount/replaceable,
!  literalFORWARD/ optional replaceablecount/replaceable | literalALL/ /optional, or
!  literalBACKWARD/ optional replaceablecount/replaceable | literalALL/ /optional.
   Omitting replaceabledirection/replaceable is the same
   as specifying literalNEXT/.
   replaceabledirection/replaceable values that require moving
***
*** 2678,2683 
--- 2678,2684 
  MOVE curs1;
  MOVE LAST FROM curs3;
  MOVE RELATIVE -2 FROM curs4;
+ MOVE FORWARD 2 FROM curs4;
  /programlisting
 /para
   /sect3
*** ./src/pl/plpgsql/src/gram.y.orig	2009-09-28 09:38:55.713470217 +0200
--- ./src/pl/plpgsql/src/gram.y	2009-09-28 11:00:53.811467762 +0200
***
*** 72,77 
--- 72,79 
  		  int until, const char *expected);
  static List*read_raise_options(void);
  
+ static PLpgSQL_stmt_fetch *complete_direction(PLpgSQL_stmt_fetch *fetch, bool *check_FROM);
+ 
  %}
  
  %expect 0
***
*** 178,183 
--- 180,186 
  		 * Keyword tokens
  		 */
  %token	K_ALIAS
+ %token	K_ALL
  %token	K_ASSIGN
  %token	K_BEGIN
  %token	K_BY
***
*** 1621,1626 
--- 1624,1635 
  
  		if (yylex() != ';')
  			yyerror(syntax error);
+ 			
+ 		if (fetch-returns_multiple_rows)
+ 			ereport(ERROR,
+ 	(errcode(ERRCODE_SYNTAX_ERROR),
+ 	 errmsg(statement FETCH returns multiple rows),
+ 	 errhint(Multirow FETCH is not allowed in PL/pgSQL.)));
  
  		fetch-lineno = $2;
  		fetch-rec		= rec;
***
*** 2252,2257 
--- 2261,2271 
  }
  
  
+ /*
+  * Read FETCH or MOVE statement direction. By default, 
+  * cursor will only move one row. To MOVE more than one row
+  * at a time see complete_direction(). 
+  */
  static PLpgSQL_stmt_fetch *
  read_fetch_direction(void)
  {
***
*** 2269,2274 
--- 2283,2289 
  	fetch-direction = FETCH_FORWARD;
  	fetch-how_many  = 1;
  	fetch-expr  = NULL;
+ 	fetch-returns_multiple_rows = false;
  
  	/*
  	 * Most of the direction keywords are not plpgsql keywords, so we
***
*** 2313,2323 
  	}
  	else if (pg_strcasecmp(yytext, forward) == 0)
  	{
! 		/* use defaults */
  	}
  	else 

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Josh Berkus

 There is another large problem, too.  The patch seems to have
 only half-baked support for global defaults (those not tied to a
 specific schema) --- it looks like you can put them in, but half
 of the code will ignore them or else fail while trying to use them.
 This isn't just a matter of a few missed cases while coding, I think.
 The generic issue that the code doesn't even think about addressing
 is which default should apply when there's potentially more than one
 applicable default?  

I thought the idea was to simply avoid that situation.  Maybe we want to
forget about global defaults if that's the case, and just do the ROLE
defaults.

I thought we were trying to keep this solution as simple as possible.
It's meant to be a simple feature for simple use cases.  I know we all
love making stuff as ornate and complex as possible around here, but
that kind of defeats the purpose of having DefaultACLs, as well as
setting the bar unreasonably high for Petr.Asking him to
future-filter-proof the feature assumes that there will be future
filters, which I'm not convinced there will.

I certainly haven't seen any good use case for having multiple
conflicting defaults.  In fact, I thought we'd agreed that any complex
cases would be better handled by PL scripts.

pg_dump support is required though.

-- 
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.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] Using results from INSERT ... RETURNING

2009-09-28 Thread Marko Tiikkaja

Robert Haas wrote:

Can you at least take a stab at it?  We can fix your grammar, but
guessing what's going on without documentation is hard.


With some help from David Fetter, I took another try at it.  I hope
someone finds this helpful.  I'm happy to answer any questions.

Regards,
Marko Tiikkaja
*** a/src/backend/executor/README
--- b/src/backend/executor/README
***
*** 25,38  There is a moderately intelligent scheme to avoid rescanning 
nodes
  unnecessarily (for example, Sort does not rescan its input if no parameters
  of the input have changed, since it can just reread its stored sorted data).
  
! The plan tree concept implements SELECT directly: it is only necessary to
! deliver the top-level result tuples to the client, or insert them into
! another table in the case of INSERT ... SELECT.  (INSERT ... VALUES is
! handled similarly, but the plan tree is just a Result node with no source
! tables.)  For UPDATE, the plan tree selects the tuples that need to be
! updated (WHERE condition) and delivers a new calculated tuple value for each
! such tuple, plus a junk (hidden) tuple CTID identifying the target tuple.
! The executor's top level then uses this information to update the correct
  tuple.  DELETE is similar to UPDATE except that only a CTID need be
  delivered by the plan tree.
  
--- 25,42 
  unnecessarily (for example, Sort does not rescan its input if no parameters
  of the input have changed, since it can just reread its stored sorted data).
  
! It is only necessary to deliver the top-level result tuples to the client.
! If the query is a SELECT, the topmost plan node is the output of the SELECT.
! If the query is a DML operation, a DML node is added to the top, which calls
! its child nodes to fetch the tuples.  If the DML operation has a RETURNING
! clause the node will output the projected tuples, otherwise it gives out
! dummy tuples until it has processed all tuples from its child nodes.  After
! that, it gives NULL.
! 
! Handling INSERT is pretty straightforward: the tuples returned from the
! subtree are inserted into the correct result relation.  In addition to the
! tuple value, UPDATE needs a junk (hidden) tuple CTID identifying the
! target tuple.  The DML node needs this information to update the correct
  tuple.  DELETE is similar to UPDATE except that only a CTID need be
  delivered by the plan tree.
  


-- 
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] syslog_line_prefix

2009-09-28 Thread Alvaro Herrera
Tom Lane escribió:
 Alvaro Herrera alvhe...@commandprompt.com writes:
  Tom Lane escribió:
  This is the same issue already raised with respect to syslog versus
  syslogger, ie, some people would rather lose log data than have the
  backends block waiting for it to be written.
 
  That could be made configurable; i.e. let the user choose whether to
  lose messages or to make everybody wait.
 
 Hmm, I guess I missed where you proposed an implementation that would
 support that?

syslog uses a nonblocking file descriptor without a retry loop to
implement their logic.  I see no reason we couldn't do that ourselves.
Mixing it with regular blocking code could turn out to be nontrivial,
but I don't think it's impossible.

-- 
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] [PATCH] DefaultACLs

2009-09-28 Thread Tom Lane
Josh Berkus j...@agliodbs.com writes:
 This isn't just a matter of a few missed cases while coding, I think.
 The generic issue that the code doesn't even think about addressing
 is which default should apply when there's potentially more than one
 applicable default?  

 I thought the idea was to simply avoid that situation.  Maybe we want to
 forget about global defaults if that's the case, and just do the ROLE
 defaults.

That seems like a pretty dead-end design.

 I thought we were trying to keep this solution as simple as possible.
 It's meant to be a simple feature for simple use cases.  I know we all
 love making stuff as ornate and complex as possible around here, but
 that kind of defeats the purpose of having DefaultACLs, as well as
 setting the bar unreasonably high for Petr.Asking him to
 future-filter-proof the feature assumes that there will be future
 filters, which I'm not convinced there will.

I already mentioned one case that there's longstanding demand for, which
is to instantiate the correct permissions on new partition child tables.

But more generally, this is a fairly large and complicated patch in
comparison to the reward, if the intention is that it will never support
anything more than the one case of IN SCHEMA foo filtering.

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] syslog_line_prefix

2009-09-28 Thread Andrew Dunstan



Alvaro Herrera wrote:

Tom Lane escribió:
  

Alvaro Herrera alvhe...@commandprompt.com writes:


Tom Lane escribió:
  

This is the same issue already raised with respect to syslog versus
syslogger, ie, some people would rather lose log data than have the
backends block waiting for it to be written.


That could be made configurable; i.e. let the user choose whether to
lose messages or to make everybody wait.
  

Hmm, I guess I missed where you proposed an implementation that would
support that?



syslog uses a nonblocking file descriptor without a retry loop to
implement their logic.  I see no reason we couldn't do that ourselves.
Mixing it with regular blocking code could turn out to be nontrivial,
but I don't think it's impossible.

  


Well, for CSV logs it's a complete non-starter. We go to quite a deal of 
trouble to ensure we don't miss messages, because if we do the CSVs will 
be hopelessly corrupted.


Frankly, if you're generating so much log output that blocking is going 
to become an issue you should probably just be using syslog on Unix anyway.



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] pg_hba.conf: samehost and samenet [REVIEW]

2009-09-28 Thread Stef Walter
Robert Haas wrote:
 So is this one Ready for Committer?

Here we go, I think this one is ready. In addition to previous patches,
it does:

 * Use some techniques from postfix for getting interface addresses.
   Couldn't use code outright, due to license incompatibilities.
 * Tested on Solaris, FreeBSD, Linux and Windows. As far as I can tell
   this should also work on Mac OS, HPUX and AIX, and probably others.
 * Added src/tools/ifaddrs/test_ifaddrs tool for testing interface
   address code.

Cheers,

Stef
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index ad4d084..e5152f4 100644
*** a/doc/src/sgml/client-auth.sgml
--- b/doc/src/sgml/client-auth.sgml
*** hostnossl  replaceabledatabase/replac
*** 244,249 
--- 244,255 
 support for IPv6 addresses.
/para
  
+   paraInstead of a replaceableCIDR-address/replaceable, you can specify 
+literalsamehost/literal to match any of the server's own IP addresses,
+or literalsamenet/literal to match any address in a subnet that the 
+server belongs to.
+   /para
+ 
para
 This field only applies to literalhost/literal,
 literalhostssl/literal, and literalhostnossl/ records.
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index e6f7db2..702971a 100644
*** a/src/backend/libpq/hba.c
--- b/src/backend/libpq/hba.c
*** check_db(const char *dbname, const char 
*** 512,517 
--- 512,608 
  	return false;
  }
  
+ /*
+  * Check to see if a connecting IP matches the address and netmask.
+  */
+ static bool
+ check_ip(SockAddr *raddr, struct sockaddr *addr, struct sockaddr *mask)
+ {
+ 	if (raddr-addr.ss_family == addr-sa_family)
+ 	{
+ 		/* Same address family */
+ 		if (!pg_range_sockaddr(raddr-addr, (struct sockaddr_storage*)addr, 
+ 		   (struct sockaddr_storage*)mask))
+ 			return false;
+ 	}
+ #ifdef HAVE_IPV6
+ 	else if (addr-sa_family == AF_INET 
+ 			 raddr-addr.ss_family == AF_INET6)
+ 	{
+ 		/*
+ 		 * Wrong address family.  We allow only one case: if the file
+ 		 * has IPv4 and the port is IPv6, promote the file address to
+ 		 * IPv6 and try to match that way.
+ 		 */
+ 		struct sockaddr_storage addrcopy,
+ 	maskcopy;
+ 
+ 		memcpy(addrcopy, addr, sizeof(addrcopy));
+ 		memcpy(maskcopy, mask, sizeof(maskcopy));
+ 		pg_promote_v4_to_v6_addr(addrcopy);
+ 		pg_promote_v4_to_v6_mask(maskcopy);
+ 
+ 		if (!pg_range_sockaddr(raddr-addr, addrcopy, maskcopy))
+ 			return false;
+ 	}
+ #endif   /* HAVE_IPV6 */
+ 	else
+ 	{
+ 		/* Wrong address family, no IPV6 */
+ 		return false;
+ 	}
+ 
+ 	return true;
+ }
+ 
+ typedef struct CheckNetwork {
+ 	NetMethod method;
+ 	SockAddr *raddr;
+ 	bool result;	
+ } CheckNetwork;
+ 
+ static void
+ callback_check_network(struct sockaddr *addr, struct sockaddr *netmask, void *data)
+ {
+ 	CheckNetwork *cn = data;
+ 	struct sockaddr_storage mask;
+ 
+ 	/* Already found a match */
+ 	if (cn-result)
+ 		return;
+ 
+ 	/* Make a fully 1's netmask of appropriate length */
+ 	if (cn-method == nmSameHost)
+ 	{
+ 		pg_sockaddr_cidr_mask(mask, NULL, addr-sa_family);
+ 		cn-result = check_ip(cn-raddr, addr, (struct sockaddr*)mask);
+ 	}
+ 
+ 	/* Use the netmask of the interface itself */
+ 	else
+ 	{
+ 		cn-result = check_ip(cn-raddr, addr, netmask);
+ 	}
+ }
+ 
+ static bool
+ check_same_host_or_net(SockAddr *raddr, NetMethod method)
+ {
+ 	CheckNetwork cn;
+ 	cn.method = method;
+ 	cn.raddr = raddr;
+ 	cn.result = false;
+ 
+ 	if (pg_foreach_ifaddr(callback_check_network, cn)  0)
+ 	{
+ 		ereport(LOG,
+ 		(errcode(ERRCODE_WARNING),
+ 		 errmsg(Error enumerating network interfaces)));
+ 		return false;
+ 	}
+ 
+ 	return cn.result;
+ }
  
  /*
   * Macros used to check and report on invalid configuration options.
*** parse_hba_line(List *line, int line_num,
*** 658,756 
  line_num, HbaFileName)));
  			return false;
  		}
- 		token = pstrdup(lfirst(line_item));
  
! 		/* Check if it has a CIDR suffix and if so isolate it */
! 		cidr_slash = strchr(token, '/');
! 		if (cidr_slash)
! 			*cidr_slash = '\0';
! 
! 		/* Get the IP address either way */
! 		hints.ai_flags = AI_NUMERICHOST;
! 		hints.ai_family = PF_UNSPEC;
! 		hints.ai_socktype = 0;
! 		hints.ai_protocol = 0;
! 		hints.ai_addrlen = 0;
! 		hints.ai_canonname = NULL;
! 		hints.ai_addr = NULL;
! 		hints.ai_next = NULL;
  
! 		ret = pg_getaddrinfo_all(token, NULL, hints, gai_result);
! 		if (ret || !gai_result)
  		{
! 			ereport(LOG,
! 	(errcode(ERRCODE_CONFIG_FILE_ERROR),
! 	 errmsg(invalid IP address \%s\: %s,
! 			token, gai_strerror(ret)),
! 	 errcontext(line %d of configuration file \%s\,
! line_num, HbaFileName)));
! 			if (cidr_slash)
! *cidr_slash = '/';
! 			if (gai_result)
! pg_freeaddrinfo_all(hints.ai_family, gai_result);
! 			return false;
  		}
  
! 		if (cidr_slash)
! 			*cidr_slash = '/';
! 
! 		memcpy(parsedline-addr, 

Re: [HACKERS] syslog_line_prefix

2009-09-28 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes:
 Alvaro Herrera wrote:
 syslog uses a nonblocking file descriptor without a retry loop to
 implement their logic.  I see no reason we couldn't do that ourselves.
 Mixing it with regular blocking code could turn out to be nontrivial,
 but I don't think it's impossible.

 Well, for CSV logs it's a complete non-starter. We go to quite a deal of 
 trouble to ensure we don't miss messages, because if we do the CSVs will 
 be hopelessly corrupted.

You could make it work if write() had all-or-nothing semantics, so that
you could write or discard a whole logical message at once.  But I don't
believe that's guaranteed for any interesting cases.

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] DefaultACLs

2009-09-28 Thread Josh Berkus
Tom,

 I thought the idea was to simply avoid that situation.  Maybe we want to
 forget about global defaults if that's the case, and just do the ROLE
 defaults.
 
 That seems like a pretty dead-end design.

Well, the whole purpose for DefaultACLs is to simplify administration
for the simplest use cases.  If we add a large host of conflicting
options, we haven't simplified stuff very much.

 I already mentioned one case that there's longstanding demand for, which
 is to instantiate the correct permissions on new partition child tables.

Wouldn't that be handled by inheritance?

 But more generally, this is a fairly large and complicated patch in
 comparison to the reward, if the intention is that it will never support
 anything more than the one case of IN SCHEMA foo filtering.

I thought we were doing ROLEs?

-- 
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.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] DefaultACLs

2009-09-28 Thread Tom Lane
Josh Berkus j...@agliodbs.com writes:
 But more generally, this is a fairly large and complicated patch in
 comparison to the reward, if the intention is that it will never support
 anything more than the one case of IN SCHEMA foo filtering.

 I thought we were doing ROLEs?

The owning-ROLE match is required, else you have issues with exactly
what the ACL really means.  What we're discussing is what other filters
might exist to determine which objects are affected.  The patch already
tries to handle the cases of all owned objects and all owned objects
in schema X, and I think it's inevitable that people will want other
cases.

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] syslog_line_prefix

2009-09-28 Thread Andrew Dunstan



Tom Lane wrote:

Andrew Dunstan and...@dunslane.net writes:
  

Alvaro Herrera wrote:


syslog uses a nonblocking file descriptor without a retry loop to
implement their logic.  I see no reason we couldn't do that ourselves.
Mixing it with regular blocking code could turn out to be nontrivial,
but I don't think it's impossible.
  


  
Well, for CSV logs it's a complete non-starter. We go to quite a deal of 
trouble to ensure we don't miss messages, because if we do the CSVs will 
be hopelessly corrupted.



You could make it work if write() had all-or-nothing semantics, so that
you could write or discard a whole logical message at once.  But I don't
believe that's guaranteed for any interesting cases.


  


Right. That's part of why we had to invent the chunking protocol between 
the backends and the syslogger, IIRC.


cheers

andrew

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


Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Josh Berkus
Tom,

 The owning-ROLE match is required, else you have issues with exactly
 what the ACL really means.  What we're discussing is what other filters
 might exist to determine which objects are affected.  The patch already
 tries to handle the cases of all owned objects and all owned objects
 in schema X, and I think it's inevitable that people will want other
 cases.

Yeah, I'm thinking we should back off from filters for 8.5; we could do
them for 8.6, maybe.  I'm one of the people who prefers a schema-based
system, but I'll do without one if it means we can keep things *simple*
(and get the feature in in 8.5).

I think trying to make this patch a panacea in the first iteration is
liable to backfire.  Especially since we're doing GRANT ALL ON at the
same time.

-- 
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.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_hba.conf: samehost and samenet [REVIEW]

2009-09-28 Thread Stef Walter
Whoops I missed this email...

Robert Haas wrote:
 Rereading the thread, it seems that the main question is whether there
 are any platforms that we support that have neither getifaddrs or
 SIOCGIFCONF, or where they don't work properly.

As far as I can tell, there are no non-ancient mainstream platforms that
we're missing here. As Tom suggested, I've looked over postfix, bind and
pcap and merged what I've learned into the (attached) samenet patch. I
believe we're hitting all the majors here:

 * Win32 using win_wsa2.dll
 * Modern versions of: Linux, BSD, Mac OS X, AIX using getifaddrs
 * Modern Solaris and HPUX using ioctl/SIOCGLIFCONF
 * Older unixes (BSD, Linux, Solaris, AIX) using ioctl/SIOCGIFCONF

SIOCGIFCONF doesn't return IPv6 information on certain platforms (such
as modern Solaris, or older Linux).

I believe we're covering every single Unix in use out there. I have
however only verified this assertion on open source OS's. I've also
verified that the SIOCGIFCONF method on Linux, BSD and Solaris, even
though they use more modern methods by default.

If a problem occurs with this code the src/tools/ifaddrs tool can be
used to diagnose the problem, and send in debugging feedback.

 By the way, in foreach_ifaddr_ifconf, what happens if the number of
 addresses is too large to fit in the arbitrary-size buffer you've
 chosen here?

The old approach was not a security vulnerability, and I find it hard to
believe that anyone would have had more than 10K of addresses. However
for the sake of completeness attached is a patch with dynamically sized
buffers. This adds some code complexity, but maybe someone out there
would have run into this (extremely) edge case.

I believe this patch to be complete, and am looking forward to review.

Cheers,

Stef

diff --git a/configure.in b/configure.in
index e545a1f..5182714 100644
*** a/configure.in
--- b/configure.in
*** AC_SUBST(OSSP_UUID_LIBS)
*** 969,975 
  ##
  
  dnl sys/socket.h is required by AC_FUNC_ACCEPT_ARGTYPES
! AC_CHECK_HEADERS([crypt.h dld.h fp_class.h getopt.h ieeefp.h langinfo.h poll.h pwd.h sys/ipc.h sys/poll.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/socket.h sys/shm.h sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h kernel/OS.h kernel/image.h SupportDefs.h])
  
  # At least on IRIX, cpp test for netinet/tcp.h will fail unless
  # netinet/in.h is included first.
--- 969,975 
  ##
  
  dnl sys/socket.h is required by AC_FUNC_ACCEPT_ARGTYPES
! AC_CHECK_HEADERS([crypt.h dld.h fp_class.h getopt.h ieeefp.h langinfo.h poll.h pwd.h sys/ipc.h sys/poll.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/socket.h sys/shm.h sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h kernel/OS.h kernel/image.h SupportDefs.h ifaddrs.h sys/sockio.h])
  
  # At least on IRIX, cpp test for netinet/tcp.h will fail unless
  # netinet/in.h is included first.
*** PGAC_VAR_INT_TIMEZONE
*** 1148,1154 
  AC_FUNC_ACCEPT_ARGTYPES
  PGAC_FUNC_GETTIMEOFDAY_1ARG
  
! AC_CHECK_FUNCS([cbrt dlopen fcvt fdatasync getpeereid getpeerucred getrlimit memmove poll pstat readlink setproctitle setsid sigprocmask symlink sysconf towlower utime utimes waitpid wcstombs])
  
  # posix_fadvise() is a no-op on Solaris, so don't incur function overhead
  # by calling it, 2009-04-02
--- 1148,1154 
  AC_FUNC_ACCEPT_ARGTYPES
  PGAC_FUNC_GETTIMEOFDAY_1ARG
  
! AC_CHECK_FUNCS([cbrt dlopen fcvt fdatasync getpeereid getpeerucred getrlimit memmove poll pstat readlink setproctitle setsid sigprocmask symlink sysconf towlower utime utimes waitpid wcstombs getifaddrs])
  
  # posix_fadvise() is a no-op on Solaris, so don't incur function overhead
  # by calling it, 2009-04-02
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index ad4d084..e5152f4 100644
*** a/doc/src/sgml/client-auth.sgml
--- b/doc/src/sgml/client-auth.sgml
*** hostnossl  replaceabledatabase/replac
*** 244,249 
--- 244,255 
 support for IPv6 addresses.
/para
  
+   paraInstead of a replaceableCIDR-address/replaceable, you can specify 
+literalsamehost/literal to match any of the server's own IP addresses,
+or literalsamenet/literal to match any address in a subnet that the 
+server belongs to.
+   /para
+ 
para
 This field only applies to literalhost/literal,
 literalhostssl/literal, and literalhostnossl/ records.
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index e6f7db2..702971a 100644
*** a/src/backend/libpq/hba.c
--- b/src/backend/libpq/hba.c
*** check_db(const char *dbname, const char 
*** 512,517 
--- 512,608 
  	return false;
  }
  
+ /*
+  * Check to see if a connecting IP matches the address and netmask.
+  */
+ static bool
+ check_ip(SockAddr *raddr, struct sockaddr *addr, struct sockaddr *mask)
+ {
+ 	if (raddr-addr.ss_family == addr-sa_family)
+ 	{
+ 		/* Same address family */
+ 	

[HACKERS] Small patch for README

2009-09-28 Thread Devrim GÜNDÜZ
Hi,

Attached is a small doc patch for README at top level, which updates
links for some projects. It could be backpatched, too.

Regards,
-- 
Devrim GÜNDÜZ, RHCE
Command Prompt - http://www.CommandPrompt.com 
devrim~gunduz.org, devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr
http://www.gunduz.org  Twitter: http://twitter.com/devrimgunduz
Index: README
===
RCS file: /projects/cvsroot/pgsql/README,v
retrieving revision 1.32
diff -c -r1.32 README
*** README	24 Jul 2006 16:55:59 -	1.32
--- README	28 Sep 2009 21:08:55 -
***
*** 12,24 
  PostgreSQL has many language interfaces including some of the more
  common listed below:
  
! C++ - http://thaiopensource.org/development/libpqxx/
  JDBC - http://jdbc.postgresql.org
! ODBC - http://odbc.postgresql.org
  Perl - http://search.cpan.org/~dbdpg/
  PHP - http://www.php.net
  Python - http://www.initd.org/
! Ruby - http://ruby.scripting.ca/postgres/
  
  Other language binding are available from a variety of contributing
  parties.
--- 12,24 
  PostgreSQL has many language interfaces including some of the more
  common listed below:
  
! C++ - http://pqxx.org/development/libpqxx/
  JDBC - http://jdbc.postgresql.org
! ODBC - http://pgfoundry.org/projects/psqlodbc/
  Perl - http://search.cpan.org/~dbdpg/
  PHP - http://www.php.net
  Python - http://www.initd.org/
! Ruby - http://pqxx.org/development/libpqxx/
  
  Other language binding are available from a variety of contributing
  parties.


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


[HACKERS] Buffer usage in EXPLAIN and pg_stat_statements (review)

2009-09-28 Thread Euler Taveira de Oliveira
Hi Itagaki-san,

I'm reviewing your patch. Your patch is in good shape. It applies cleanly. All
of the things are built as intended (including the two contrib modules). It
doesn't include docs but I wrote it. Basically, I produced another patch (that
are attached) correcting some minor gripes; docs are included too. The
comments are in-line.

   static bool auto_explain_log_analyze = false;
   static bool auto_explain_log_verbose = false;
 + static bool auto_explain_log_buffer = false;
Rename it to auto_explain_log_buffers. That's because I renamed the option for
plural form. See above.

   es.verbose = auto_explain_log_verbose;
 + es.buffer = auto_explain_log_buffer;
Change this check to look at es.analyze too. So the es.buffers will only be
enabled iif the es.analyze is enabled too.


 + /* Build a tuple descriptor for our result type */
 + if (get_call_result_type(fcinfo, NULL, tupdesc) != TYPEFUNC_COMPOSITE)
 + elog(ERROR, return type must be a row type);
 + 
   per_query_ctx = rsinfo-econtext-ecxt_per_query_memory;
   oldcontext = MemoryContextSwitchTo(per_query_ctx);
   
 ! tupdesc = CreateTupleDescCopy(tupdesc);
   
Out of curiosity, any reason for this change?

   else if (strcmp(opt-defname, costs) == 0)
   es.costs = defGetBoolean(opt);
 + else if (strcmp(opt-defname, buffer) == 0)
 + es.buffer = defGetBoolean(opt);
I decided to change buffer to buffers. That's because we already have
costs and the statistics will not be about one kind of buffer so plural form
sounds more natural.

 + if (es-format == EXPLAIN_FORMAT_TEXT)
 + {
 + appendStringInfo(es-str,  (gets=%ld reads=%ld 
 temp=%ld),
 + num_gets, num_reads, num_temp);
Rename gets and reads to hit and read. Maybe we could prefix it with
buf_ or something else.

Rename num_gets and num_reads to num_hit and num_read. The later
terminology is used all over the code.

 + }
 + else
 + {
 + ExplainPropertyLong(Buffer Gets, num_gets, 
 es);
 + ExplainPropertyLong(Buffer Reads, num_reads, 
 es);
 + ExplainPropertyLong(Buffer Temp, num_temp, 
 es);
I didn't like these terminologies. I came up with Hit Buffers, Read
Buffers, and Temp Buffers. I confess that I don't like the last ones.
Read Buffers? We're reading from disk blocks. Read Blocks? Read Disk
Blocks? Read Data Blocks?
Temp Buffers? It could be temporary sort files, hash files (?), or temporary
relations. Hit Local Buffers? Local Buffers? Hit Temp Buffers?

   #include parser/parsetree.h
 + #include storage/buf_internals.h
It's not used. Removed.

 + CurrentInstrument = instr-prev;
 + }
 + else
 + elog(WARNING, Instrumentation stack is broken);
WARNING? I changed it to DEBUG2 and return immediately (as it does some lines
of code above).

 + /* for log_[parser|planner|executor|statement]_stats */
 + static long GlobalReadBufferCount;
 + static long GlobalReadLocalBufferCount;
 + static long GlobalBufferHitCount;
 + static long GlobalLocalBufferHitCount;
 + static long GlobalBufferFlushCount;
 + static long GlobalLocalBufferFlushCount;
 + static long GlobalBufFileReadCount;
 + static long GlobalBufFileWriteCount;
 + 
I'm not sure if this is the right place for these counters. Maybe we should
put it in buf_init.c. Ideas?

   boolcosts;  /* print costs */
 + boolbuffer; /* print buffer stats */
Rename it to buffers.

 + /* Buffer usage */
 + longbuffer_gets;/* # of buffer reads */
 + longbuffer_reads;   /* # of disk reads */
 + longbuffer_temp;/* # of temp file reads */
Rename them to buffers_hit, buffers_read, and buffers_temp.

I didn't test this changes with big queries because I don't have some at
hand right now. Also, I didn't notice any slowdowns caused by the patch.
Comments? If none, it is ready for a committer.


-- 
  Euler Taveira de Oliveira
  http://www.timbira.com/



buffer_usage-20090928.diff.gz
Description: GNU Zip compressed 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] Rejecting weak passwords

2009-09-28 Thread marcin mank
 The case that ENCRYPTED
 protects against is database superusers finding out other users'
 original passwords, which is a security issue to the extent that those
 users have used the same/similar passwords for other systems.

I just want to note that md5 is not much of a protection against this
case these days. Take a look at this:
http://www.golubev.com/hashgpu.htm

It takes about 32 hours to brute force all passwords from [a-zA-Z0-9]
of up to 8 chars in length.

Maybe it is time to look at something like bcrypt.
http://chargen.matasano.com/chargen/2007/9/7/enough-with-the-rainbow-tables-what-you-need-to-know-about-s.html

Greetings
Marcin

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


Re: [HACKERS] TODO item: Allow more complex user/database default GUC settings

2009-09-28 Thread Bernd Helmle



--On 27. September 2009 21:59:37 -0400 Robert Haas robertmh...@gmail.com 
wrote:



Bernd,

Can you review this new version and mark this as Ready for Committer
if it looks OK, or else respond with comments and set it back to
Waiting on Author?


Seems Alvaro forgot to include pg_db_role_setting.h, it doesn't compile 
anymore with this error:


catalog.c:34:40: error: catalog/pg_db_role_setting.h: No such file or 
directory

catalog.c: In function ‘IsSharedRelation’:
catalog.c:311: error: ‘DbRoleSettingRelationId’ undeclared (first use 
in this function)



--
Thanks

Bernd

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


Re: [HACKERS] TODO item: Allow more complex user/database default GUC settings

2009-09-28 Thread decibel

On Sep 27, 2009, at 9:19 PM, Tom Lane wrote:

What we seem to be lacking in this case is a good tech-speak
option for the underlying catalog name.  I'm still not happy
with having a catalog and a view that are exactly the same
except for s, especially since as Alvaro notes that won't
lead to desirable tab-completion behavior.  OTOH, we have
survived with pg_index vs pg_indexes, so maybe it wouldn't
kill us.



Another option is to revisit the set of system views (http:// 
pgfoundry.org/projects/newsysviews/). IIRC there was some other  
recent reason we wanted to do that.

--
Decibel!, aka Jim C. Nasby, Database Architect  deci...@decibel.org
Give your computer some brain candy! www.distributed.net Team #1828



--
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] Rejecting weak passwords

2009-09-28 Thread Josh Berkus

 It takes about 32 hours to brute force all passwords from [a-zA-Z0-9]
 of up to 8 chars in length.

That would be a reason to limit the number of failed connection attempts
from a single source, then, rather than a reason to change the hash
function.

Hmmm, that would be a useful, easy (I think) security feature: add a GUC
for failed_logins_allowed.

-- 
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.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] TODO item: Allow more complex user/database default GUC settings

2009-09-28 Thread Alvaro Herrera
Bernd Helmle escribió:

 Seems Alvaro forgot to include pg_db_role_setting.h, it doesn't
 compile anymore with this error:

Huh, you're right, I did :-(  Let me unpack the laptop ...

-- 
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] Rejecting weak passwords

2009-09-28 Thread Joshua D. Drake
On Mon, 2009-09-28 at 15:52 -0700, Josh Berkus wrote:
  It takes about 32 hours to brute force all passwords from [a-zA-Z0-9]
  of up to 8 chars in length.
 
 That would be a reason to limit the number of failed connection attempts
 from a single source, then, rather than a reason to change the hash
 function.
 
 Hmmm, that would be a useful, easy (I think) security feature: add a GUC
 for failed_logins_allowed.

Why a GUC, can't we just use ALTER ROLE (or ALTER DATABASE)?

Joshua D. Drake


-- 
PostgreSQL.org Major Contributor
Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564
Consulting, Training, Support, Custom Development, Engineering
If the world pushes look it in the eye and GRR. Then push back harder. - 
Salamander


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


Re: [HACKERS] TODO item: Allow more complex user/database default GUC settings

2009-09-28 Thread Alvaro Herrera
Bernd Helmle escribió:

 Seems Alvaro forgot to include pg_db_role_setting.h, it doesn't
 compile anymore with this error:

Here they are.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
/*
 * pg_db_role_setting.c
 *		Routines to support manipulation of the pg_db_role_setting relation
 *
 * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
 * Portions Copyright (c) 1994, Regents of the University of California
 *
 * IDENTIFICATION
 *		$PostgreSQL$
 */
#include postgres.h

#include access/genam.h
#include access/heapam.h
#include access/htup.h
#include access/skey.h
#include catalog/indexing.h
#include catalog/pg_db_role_setting.h
#include utils/fmgroids.h
#include utils/rel.h
#include utils/tqual.h

void
AlterSetting(Oid databaseid, Oid roleid, VariableSetStmt *setstmt)
{
	char	   *valuestr;
	HeapTuple	tuple;
	Relation	rel;
	ScanKeyData scankey[2];
	SysScanDesc scan;

	valuestr = ExtractSetVariableArgs(setstmt);

	/* Get the old tuple, if any. */

	rel = heap_open(DbRoleSettingRelationId, RowExclusiveLock);
	ScanKeyInit(scankey[0],
Anum_pg_db_role_setting_setdatabase,
BTEqualStrategyNumber, F_OIDEQ,
ObjectIdGetDatum(databaseid));
	ScanKeyInit(scankey[1],
Anum_pg_db_role_setting_setrole,
BTEqualStrategyNumber, F_OIDEQ,
ObjectIdGetDatum(roleid));
	scan = systable_beginscan(rel, DbRoleSettingDatidRolidIndexId, true,
			  SnapshotNow, 2, scankey);
	tuple = systable_getnext(scan);

	/*
	 * There are three cases:
	 *
	 * - in RESET ALL, simply delete the pg_db_role_setting tuple (if any)
	 *
	 * - in other commands, if there's a tuple in pg_db_role_setting, update it;
	 *   if it ends up empty, delete it
	 *
	 * - otherwise, insert a new pg_db_role_setting tuple, but only if the
	 *   command is not RESET
	 */
	if (setstmt-kind == VAR_RESET_ALL)
	{
		if (HeapTupleIsValid(tuple))
			simple_heap_delete(rel, tuple-t_self);
	}
	else if (HeapTupleIsValid(tuple))
	{
		Datum		repl_val[Natts_pg_db_role_setting];
		bool		repl_null[Natts_pg_db_role_setting];
		bool		repl_repl[Natts_pg_db_role_setting];
		HeapTuple	newtuple;
		Datum		datum;
		bool		isnull;
		ArrayType  *a;

		memset(repl_repl, false, sizeof(repl_repl));
		repl_repl[Anum_pg_db_role_setting_setconfig - 1] = true;
		repl_null[Anum_pg_db_role_setting_setconfig - 1] = false;

		/* Extract old value of setconfig */
		datum = heap_getattr(tuple, Anum_pg_db_role_setting_setconfig,
			 RelationGetDescr(rel), isnull);
		a = isnull ? NULL : DatumGetArrayTypeP(datum);

		/* Update (valuestr is NULL in RESET cases) */
		if (valuestr)
			a = GUCArrayAdd(a, setstmt-name, valuestr);
		else
			a = GUCArrayDelete(a, setstmt-name);

		if (a)
		{
			repl_val[Anum_pg_db_role_setting_setconfig - 1] =
PointerGetDatum(a);

			newtuple = heap_modify_tuple(tuple, RelationGetDescr(rel),
		 repl_val, repl_null, repl_repl);
			simple_heap_update(rel, tuple-t_self, newtuple);

			/* Update indexes */
			CatalogUpdateIndexes(rel, newtuple);
		}
		else
			simple_heap_delete(rel, tuple-t_self);
	}
	else if (valuestr)
	{
		/* non-null valuestr means it's not RESET, so insert a new tuple */
		HeapTuple	newtuple;
		Datum		values[Natts_pg_db_role_setting];
		bool		nulls[Natts_pg_db_role_setting];
		ArrayType  *a;

		memset(nulls, false, sizeof(nulls));
		
		a = GUCArrayAdd(NULL, setstmt-name, valuestr);

		values[Anum_pg_db_role_setting_setdatabase - 1] =
			ObjectIdGetDatum(databaseid);
		values[Anum_pg_db_role_setting_setrole - 1] = ObjectIdGetDatum(roleid);
		values[Anum_pg_db_role_setting_setconfig - 1] = PointerGetDatum(a);
		newtuple = heap_form_tuple(RelationGetDescr(rel), values, nulls);

		simple_heap_insert(rel, newtuple);

		/* Update indexes */
		CatalogUpdateIndexes(rel, newtuple);
	}

	systable_endscan(scan);

	/* Close pg_db_role_setting, but keep lock till commit */
	heap_close(rel, NoLock);
}

/*
 * Drop some settings from the catalog.  These can be for a particular
 * database, or for a particular role.  (It is of course possible to do both
 * too, but it doesn't make sense for current uses.)
 */
void
DropSetting(Oid databaseid, Oid roleid)
{
	Relation		relsetting;
	HeapScanDesc	scan;
	ScanKeyData		keys[2];
	HeapTuple		tup;
	intnumkeys = 0;

	relsetting = heap_open(DbRoleSettingRelationId, RowExclusiveLock);

	if (OidIsValid(databaseid))
	{
		ScanKeyInit(keys[numkeys],
	Anum_pg_db_role_setting_setdatabase,
	BTEqualStrategyNumber,
	F_OIDEQ,
	ObjectIdGetDatum(databaseid));
		numkeys++;
	}
	if (OidIsValid(roleid))
	{
		ScanKeyInit(keys[numkeys],
	Anum_pg_db_role_setting_setrole,
	BTEqualStrategyNumber,
	F_OIDEQ,
	ObjectIdGetDatum(roleid));
		numkeys++;
	}

	scan = heap_beginscan(relsetting, SnapshotNow, numkeys, keys);
	while (HeapTupleIsValid(tup = heap_getnext(scan, ForwardScanDirection)))
	{
		simple_heap_delete(relsetting, tup-t_self);
	}
	heap_endscan(scan);

	heap_close(relsetting, 

Re: [HACKERS] Rejecting weak passwords

2009-09-28 Thread Jeff Davis
On Mon, 2009-09-28 at 15:52 -0700, Josh Berkus wrote:
  It takes about 32 hours to brute force all passwords from [a-zA-Z0-9]
  of up to 8 chars in length.
 
 That would be a reason to limit the number of failed connection attempts
 from a single source, then, rather than a reason to change the hash
 function.

That doesn't solve the problem of an administrator brute-forcing your password.

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] Rejecting weak passwords

2009-09-28 Thread Andrew Dunstan



Jeff Davis wrote:

On Mon, 2009-09-28 at 15:52 -0700, Josh Berkus wrote:
  

It takes about 32 hours to brute force all passwords from [a-zA-Z0-9]
of up to 8 chars in length.
  

That would be a reason to limit the number of failed connection attempts
from a single source, then, rather than a reason to change the hash
function.



That doesn't solve the problem of an administrator brute-forcing your password.



  


Indeed. These brute force checkers aren't checking them by actually 
trying the connection.


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] Rejecting weak passwords

2009-09-28 Thread Tom Lane
Josh Berkus j...@agliodbs.com writes:
 Hmmm, that would be a useful, easy (I think) security feature: add a GUC
 for failed_logins_allowed.

And the counts would be tracked and enforced where?

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] DefaultACLs

2009-09-28 Thread Tom Lane
Josh Berkus j...@agliodbs.com writes:
 I think trying to make this patch a panacea in the first iteration is
 liable to backfire.

I did not suggest any such thing --- the current scope of functionality
is fine by me for a first cut.  What I *am* saying is that we had better
have some idea of how we are going to extend it when (not if) we try
to extend it.  Otherwise we are likely to find we painted ourselves
into a corner.

 Especially since we're doing GRANT ALL ON at the same time.

That's another patch that has an *excellent* chance of getting rejected
on pretty much the same grounds.

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] DefaultACLs

2009-09-28 Thread Cathy Mullican
On 9/28/09, Josh Berkus j...@agliodbs.com wrote

  I already mentioned one case that there's longstanding demand for, which
  is to instantiate the correct permissions on new partition child tables.

 Wouldn't that be handled by inheritance?


I wish, but no:
http://www.postgresql.org/docs/current/interactive/ddl-inherit.html
The first paragraph under 5.8.1 Caveats:
Table access permissions are not automatically inherited. Therefore, a user
attempting to access a parent table must either have permissions to do the
same operation on all its child tables as well, or must use the
ONLYnotation. When adding a new child table to an existing inheritance
hierarchy, be careful to grant all the needed permissions on it.


Re: [HACKERS] Rejecting weak passwords

2009-09-28 Thread Tom Lane
marcin mank marcin.m...@gmail.com writes:
 The case that ENCRYPTED
 protects against is database superusers finding out other users'
 original passwords, which is a security issue to the extent that those
 users have used the same/similar passwords for other systems.

 I just want to note that md5 is not much of a protection against this
 case these days. Take a look at this:
 http://www.golubev.com/hashgpu.htm

 It takes about 32 hours to brute force all passwords from [a-zA-Z0-9]
 of up to 8 chars in length.

Yeah, but that will find you a password that hashes to the same thing.
Not necessarily the same password.  It'll get you into the Postgres
DB just fine, which you don't care about because you're already a
superuser there.  It won't necessarily get you into the assumed
third-party systems.

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] Buffer usage in EXPLAIN and pg_stat_statements (review)

2009-09-28 Thread Itagaki Takahiro

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

 I'm reviewing your patch. Your patch is in good shape. It applies cleanly. All
 of the things are built as intended (including the two contrib modules). It
 doesn't include docs but I wrote it. Basically, I produced another patch (that
 are attached) correcting some minor gripes; docs are included too. The
 comments are in-line.

Thanks. Except choice of words, can I think the basic architecture of
the patch is ok? The codes to handle global variables like ReadBufferCount
and GlobalReadBufferCount could be rewrite in cleaner way if we could
drop supports of log_{parser|planner|executor|statement}_stats.

  +   /* Build a tuple descriptor for our result type */
  +   if (get_call_result_type(fcinfo, NULL, tupdesc) != TYPEFUNC_COMPOSITE)
  +   elog(ERROR, return type must be a row type);
  + 
  per_query_ctx = rsinfo-econtext-ecxt_per_query_memory;
  oldcontext = MemoryContextSwitchTo(per_query_ctx);

  !   tupdesc = CreateTupleDescCopy(tupdesc);

 Out of curiosity, any reason for this change?

That's because the new code is cleaner, I think. Since the result tuple
type is defined in OUT parameters, we don't have to re-define the result
with CreateTemplateTupleDesc().

  +   appendStringInfo(es-str,  (gets=%ld reads=%ld 
  temp=%ld),
  +   num_gets, num_reads, num_temp);
 Rename gets and reads to hit and read. Maybe we could prefix it with
 buf_ or something else.
 
 Rename num_gets and num_reads to num_hit and num_read. The later
 terminology is used all over the code.

We should define the meanings of get and hit before rename them.
I'd like to propose the meanings as following:
* get  : number of page access (= hit + read)
* hit  : number of cache read (no disk read)
* read : number of disk read (= number of read() calls)

But there are some confusions in postgres; ReadBufferCount and
BufferHitCount are used for get and hit, but heap_blks_read
and heap_blks_hit are used for read and hit in pg_statio_all_tables.
Can I rename ReadBufferCount to BufferGetCount? And which values should
we show in EXPLAIN and pg_stat_statements? (two of the three are enough)

 I didn't like these terminologies. I came up with Hit Buffers, Read
 Buffers, and Temp Buffers. I confess that I don't like the last ones.
 Read Buffers? We're reading from disk blocks. Read Blocks? Read Disk
 Blocks? Read Data Blocks?
 Temp Buffers? It could be temporary sort files, hash files (?), or temporary
 relations. Hit Local Buffers? Local Buffers? Hit Temp Buffers?

I borrowed the terms Buffer Gets and Buffer Reads from STATSPACK report
in Oracle Database. But I'm willing to rename them if appropriate.
http://www.oracle.com/apps_benchmark/doc/awrrpt_20090325b_900.html#600

Presently Temp Buffers contains temporary sort files, hash files, and
materialized executor plan. Local buffer statistics for temp tables are
not included here but merged with shared buffer statistics. Are there
any better way to group them?

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] Reworks for Access Control facilities (r2311)

2009-09-28 Thread Stephen Frost
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
 BTW, I raised a few issues. Do you have any opinions?

Certainly, though they're my opinions and I don't know if the committers
will agree, but I suspect they will.

 * deployment of the source code
 
 The current patch implements all the access control abstractions at the
 src/backend/security/access_control.c. Its size is about 4,500 lines
 which includs source comments.
 It is an approach to sort out a series of functionalities into a single
 big file, such as aclchk.c. One other approach is to put these codes in
 the short many files deployed in a directory, such as backend/catalog/*.
 Which is the preferable in PostgreSQL?

A single, larger file, as implemented, is preferable, I believe.

 * pg_class_ownercheck() in EnableDisableRule()
 
 As I mentioned in the another message, pg_class_ownercheck() in the
 EnableDisableRule() is redundant.
 
 The EnableDisableRule() is called from ATExecEnableDisableRule() only,
 and it is also called from the ATExecCmd() with AT_EnableRule,
 AT_EnableAlwaysRule, AT_EnableReplicaRule and AT_DisableRule.
 In this path, ATPrepCmd() already calls ATSimplePermissions() which
 also calls pg_class_ownercheck() for the target.
 
 I don't think it is necessary to check ownership of the relation twice.
 My opinion is to remove the checks from the EnableDisableRule() and
 the ac_rule_toggle() is also removed from the patch.
 It does not have any compatibility issue.
 
 Any comments?

I agree that we don't need to check the ownership twice.  You might
check if there was some history to having both checks (perhaps there was
another code path before which didn't check before calling
EnableDisableRule()?).  I'd feel alot more comfortable removing the
check if we can show why it was there originally as well as why it's not
needed now.

I'm working on a more comprehensive review, but wanted to answer these
questions first.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] [PATCH] Reworks for Access Control facilities (r2311)

2009-09-28 Thread KaiGai Kohei
Stephen Frost wrote:
 * KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
 BTW, I raised a few issues. Do you have any opinions?
 
 Certainly, though they're my opinions and I don't know if the committers
 will agree, but I suspect they will.

Thanks for your comments.

 * deployment of the source code

 The current patch implements all the access control abstractions at the
 src/backend/security/access_control.c. Its size is about 4,500 lines
 which includs source comments.
 It is an approach to sort out a series of functionalities into a single
 big file, such as aclchk.c. One other approach is to put these codes in
 the short many files deployed in a directory, such as backend/catalog/*.
 Which is the preferable in PostgreSQL?
 
 A single, larger file, as implemented, is preferable, I believe.

OK,

 * pg_class_ownercheck() in EnableDisableRule()

 As I mentioned in the another message, pg_class_ownercheck() in the
 EnableDisableRule() is redundant.

 The EnableDisableRule() is called from ATExecEnableDisableRule() only,
 and it is also called from the ATExecCmd() with AT_EnableRule,
 AT_EnableAlwaysRule, AT_EnableReplicaRule and AT_DisableRule.
 In this path, ATPrepCmd() already calls ATSimplePermissions() which
 also calls pg_class_ownercheck() for the target.

 I don't think it is necessary to check ownership of the relation twice.
 My opinion is to remove the checks from the EnableDisableRule() and
 the ac_rule_toggle() is also removed from the patch.
 It does not have any compatibility issue.

 Any comments?
 
 I agree that we don't need to check the ownership twice.  You might
 check if there was some history to having both checks (perhaps there was
 another code path before which didn't check before calling
 EnableDisableRule()?).  I'd feel alot more comfortable removing the
 check if we can show why it was there originally as well as why it's not
 needed now.

I checked history of the repository, and this commit adds EnableDisableRule().

* Changes pg_trigger and extend pg_rewrite in order to allow triggers and
  Jan Wieck [Mon, 19 Mar 2007 23:38:32 + (23:38 +)]
  
http://git.postgresql.org/gitweb?p=postgresql.git;a=commitdiff;h=31e6bde46c0594c6f8bb82606c49f93295f1195c#patch8

The corresponding AT_ command calls ATSimplePermissions() from ATPrepCmd(),
and ATExecCmd() is the only caller from the begining.

It seems to me this redundant check does not have any explicit reason.
I think it is harmless to remove this pg_class_ownercheck() from here.

 I'm working on a more comprehensive review, but wanted to answer these
 questions first.

Thanks for your efforts.
I'm looking forward to see rest of the comments.
-- 
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] [PATCH] DefaultACLs

2009-09-28 Thread Robert Haas
On Mon, Sep 28, 2009 at 1:32 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 The generic issue that the code doesn't even think about addressing
 is which default should apply when there's potentially more than one
 applicable default?  As long as there's only global and per-schema
 defaults, it's not too hard to decide that the latter take precedence
 over the former; but I have no idea what we're going to do in order
 to add any other features.  This seems like a sufficiently big
 conceptual issue that it had better be resolved now, even if the first
 version of the patch doesn't really need to deal with it.

I haven't read the patch, but it seems like one possible solution to
this problem would be to declare that any any DEFAULT PRIVILEGES you
set are cumulative.  If you configure a global default, a per-schema
default, a default for tables whose names begin with the letter q, and
a default for tables created between midnight and 4am, then a table
called quux created in that schema at 2:30 in the morning will get the
union of all four sets of privileges.

...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] Buffer usage in EXPLAIN and pg_stat_statements (review)

2009-09-28 Thread Euler Taveira de Oliveira
Itagaki Takahiro escreveu:
 Thanks. Except choice of words, can I think the basic architecture of
 the patch is ok? The codes to handle global variables like ReadBufferCount
 and GlobalReadBufferCount could be rewrite in cleaner way if we could
 drop supports of log_{parser|planner|executor|statement}_stats.
 
Yes, it is. I'm afraid someone is relying on that piece of code. We can ask
people if it is ok to deprecated it; but it should be removed after 2 releases
or so. BTW, Isn't it make sense to move the Global* variables to buf_init.c,
is it?

 We should define the meanings of get and hit before rename them.
 I'd like to propose the meanings as following:
 * get  : number of page access (= hit + read)
 * hit  : number of cache read (no disk read)
 * read : number of disk read (= number of read() calls)
 
+1.

 But there are some confusions in postgres; ReadBufferCount and
 BufferHitCount are used for get and hit, but heap_blks_read
 and heap_blks_hit are used for read and hit in pg_statio_all_tables.
I see. :(

 Can I rename ReadBufferCount to BufferGetCount? And which values should
 we show in EXPLAIN and pg_stat_statements? (two of the three are enough)
 
Do you want to include number of page access in the stats? If not, we don't
need to rename the variables for now (maybe a separate patch?). And IMHO we
should include hit and read because get is just a simple math.

 I borrowed the terms Buffer Gets and Buffer Reads from STATSPACK report
 in Oracle Database. But I'm willing to rename them if appropriate.
 http://www.oracle.com/apps_benchmark/doc/awrrpt_20090325b_900.html#600
 
Hmm... I don't have a strong opinion about those names as I said. So if you
want to revert the old names...

 Presently Temp Buffers contains temporary sort files, hash files, and
 materialized executor plan. Local buffer statistics for temp tables are
 not included here but merged with shared buffer statistics. Are there
 any better way to group them?
 
Are you sure? Looking at ReadBuffer_common() function I see an 'if
(isLocalBuf)' test.

As I said your patch is in good shape and if we solve those terminologies, it
is done for a committer.

Would you care to submit another patch if you want to do some modifications?


-- 
  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] [PATCH] DefaultACLs

2009-09-28 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 I haven't read the patch, but it seems like one possible solution to
 this problem would be to declare that any any DEFAULT PRIVILEGES you
 set are cumulative.  If you configure a global default, a per-schema
 default, a default for tables whose names begin with the letter q, and
 a default for tables created between midnight and 4am, then a table
 called quux created in that schema at 2:30 in the morning will get the
 union of all four sets of privileges.

Hmm ... interesting proposal.  Simple to understand and simple to
implement, which are both to the good.  I'm not clear though on whether
this behavior would be useful in practice.  Any comments from those
who've been asking for default ACLs?

One potential trouble spot is that presumably the built-in default
privileges (eg, PUBLIC EXECUTE for functions) would *not* cumulate
with user-specified defaults.  So you'd have a behavior where a
function would not get PUBLIC EXECUTE automatically if it matched
any of the available defaults, but would get it if it managed to
miss matching them all.  I am not sure if that's bad or not, but
it seems kind of inconsistent.

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] Buffer usage in EXPLAIN and pg_stat_statements (review)

2009-09-28 Thread Tom Lane
Euler Taveira de Oliveira eu...@timbira.com writes:
 Itagaki Takahiro escreveu:
 Thanks. Except choice of words, can I think the basic architecture of
 the patch is ok? The codes to handle global variables like ReadBufferCount
 and GlobalReadBufferCount could be rewrite in cleaner way if we could
 drop supports of log_{parser|planner|executor|statement}_stats.
 
 Yes, it is. I'm afraid someone is relying on that piece of code.

If we have a better substitute, I think there'd be nothing wrong with
removing those features.  They were never anything but pretty crufty
anyway, and they are *not* a compatibility issue because applications
have no direct way to access those stats.  However, you'd have to be
sure that the substitute covers all the use-cases for the old stats
... which strikes me as a lot more territory than this patch has
proposed to cover.

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] DefaultACLs

2009-09-28 Thread Robert Haas
On Mon, Sep 28, 2009 at 10:44 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 I haven't read the patch, but it seems like one possible solution to
 this problem would be to declare that any any DEFAULT PRIVILEGES you
 set are cumulative.  If you configure a global default, a per-schema
 default, a default for tables whose names begin with the letter q, and
 a default for tables created between midnight and 4am, then a table
 called quux created in that schema at 2:30 in the morning will get the
 union of all four sets of privileges.

 Hmm ... interesting proposal.  Simple to understand and simple to
 implement, which are both to the good.  I'm not clear though on whether
 this behavior would be useful in practice.  Any comments from those
 who've been asking for default ACLs?

 One potential trouble spot is that presumably the built-in default
 privileges (eg, PUBLIC EXECUTE for functions) would *not* cumulate
 with user-specified defaults.

Why not?

...Robert

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


Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote:
  One potential trouble spot is that presumably the built-in default
  privileges (eg, PUBLIC EXECUTE for functions) would *not* cumulate
  with user-specified defaults.
 
 Why not?

How would you have a default that says I *don't* want public execute on
my new functions?

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Unicode UTF-8 table formatting for psql text output

2009-09-28 Thread Brad T. Sliger
On Sunday 27 September 2009 19:03:33 Robert Haas wrote:
 On Sun, Sep 27, 2009 at 9:24 PM, Selena Deckelmann

 selenama...@gmail.com wrote:
  Hi!
 
  On Wed, Sep 23, 2009 at 2:16 AM, Roger Leigh rle...@codelibre.net wrote:
  On Fri, Sep 18, 2009 at 11:30:05AM -0700, Selena Deckelmann wrote:
  Brad says:
 
         The patched code compiles without any additional warnings.
  Lint gripes about a trailing ',' in 'typedef enum printTextRule' in
  print.h.  Other additional lint seem to be false positives.  The
  regression tests pass against the new patch.
 
  I've attached a new patch which tidies up those extra commas, plus
  a patch showing the changes from the previous patch.
 
  Great!  Thank you.
 
  Brad -- can you review this patch? Is it ready for a committer?

 Brad already marked it that way on the CommitFest application, so I
 think that probably means yes.  :-)

 ...Robert

I looked at the new (psql-utf8-table-4.patch) patch.  I think the style 
issues are fixed.

During this review I found that `gmake check` will fail when 
LANG=en_US.UTF-8 in the environment.  In this case 
the patched psql produces UTF8 line art and the tests expect ASCII line art.

pg_regress clears LC_ALL by default, but does not clear LANG by 
default.  Please find attached a patch that 
causes pg_regress to also clear LANG by default.

Thanks,

--bts

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index f2f9603..68ba30b 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -697,11 +697,6 @@ initialize_environment(void)
 		unsetenv(LC_MONETARY);
 		unsetenv(LC_NUMERIC);
 		unsetenv(LC_TIME);
-		unsetenv(LANG);
-		/* On Windows the default locale cannot be English, so force it */
-#if defined(WIN32) || defined(__CYGWIN__)
-		putenv(LANG=en);
-#endif
 	}
 
 	/*
@@ -712,8 +707,14 @@ initialize_environment(void)
 	 */
 	unsetenv(LANGUAGE);
 	unsetenv(LC_ALL);
+	unsetenv(LANG);
+	/* On Windows the default locale cannot be English, so force it */
+#if defined(WIN32) || defined(__CYGWIN__)
+	putenv(LANG=en);
+#endif
 	putenv(LC_MESSAGES=C);
 
+
 	/*
 	 * Set multibyte as requested
 	 */

-- 
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] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server

2009-09-28 Thread Alvaro Herrera
Jim Cox escribió:

 Attached s/b a patch for the 8.5 TODO Add comments to output indicating 
 version
 of pg_dump and of the database server (pg_dump/pg_restore section, 9.2).

Hmm, what happens if you do a pg_dump -Fc?  Is this info saved anywhere
in the dump?  Surely if thi is useful in the text dump, it is useful in
the binary format dumps too.

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