Re: [HACKERS] [PATCH] Largeobject access controls
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
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)
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
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
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
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
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
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
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
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
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[]
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
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/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
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
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
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
* 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
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
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/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
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/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
[ 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
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
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
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
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
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
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
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
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
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/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]
- 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
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
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
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
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/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
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
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)
(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
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
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[]
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
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
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
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
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
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/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
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
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
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
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
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]
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
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
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
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
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
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]
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
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)
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
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
--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
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
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
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
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
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
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
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
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
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
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
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)
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)
* 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)
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
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)
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
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)
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
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
* 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
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
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