Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 16 November 2013 09:43 Amit Kapila wrote: > On Fri, Nov 15, 2013 at 10:18 PM, Peter Eisentraut > wrote: > > On 11/14/13, 2:50 AM, Amit Kapila wrote: > >> Find the rebased version attached with this mail. > > > > Doesn't build: > > > > openjade -wall -wno-unused-param -wno-empty -wfully-tagged -D . -D . > -c /usr/share/sgml/docbook/stylesheet/dsssl/modular/catalog -d > stylesheet.dsl -t sgml -i output-html -V html-index postgres.sgml > > openjade:reference.sgml:61:3:E: cannot find "alter_system.sgml"; > tried "ref/alter_system.sgml", "./alter_system.sgml", > "./alter_system.sgml", "/usr/local/share/sgml/alter_system.sgml", > "/usr/share/sgml/alter_system.sgml" > > openjade:config.sgml:164:27:X: reference to non-existent ID "SQL- > ALTERSYSTEM" > > make[3]: *** [HTML.index] Error 1 > > make[3]: *** Deleting file `HTML.index' > > osx -D. -x lower -i include-xslt-index postgres.sgml >postgres.xmltmp > > osx:reference.sgml:61:3:E: cannot find "alter_system.sgml"; tried > "ref/alter_system.sgml", "./alter_system.sgml", > "/usr/local/share/sgml/alter_system.sgml", > "/usr/share/sgml/alter_system.sgml" > > osx:config.sgml:164:27:X: reference to non-existent ID "SQL- > ALTERSYSTEM" > > make[3]: *** [postgres.xml] Error 1 > > > > New file missing in patch? > > Oops, missed the new sgml file in patch, updated patch to include it. > Many thanks for checking it. 1. Patch applied properly 2. No warnings in the compilation 3. Regress test passed. 4. Basic tests are passed. Please find review comments: + * ALTER SYSTEM SET + * + * Command to edit postgresql.conf + */ I feel it should be "Command to change the configuration parameter" because this command is not edits the postgresql.conf file. ereport(ERROR, (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("configuration file \"%s\" contains errors", - ConfigFileName))); + ErrorConfFile))); The ErrorConfFile prints "postgresql.auto.conf" only if there is any parsing problem with postgresql.auto.conf otherwise it always print "postgresql.conf" because of any other error. + * A stale temporary file may be left behind in case we crash. + * Such files are removed on the next server restart. The above comment is wrong, the stale temporary file will be used in the next ALTER SYSTEM command. I didn't find any code where it gets deleted on the next server restart. if any postmaster setting which are set by the alter system command which leads to failure of server start, what is the solution to user to proceed further to start the server. As it is mentioned that the auto.conf file shouldn't be edited manually. Regards, Hari babu. -- Sent 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:Patch: SSL: prefer server cipher order
On Fri, Nov 15, 2013 at 02:16:52PM -0800, Adrian Klaver wrote: > On 11/15/2013 11:49 AM, Marko Kreen wrote: > >On Fri, Nov 15, 2013 at 11:16:25AM -0800, Adrian Klaver wrote: > >>The description of the GUCs show up in the documentation but I am > >>not seeing the GUCs themselves in postgresql.conf, so I could test > >>no further. It is entirely possible I am missing a step and would > >>appreciate enlightenment. > > > >Sorry, I forgot to update sample config. > > > >ssl-prefer-server-cipher-order-v2.patch > >- Add GUC to sample config > >- Change default value to 'true', per comments from Alvaro and Magnus. > > > >ssl-ecdh-v2.patch > >- Add GUC to sample config > > > > Well that worked. > I made ssl connections to the server using psql and verified it > respected the order of ssl_ciphers. I do not have a client available > with a different view of cipher order so I cannot test that. Well, these are GUC patches so the thing to test is whether the GUCs work. ssl-prefer-server-cipher-order: Use non-standard cipher order in server, eg: RC4-SHA:DHE-RSA-AES128-SHA, see if on/off works. You can see OpenSSL default order with "openssl ciphers -v". ssl-ecdh: It should start using ECDHE-RSA immediately. Also see if adding !ECDH to ciphers will fall back to DHE. It's kind of hard to test the ssl_ecdh_curve as you can't see it anywhere. I tested it by measuring if bigger curve slowed connecting down... Bonus - test EC keys: $ openssl ecparam -name prime256v1 -out ecparam.pem $ openssl req -x509 -newkey ec:ecparam.pem -days 9000 -nodes \ -subj '/C=US/ST=Somewhere/L=Test/CN=localhost' \ -keyout server.key -out server.crt ssl-better-default: SSL should stay working, openssl ciphers -v 'value' should not contain any weak suites (RC4, SEED, DES-CBC, EXP, NULL) and no non-authenticated suites (ADH/AECDH). -- marko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] CREATE TABLE IF NOT EXISTS AS
Hackers, Co-worker asked a question I could not answer: Why is IF NOT EXISTS not supported by CREATE TABLE AS? Oversight, perhaps? Or maybe because one expects the table to have the data from the SELECT statement? If the latter, maybe OR REPLACE would be useful? Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] New option for pg_basebackup, to specify a different directory for pg_xlog
On Sat, Nov 16, 2013 at 2:27 PM, Haribabu kommi wrote: > on 15 November 2013 17:26 Magnus Hagander wrote: > >>On Fri, Nov 15, 2013 at 12:10 PM, Haribabu kommi >> wrote: > >>On 14 November 2013 23:59 Fujii Masao wrote: >>> On Thu, Nov 14, 2013 at 9:08 PM, Haribabu kommi >>> wrote: >>> > Please find attached the patch, for adding a new option for >>> > pg_basebackup, to specify a different directory for pg_xlog. >>> Sounds good! Here are the review comments: Don't we need to prevent users from specifying the same directory in both --pgdata and --xlogdir? > >>>I feel no need to prevent, even if user specifies both --pgdata and >>> --xlogdir as same directory >>>all the transaction log files will be created in the base directory >>> instead of pg_xlog directory. > > > >>Given how easy it would be to prevent that, I think we should. It would be >> an easy misunderstanding to specify that when you actually want it in >> /pg_xlog. Specifying that would be redundant in the first place, >> but people ca do that, but it > >>would also be very easy to do it by mistake, and you'd end up with >> something that's really bad, including a recursive symlink. > > > > Presently with initdb also user can specify both data and xlog directories > as same. > > To prevent the data directory and xlog directory as same, there is a way in > windows (_fullpath api) to get absolute path from a relative path, but I > didn’t get any such possibilities in linux. > > I didn’t find any other way to check it, if anyone have any idea regarding > this please let me know. What about make_absolute_path() in miscinit.c? Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pre-commit triggers
On 11/15/2013 07:01 PM, Andrew Dunstan wrote: > > Attached is a patch to provide a new event trigger that will fire on > transaction commit. I have tried to make certain that it fires at a > sufficiently early stage in the commit process that some of the evils > mentioned in previous discussions on this topic aren't relevant. > > The triggers don't fire if there is no real XID, so only actual data > changes should cause the trigger to fire. I have not looked at the patch, but does it also run pre-rollback ? If not, how hard would it be to make it so ? > They also don't fire in single user mode, so that if you do something > stupid like create a trigger that unconditionally raises an error you > have a way to recover. > > This is intended to be somewhat similar to the same feature in the > Firebird database, and the initial demand came from a client migrating > from that system to Postgres. > > cheers > > andrew > > -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ
[HACKERS] freeze cannot be finished
Hello! Could anyone review patch suggested by Jeff Janes ? Initial thread http://www.postgresql.org/message-id/flat/1384356585.995240...@f50.i.mail.ru#1384356585.995240...@f50.i.mail.ru Thanks in advance! > > On Wed, Nov 13, 2013 at 3:53 PM, Sergey Burladyan < eshkin...@gmail.com > > wrote: > >Jeff Janes < jeff.ja...@gmail.com > writes: > > > >If I not mistaken, looks like lazy_scan_heap() called from lazy_vacuum_rel() > >(see [1]) skip pages, even if it run with scan_all == true, lazy_scan_heap() > >does not increment scanned_pages if lazy_check_needs_freeze() return false, > >so > >if this occurred at wraparound vacuum it cannot update pg_class, because > >pg_class updated via this code: > > > > new_frozen_xid = FreezeLimit; > > if (vacrelstats->scanned_pages < vacrelstats->rel_pages) > > new_frozen_xid = InvalidTransactionId; > > > > vac_update_relstats(onerel, > > new_rel_pages, > > new_rel_tuples, > > new_rel_allvisible, > > vacrelstats->hasindex, > > new_frozen_xid); > > > >so i think in our prevent wraparound vacuum vacrelstats->scanned_pages always > >less than vacrelstats->rel_pages and pg_class relfrozenxid never updated. > > Yeah, I think that that is a bug. If the clean-up lock is unavailable but > the page is inspected without it and found not to need freezing, then the > page needs to be counted as scanned, but is not so counted. > > commit bbb6e559c4ea0fb4c346beda76736451dc24eb4e > Date: Mon Nov 7 21:39:40 2011 -0500 > > But this was introduced in 9.2.0, so unless the OP didn't upgrade to 9.2 > until recently, I don't know why it just started happening. > > It looks like a simple fix (to HEAD attached), but I don't know how to test > it. > > Also, it seem like it might be worth issuing a warning if scan_all is true > but all was not scanned. > > Cheers, > > Jeff > > -- > Sent via pgsql-general mailing list (pgsql-gene...@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-general > -- diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c new file mode 100644 index e1d6d1c..6778c7d *** a/src/backend/commands/vacuumlazy.c --- b/src/backend/commands/vacuumlazy.c *** lazy_scan_heap(Relation onerel, LVRelSta *** 602,607 --- 602,608 if (!lazy_check_needs_freeze(buf)) { UnlockReleaseBuffer(buf); + vacrelstats->scanned_pages++; continue; } LockBuffer(buf, BUFFER_LOCK_UNLOCK); -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] additional json functionality
On 11/16/2013 12:15 AM, Josh Berkus wrote: > On 11/15/2013 02:59 PM, Merlin Moncure wrote: >> On Fri, Nov 15, 2013 at 4:31 PM, Hannu Krosing >> wrote: >> I think you may be on to something here. This might also be a way >> opt-in to fast(er) serialization (upthread it was noted this is >> unimportant; I'm skeptical). I deeply feel that two types is not the >> right path but I'm pretty sure that this can be finessed. >> >>> As far as I understand merlin is mostly ok with stored json being >>> normalised and the problem is just with constructing "extended" >>> json (a.k.a. "processing instructions") to be used as source for >>> specialised parsers and renderers. > Thing is, I'm not particularly concerned about *Merlin's* specific use > case, which there are ways around. What I am concerned about is that we > may have users who have years of data stored in JSON text fields which > won't survive an upgrade to binary JSON, because we will stop allowing > certain things (ordering, duplicate keys) which are currently allowed in > those columns. At the very least, if we're going to have that kind of > backwards compatibilty break we'll want to call the new version 10.0. > > That's why naming old JSON as "json_text" won't work; it'll be a > hardened roadblock to upgrading. Then perhaps name the "new binary json" as jsob (JavaScript Object Binary) or just jsobj (JavaScript Object) and keep current json for what it is, namely JavaScript Object Notation. Cheers -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ -- Sent 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:Patch: SSL: prefer server cipher order
On 11/16/2013 06:24 AM, Marko Kreen wrote: On Fri, Nov 15, 2013 at 02:16:52PM -0800, Adrian Klaver wrote: On 11/15/2013 11:49 AM, Marko Kreen wrote: On Fri, Nov 15, 2013 at 11:16:25AM -0800, Adrian Klaver wrote: The description of the GUCs show up in the documentation but I am not seeing the GUCs themselves in postgresql.conf, so I could test no further. It is entirely possible I am missing a step and would appreciate enlightenment. Sorry, I forgot to update sample config. ssl-prefer-server-cipher-order-v2.patch - Add GUC to sample config - Change default value to 'true', per comments from Alvaro and Magnus. ssl-ecdh-v2.patch - Add GUC to sample config Well that worked. I made ssl connections to the server using psql and verified it respected the order of ssl_ciphers. I do not have a client available with a different view of cipher order so I cannot test that. Well, these are GUC patches so the thing to test is whether the GUCs work. ssl-prefer-server-cipher-order: Use non-standard cipher order in server, eg: RC4-SHA:DHE-RSA-AES128-SHA, see if on/off works. You can see OpenSSL default order with "openssl ciphers -v". ssl_ciphers = 'RC4-SHA:DHE-RSA-AES128-SHA' ssl_prefer_server_ciphers = on #ssl_ecdh_curve = 'prime256v1' aklaver@panda:~/pgsqlTest/data> ../bin/psql -d postgres -U aklaver -h localhost psql (9.4devel) SSL connection (cipher: RC4-SHA, bits: 128) ssl_ciphers = 'RC4-SHA:DHE-RSA-AES128-SHA' ssl_prefer_server_ciphers = off #ssl_ecdh_curve = 'prime256v1' aklaver@panda:~/pgsqlTest/data> ../bin/psql -d postgres -U aklaver -h localhost psql (9.4devel) SSL connection (cipher: DHE-RSA-AES128-SHA, bits: 128) ssl-ecdh: It should start using ECDHE-RSA immediately. Also see if adding !ECDH to ciphers will fall back to DHE. It's kind of hard to test the ssl_ecdh_curve as you can't see it anywhere. I tested it by measuring if bigger curve slowed connecting down... ssl_ciphers = 'RC4-SHA:DHE-RSA-AES128-SHA' ssl_prefer_server_ciphers = off ssl_ecdh_curve = 'prime256v1' aklaver@panda:~/pgsqlTest/data> ../bin/psql -d postgres -U aklaver -h localhost psql (9.4devel) SSL connection (cipher: DHE-RSA-AES128-SHA, bits: 128) ssl_ciphers = 'RC4-SHA:DHE-RSA-AES128-SHA' ssl_prefer_server_ciphers = on ssl_ecdh_curve = 'prime256v1' aklaver@panda:~/pgsqlTest/data> ../bin/psql -d postgres -U aklaver -h localhost psql (9.4devel) SSL connection (cipher: RC4-SHA, bits: 128) ssl_ciphers = 'DEFAULT:!LOW:!EXP:!MD5:@STRENGTH' ssl_prefer_server_ciphers = on OR off ssl_ecdh_curve = 'prime256v1' aklaver@panda:~/pgsqlTest/data> ../bin/psql -d postgres -U aklaver -h localhost psql (9.4devel) SSL connection (cipher: ECDHE-RSA-AES256-SHA, bits: 256) ssl_ciphers = 'DEFAULT:!ECDH:!LOW:!EXP:!MD5:@STRENGTH' ssl_prefer_server_ciphers = on ssl_ecdh_curve = 'prime256v1' aklaver@panda:~/pgsqlTest/data> ../bin/psql -d postgres -U aklaver -h localhost psql (9.4devel) SSL connection (cipher: DHE-RSA-AES256-SHA, bits: 256) Bonus - test EC keys: $ openssl ecparam -name prime256v1 -out ecparam.pem $ openssl req -x509 -newkey ec:ecparam.pem -days 9000 -nodes \ -subj '/C=US/ST=Somewhere/L=Test/CN=localhost' \ -keyout server.key -out server.crt EC test: ssl_ciphers = 'DEFAULT:!LOW:!EXP:!MD5:@STRENGTH' ssl_prefer_server_ciphers = off OR on ssl_ecdh_curve = 'prime256v1' aklaver@panda:~/pgsqlTest/data> ../bin/psql -d postgres -U aklaver -h localhost psql (9.4devel) SSL connection (cipher: ECDHE-ECDSA-AES256-SHA, bits: 256) ssl_ciphers = 'RC4-SHA:DHE-RSA-AES128-SHA' ssl_prefer_server_ciphers = on #ssl_ecdh_curve = 'prime256v1' Or ssl_ecdh_curve = 'prime256v1' aklaver@panda:~/pgsqlTest/data> ../bin/psql -d postgres -U aklaver -h localhost psql: SSL error: sslv3 alert handshake failure FATAL: no pg_hba.conf entry for host "::1", user "aklaver", database "postgres", SSL off ssl-better-default: SSL should stay working, openssl ciphers -v 'value' should not contain any weak suites (RC4, SEED, DES-CBC, EXP, NULL) and no non-authenticated suites (ADH/AECDH). Not sure about the above, if it is a GUC I can't find it. If it is something else than I will have to plead ignorance. -- Adrian Klaver adrian.kla...@gmail.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Wildcard usage enhancements in .pgpass
Hi, Attached is the patch that improves usage of '*' wildcard in .pgpass, particularly in the host part. The use case is below. I work with multiple environments (like staging, production, performance and so on), each one containing tens of different database clusters, each cluster has its own subdomain in DNS, i.e. foo.test.db, bar.test.db and so on. Each user has the same credentials on every database of a single domain, i.e. .test.db databases, but different ones in other domains. For those databases, each line in pgpass currently corresponds to a single database, i.e. foo1.test.db:5432:postgres:postgres:keep!tester foo2.test.db:5432:postgres:postgres:keep!tsecret ... foo99.test.db:5432:postgres:postgres:keep!tsecret bar1.another.db.:5432:postgres:postgres:trustno1 bar2.another.db.:5432:postgres:postgres:trustno1 ... The problem I'm having is that there are just too many lines like those (tens or even hundreds) and the new databases are added very frequently, making it hard to keep .pgpass up-to-date. What I'd like to have is an ability to specify a pattern in the hostname of .pgpass, to replace the plenty of lines with one: *.test.db:5432:postgres:postgres:keep!secret bar*.*.db:5432:postgres:postgres:trustno1 The patch in attachment implements exactly this, by allowing '*' in the hostname to substitute arbitrary number of characters until the dot. The pattern is only matched when there is a '.' or ':' after the * and only in the hostname, otherwise, '*' is treated like a normal character. It appears that it can only be used for IPv4 addresses, i.e. instead of 255 hosts for 192.168.1.0/24 one can just specify 192.168.1.*. I do understand that it might be a very limited use case in terms of community plans for improving .pgpass, but I doubt that I'm the only one to stumble upon the current limitation; the patch is quite small and might be the first step into extending the functionality of .pgpass It's currently missing the documentation, which I will add in case there will be an interest in making this patch a part of the core. Your feedback is greatly appreciated. -- Regards, Alexey Klyukin diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c new file mode 100644 index 18fcb0c..003739f *** a/src/interfaces/libpq/fe-connect.c --- b/src/interfaces/libpq/fe-connect.c *** static int parseServiceFile(const char * *** 373,379 PQconninfoOption *options, PQExpBuffer errorMessage, bool *group_found); ! static char *pwdfMatchesString(char *buf, char *token); static char *PasswordFromFile(char *hostname, char *port, char *dbname, char *username); static bool getPgPassFilename(char *pgpassfile); --- 373,379 PQconninfoOption *options, PQExpBuffer errorMessage, bool *group_found); ! static char *pwdfMatchesString(char *buf, char *token, bool is_hostname); static char *PasswordFromFile(char *hostname, char *port, char *dbname, char *username); static bool getPgPassFilename(char *pgpassfile); *** defaultNoticeProcessor(void *arg, const *** 5466,5472 * token doesn't match */ static char * ! pwdfMatchesString(char *buf, char *token) { char *tbuf, *ttok; --- 5466,5472 * token doesn't match */ static char * ! pwdfMatchesString(char *buf, char *token, bool is_hostname) { char *tbuf, *ttok; *** pwdfMatchesString(char *buf, char *token *** 5480,5485 --- 5480,5497 return tbuf + 2; while (*tbuf != 0) { + /* '*' matches everything until '.' or end, but only for the hostname */ + if (*tbuf == '*' && (*(tbuf + 1) == '.' || *(tbuf + 1) == ':') && + !bslash && is_hostname) + { + tbuf++; + while (*ttok != *tbuf) + { + if (*ttok == 0) + return (*tbuf == ':') ? tbuf + 1 : NULL; + ttok++; + } + } if (*tbuf == '\\' && !bslash) { tbuf++; *** PasswordFromFile(char *hostname, char *p *** 5588,5597 if (buf[len - 1] == '\n') buf[len - 1] = 0; ! if ((t = pwdfMatchesString(t, hostname)) == NULL || ! (t = pwdfMatchesString(t, port)) == NULL || ! (t = pwdfMatchesString(t, dbname)) == NULL || ! (t = pwdfMatchesString(t, username)) == NULL) continue;
Re: [HACKERS] pre-commit triggers
On 11/16/2013 03:00 PM, Hannu Krosing wrote: On 11/15/2013 07:01 PM, Andrew Dunstan wrote: Attached is a patch to provide a new event trigger that will fire on transaction commit. I have tried to make certain that it fires at a sufficiently early stage in the commit process that some of the evils mentioned in previous discussions on this topic aren't relevant. The triggers don't fire if there is no real XID, so only actual data changes should cause the trigger to fire. I have not looked at the patch, but does it also run pre-rollback ? If not, how hard would it be to make it so ? No it doesn't. The things you can do once a rollback has been initiated are extremely limited, so I'm not sure value there would be in such a thing. The requirements I was given specifically excluded this, so I haven't looked at it, but I suspect the answer to your second question is "quite hard". But feel free to prove me wrong :-) 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] Review:Patch: SSL: prefer server cipher order
Thanks for testing! On Sat, Nov 16, 2013 at 12:17:40PM -0800, Adrian Klaver wrote: > On 11/16/2013 06:24 AM, Marko Kreen wrote: > >ssl-better-default: > > SSL should stay working, openssl ciphers -v 'value' should not contain > > any weak suites (RC4, SEED, DES-CBC, EXP, NULL) and no non-authenticated > > suites (ADH/AECDH). > > Not sure about the above, if it is a GUC I can't find it. If it is > something else than I will have to plead ignorance. The patch just changes the default value for 'ssl_ciphers' GUC. The question is if the value works at all, and is good. -- 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] Wait free LW_SHARED acquisition - v0.2
On Fri, 2013-11-15 at 20:47 +0100, Andres Freund wrote: > So, here's the next version of this patchset: The 0002 patch contains non-ASCII, non-UTF8 characters: 0002-Very-basic-atomic-ops-implementation.patch: line 609, char 1, byte offset 43: invalid UTF-8 code Please change that to ASCII, or UTF-8 if necessary. -- Sent 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:Patch: SSL: prefer server cipher order
On 11/16/2013 12:37 PM, Marko Kreen wrote: Thanks for testing! On Sat, Nov 16, 2013 at 12:17:40PM -0800, Adrian Klaver wrote: On 11/16/2013 06:24 AM, Marko Kreen wrote: ssl-better-default: SSL should stay working, openssl ciphers -v 'value' should not contain any weak suites (RC4, SEED, DES-CBC, EXP, NULL) and no non-authenticated suites (ADH/AECDH). Not sure about the above, if it is a GUC I can't find it. If it is something else than I will have to plead ignorance. The patch just changes the default value for 'ssl_ciphers' GUC. I am still not sure what patch you are talking about. The two patches I saw where for server_prefer and ECDH key exchange. The question is if the value works at all, and is good. What value would we be talking about? Note: I have been working through a head cold and thought processes are sluggish, handle accordingly:) -- Adrian Klaver adrian.kla...@gmail.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review:Patch: SSL: prefer server cipher order
On Sat, Nov 16, 2013 at 01:03:05PM -0800, Adrian Klaver wrote: > On 11/16/2013 12:37 PM, Marko Kreen wrote: > >Thanks for testing! > > > >On Sat, Nov 16, 2013 at 12:17:40PM -0800, Adrian Klaver wrote: > >>On 11/16/2013 06:24 AM, Marko Kreen wrote: > >>>ssl-better-default: > >>> SSL should stay working, openssl ciphers -v 'value' should not contain > >>> any weak suites (RC4, SEED, DES-CBC, EXP, NULL) and no non-authenticated > >>> suites (ADH/AECDH). > >> > >>Not sure about the above, if it is a GUC I can't find it. If it is > >>something else than I will have to plead ignorance. > > > >The patch just changes the default value for 'ssl_ciphers' GUC. > > I am still not sure what patch you are talking about. The two > patches I saw where for server_prefer and ECDH key exchange. > > > > >The question is if the value works at all, and is good. > > > > What value would we be talking about? Ah, sorry. It's this one: https://commitfest.postgresql.org/action/patch_view?id=1310 > Note: I have been working through a head cold and thought processes > are sluggish, handle accordingly:) Get better soon! :) -- 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] additional json functionality
On Nov 16, 2013, at 12:04 PM, Hannu Krosing wrote: > Then perhaps name the "new binary json" as jsob (JavaScript Object Binary) > or just jsobj (JavaScript Object) and keep current json for what it is, > namely > JavaScript Object Notation. It’s still input and output as JSON, though. I still like JSONB best. David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] additional json functionality
On 11/16/2013 10:30 PM, David E. Wheeler wrote: > On Nov 16, 2013, at 12:04 PM, Hannu Krosing wrote: > >> Then perhaps name the "new binary json" as jsob (JavaScript Object Binary) >> or just jsobj (JavaScript Object) and keep current json for what it is, >> namely >> JavaScript Object Notation. > It’s still input and output as JSON, though. Yes, because JavaScript Object Notation *is* a serialization format (aka Notation) for converting JavaScript Objects to text format and back :) > I still like JSONB best. To me it feels redundant, like binarytextbinary the binary representation of JSON is JavaScript(-like) Object, not "binary json" So my vote would be either jsobj or jsdoc (as "document databases") tend to call the structured types "documents" Cheers -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ -- Sent 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:Patch: SSL: prefer server cipher order
On 11/16/2013 01:13 PM, Marko Kreen wrote: On Sat, Nov 16, 2013 at 01:03:05PM -0800, Adrian Klaver wrote: On 11/16/2013 12:37 PM, Marko Kreen wrote: Thanks for testing! On Sat, Nov 16, 2013 at 12:17:40PM -0800, Adrian Klaver wrote: On 11/16/2013 06:24 AM, Marko Kreen wrote: ssl-better-default: SSL should stay working, openssl ciphers -v 'value' should not contain any weak suites (RC4, SEED, DES-CBC, EXP, NULL) and no non-authenticated suites (ADH/AECDH). Not sure about the above, if it is a GUC I can't find it. If it is something else than I will have to plead ignorance. The patch just changes the default value for 'ssl_ciphers' GUC. I am still not sure what patch you are talking about. The two patches I saw where for server_prefer and ECDH key exchange. The question is if the value works at all, and is good. What value would we be talking about? Ah, sorry. It's this one: https://commitfest.postgresql.org/action/patch_view?id=1310 Got it, applied it. Results: openssl ciphers -v 'HIGH:!aNULL'|egrep '(RC4|SEED|DES-CBC|EXP|NULL|ADH|AECDH)' ECDHE-RSA-DES-CBC3-SHA SSLv3 Kx=ECDH Au=RSA Enc=3DES(168) Mac=SHA1 ECDHE-ECDSA-DES-CBC3-SHA SSLv3 Kx=ECDH Au=ECDSA Enc=3DES(168) Mac=SHA1 EDH-RSA-DES-CBC3-SHASSLv3 Kx=DH Au=RSA Enc=3DES(168) Mac=SHA1 EDH-DSS-DES-CBC3-SHASSLv3 Kx=DH Au=DSS Enc=3DES(168) Mac=SHA1 ECDH-RSA-DES-CBC3-SHA SSLv3 Kx=ECDH/RSA Au=ECDH Enc=3DES(168) Mac=SHA1 ECDH-ECDSA-DES-CBC3-SHA SSLv3 Kx=ECDH/ECDSA Au=ECDH Enc=3DES(168) Mac=SHA1 DES-CBC3-SHASSLv3 Kx=RSA Au=RSA Enc=3DES(168) Mac=SHA1 DES-CBC3-MD5SSLv2 Kx=RSA Au=RSA Enc=3DES(168) Mac=MD5 Note: I have been working through a head cold and thought processes are sluggish, handle accordingly:) Get better soon! :) Thanks, the worst is over. -- Adrian Klaver adrian.kla...@gmail.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review:Patch: SSL: prefer server cipher order
On Sat, Nov 16, 2013 at 02:07:57PM -0800, Adrian Klaver wrote: > On 11/16/2013 01:13 PM, Marko Kreen wrote: > >https://commitfest.postgresql.org/action/patch_view?id=1310 > > Got it, applied it. > > Results: > > openssl ciphers -v 'HIGH:!aNULL'|egrep > '(RC4|SEED|DES-CBC|EXP|NULL|ADH|AECDH)' > > ECDHE-RSA-DES-CBC3-SHA SSLv3 Kx=ECDH Au=RSA Enc=3DES(168) Mac=SHA1 > ECDHE-ECDSA-DES-CBC3-SHA SSLv3 Kx=ECDH Au=ECDSA Enc=3DES(168) Mac=SHA1 > EDH-RSA-DES-CBC3-SHASSLv3 Kx=DH Au=RSA Enc=3DES(168) Mac=SHA1 > EDH-DSS-DES-CBC3-SHASSLv3 Kx=DH Au=DSS Enc=3DES(168) Mac=SHA1 > ECDH-RSA-DES-CBC3-SHA SSLv3 Kx=ECDH/RSA Au=ECDH Enc=3DES(168) Mac=SHA1 > ECDH-ECDSA-DES-CBC3-SHA SSLv3 Kx=ECDH/ECDSA Au=ECDH Enc=3DES(168) Mac=SHA1 > DES-CBC3-SHASSLv3 Kx=RSA Au=RSA Enc=3DES(168) Mac=SHA1 > DES-CBC3-MD5SSLv2 Kx=RSA Au=RSA Enc=3DES(168) Mac=MD5 DES-CBC3 is 3DES, which is fine. Plain DES-CBC would be bad. If you don't see any other issues perhaps they are ready for committer? -- 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] Review:Patch: SSL: prefer server cipher order
On 11/16/2013 02:41 PM, Marko Kreen wrote: On Sat, Nov 16, 2013 at 02:07:57PM -0800, Adrian Klaver wrote: On 11/16/2013 01:13 PM, Marko Kreen wrote: https://commitfest.postgresql.org/action/patch_view?id=1310 Got it, applied it. Results: openssl ciphers -v 'HIGH:!aNULL'|egrep '(RC4|SEED|DES-CBC|EXP|NULL|ADH|AECDH)' ECDHE-RSA-DES-CBC3-SHA SSLv3 Kx=ECDH Au=RSA Enc=3DES(168) Mac=SHA1 ECDHE-ECDSA-DES-CBC3-SHA SSLv3 Kx=ECDH Au=ECDSA Enc=3DES(168) Mac=SHA1 EDH-RSA-DES-CBC3-SHASSLv3 Kx=DH Au=RSA Enc=3DES(168) Mac=SHA1 EDH-DSS-DES-CBC3-SHASSLv3 Kx=DH Au=DSS Enc=3DES(168) Mac=SHA1 ECDH-RSA-DES-CBC3-SHA SSLv3 Kx=ECDH/RSA Au=ECDH Enc=3DES(168) Mac=SHA1 ECDH-ECDSA-DES-CBC3-SHA SSLv3 Kx=ECDH/ECDSA Au=ECDH Enc=3DES(168) Mac=SHA1 DES-CBC3-SHASSLv3 Kx=RSA Au=RSA Enc=3DES(168) Mac=SHA1 DES-CBC3-MD5SSLv2 Kx=RSA Au=RSA Enc=3DES(168) Mac=MD5 DES-CBC3 is 3DES, which is fine. Plain DES-CBC would be bad. If you don't see any other issues perhaps they are ready for committer? I do not have any other questions/issues at this point. I am new to the review process, so I am not quite sure how it proceeds from here. -- Adrian Klaver adrian.kla...@gmail.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review:Patch: SSL: prefer server cipher order
On Sat, Nov 16, 2013 at 02:54:22PM -0800, Adrian Klaver wrote: > On 11/16/2013 02:41 PM, Marko Kreen wrote: > >If you don't see any other issues perhaps they are ready for committer? > > I do not have any other questions/issues at this point. I am new to > the review process, so I am not quite sure how it proceeds from > here. Simple - just click on "edit patch" on commitfest page and change patch status to "ready for committer". Then committers will know that they should look at the patch. -- 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] Review:Patch: SSL: prefer server cipher order
On 11/16/2013 03:09 PM, Marko Kreen wrote: On Sat, Nov 16, 2013 at 02:54:22PM -0800, Adrian Klaver wrote: On 11/16/2013 02:41 PM, Marko Kreen wrote: If you don't see any other issues perhaps they are ready for committer? I do not have any other questions/issues at this point. I am new to the review process, so I am not quite sure how it proceeds from here. Simple - just click on "edit patch" on commitfest page and change patch status to "ready for committer". Then committers will know that they should look at the patch. Done for both: SSL: better default ciphersuite SSL: prefer server cipher order Thanks for helping me through the process. -- Adrian Klaver adrian.kla...@gmail.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review:Patch: SSL: prefer server cipher order
On Sat, Nov 16, 2013 at 03:21:19PM -0800, Adrian Klaver wrote: > On 11/16/2013 03:09 PM, Marko Kreen wrote: > >On Sat, Nov 16, 2013 at 02:54:22PM -0800, Adrian Klaver wrote: > >>On 11/16/2013 02:41 PM, Marko Kreen wrote: > >>>If you don't see any other issues perhaps they are ready for committer? > >> > >>I do not have any other questions/issues at this point. I am new to > >>the review process, so I am not quite sure how it proceeds from > >>here. > > > >Simple - just click on "edit patch" on commitfest page and change > >patch status to "ready for committer". Then committers will know > >that they should look at the patch. > > > > Done for both: > > SSL: better default ciphersuite > SSL: prefer server cipher order I think you already handled the ECDH one too: https://commitfest.postgresql.org/action/patch_view?id=1286 > Thanks for helping me through the process. Thanks for reviewing. -- 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] Review:Patch: SSL: prefer server cipher order
On 11/16/2013 03:46 PM, Marko Kreen wrote: On Sat, Nov 16, 2013 at 03:21:19PM -0800, Adrian Klaver wrote: On 11/16/2013 03:09 PM, Marko Kreen wrote: On Sat, Nov 16, 2013 at 02:54:22PM -0800, Adrian Klaver wrote: On 11/16/2013 02:41 PM, Marko Kreen wrote: If you don't see any other issues perhaps they are ready for committer? I do not have any other questions/issues at this point. I am new to the review process, so I am not quite sure how it proceeds from here. Simple - just click on "edit patch" on commitfest page and change patch status to "ready for committer". Then committers will know that they should look at the patch. Done for both: SSL: better default ciphersuite SSL: prefer server cipher order I think you already handled the ECDH one too: https://commitfest.postgresql.org/action/patch_view?id=1286 Aah, missed that one. I updated to show my review and mark as Ready for Committer. Thanks for helping me through the process. Thanks for reviewing. -- Adrian Klaver adrian.kla...@gmail.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] additional json functionality
On 11/16/2013 02:04 PM, Hannu Krosing wrote: > On 11/16/2013 10:30 PM, David E. Wheeler wrote: >> I still like JSONB best. > To me it feels redundant, like binarytextbinary > > the binary representation of JSON is JavaScript(-like) Object, not > "binary json" JSONB is as close as we can get to "JSON", and it gives people the idea that this is the successor type to JSON. So +1 from me as well on "JSONB". -- Josh Berkus PostgreSQL Experts Inc. http://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] Improving avg performance for numeric
Pavel Stehule writes: > [ numeric-optimize-v8.patch ] I've committed this with some adjustments. Aside from cosmetic changes and documentation/comment improvements: * I don't agree at all with the claim that aggtransspace is only useful for INTERNAL transition data. There are lots of cases with regular SQL types where the planner doesn't have a good idea of how large the transition value will be, and with the current code it tends to guess on the small side (frequently 32 bytes :-(). It seems to me to be useful to give users a knob to twiddle there. Consider for example an aggregate that uses an integer array as transition state; the author of the aggregate might know that there are usually hundreds of entries in the array, and wish to dial up aggtransspace to prevent the planner from optimistically choosing hash aggregation. As committed, I just took out the restriction in CREATE AGGREGATE altogether; it will let you set SSPACE for any transition datatype. The planner will ignore it for pass-by-value types, where the behavior is known, but otherwise honor it. It's possible that we should put in a restriction to INTERNAL plus pass-by-reference types, or even INTERNAL plus pass-by-reference variable-length types, but I can't get excited about that. The error message would be too confusing I think. * There was a stray "else" added to clauses.c that disabled space accounting for polymorphic aggregates. * I simplified the summing code in do_numeric_accum; the copying and reallocating it was doing was not only unnecessary but contained unpleasant assumptions about what you can do with a NumericVar. This probably makes the committed patch a bit faster than what was submitted, but I didn't try to benchmark the change. * I made sure all the functions accessed their input state pointer with state = PG_ARGISNULL(0) ? NULL : (NumericAggState *) PG_GETARG_POINTER(0); There were places that omitted the ARGISNULL test, and thus only happened to work if the uninitialized Datum value they got was zero. While that might chance to be true in the current state of the code, it's not an assumption you're supposed to make. * numeric sum/avg failed to check for NaN inputs. * I fixed incorrect tests in the code added to pg_dump. 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] Storing pg_stat_statements query texts externally, pg_stat_statements in core
On Fri, Nov 15, 2013 at 8:51 AM, Peter Eisentraut wrote: > Compiler warnings with fortify settings: I'll post a revision with fixes for those. Another concern is that I see some race conditions that only occur when pg_stat_statements cache is under a lot of pressure with a lot of concurrency, that wasn't revealed by my original testing. I'm working on a fix for that. -- Peter Geoghegan -- Sent 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 TABLE IF NOT EXISTS AS
"David E. Wheeler" writes: > Co-worker asked a question I could not answer: Why is IF NOT EXISTS not > supported by CREATE TABLE AS? That's an even worse idea than plain CREATE IF NOT EXISTS (which was put in over vocal objections from me and some other people). Not only would you not have the faintest clue as to the properties of the table afterwards, but no clue as to its contents either. 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] pre-commit triggers
Hannu Krosing writes: > I have not looked at the patch, but does it also run pre-rollback ? error in trigger -> instant infinite loop. Besides, exactly what would you do in such a trigger? Not modify the database, for certain, because we're about to roll back. 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] Storing pg_stat_statements query texts externally, pg_stat_statements in core
On Sat, Nov 16, 2013 at 4:36 PM, Peter Geoghegan wrote: > I'll post a revision with fixes for those. Another concern is that I > see some race conditions that only occur when pg_stat_statements cache > is under a lot of pressure with a lot of concurrency, that wasn't > revealed by my original testing. I'm working on a fix for that. Revision attached. -- Peter Geoghegan pg_stat_statements_ext_text.v2.2013_11_16.patch.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] [PATCH] Use MAP_HUGETLB where supported (v3)
I was recently running some tests with huge page tables. I ran them on two different architectures: x86 and PPC64. I saw some discussion going on over here so thought of sharing. I was using 3 Cores, 8GB RAM, 2 LUN for filesystem (1 for dbfiles and 1 for logfiles) for these tests... I had dedicated (shared_buffers + 400bytes*max_connection + wal_buffers)/Pagesize [from /proc/meminfo] for huge pages. I kept some overcommit_hugepages to be used by work_mem (max_connection*work_mem)/Pagesize x86_64 bit gave me a benefit of 2-5% for TPC-C workload( I scaled from 1 to 100 users). PPC64 which uses 16MB and 64MB did not give me any benefits in fact the performance degraded as the concurrency of system increased. my 2 cents, hope it helps.
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Sat, Nov 16, 2013 at 4:35 PM, Haribabu kommi wrote: > On 16 November 2013 09:43 Amit Kapila wrote: >> On Fri, Nov 15, 2013 at 10:18 PM, Peter Eisentraut >> wrote: >> > On 11/14/13, 2:50 AM, Amit Kapila wrote: >> >> Find the rebased version attached with this mail. >> > > Please find review comments: > > + * ALTER SYSTEM SET > + * > + * Command to edit postgresql.conf > + > */ > > I feel it should be "Command to change the configuration parameter" > because this command is not edits the postgresql.conf file. Changed the comment, but I think it is better to use persistently in line suggested by you, so I have done that way. > ereport(ERROR, > (errcode(ERRCODE_CONFIG_FILE_ERROR), > errmsg("configuration file \"%s\" > contains errors", > - ConfigFileName))); > + ErrorConfFile))); > > The ErrorConfFile prints "postgresql.auto.conf" only if there is any parsing > problem > with postgresql.auto.conf otherwise it always print "postgresql.conf" because > of any other error. Changed to ensure ErrorConfFile contains proper config file name. Note: I have not asssigned file name incase of error in below loop, as file name in gconf is NULL in most cases and moreover this loops over guc_variables which doesn't contain values/parameters from auto.conf. So I don't think it is required to assign ErrorConfFile in this loop. ProcessConfigFile(GucContext context) { .. for (i = 0; i < num_guc_variables; i++) { struct config_generic *gconf = guc_variables[i]; .. } > > + * A stale temporary file may be left behind in case we crash. > + * Such files are removed on the next server restart. > > The above comment is wrong, the stale temporary file will be used > in the next ALTER SYSTEM command. I didn't find any code where it gets > deleted on the next server restart. Removed the comment from top of function and modified the comment where file is getting opened. > > if any postmaster setting which are set by the alter system command which > leads to failure of server start, what is the solution to user to proceed > further to start the server. As it is mentioned that the auto.conf file > shouldn't be edited manually. Yeah, but in case of emergency user can change it to get server started. Now the question is whether to mention it in documentation, I think we can leave this decision to committer. If he thinks that it is better to document then I will update it. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com alter_system_v12.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] Proof of concept: standalone backend with full FE/BE protocol
On Fri, Nov 15, 2013 at 5:21 PM, Simon Riggs wrote: > On 14 November 2013 03:41, Amit Kapila wrote: > >> I have gone through the mail chain of this thread and tried to find >> the different concerns or open ends for this patch. > > Not enough. This feature is clearly being suggested as a way to offer > Postgres in embedded mode for users by a back door. Current patch doesn't have such facility and I don't think somebody can use it as an embedded database. > Doing that forces > us to turn off many of the server's features and we will take a huge > step backwards in features, testing, maintainability of code and > wasted community time. > > "No administrative hassles" is just a complete fiction. Admin will > become a huge burden for any user in this mode, which will bite the > community and cause us to waste much time redesigning the server to > operate on a single session. > > -1 from me What I could understand from your objection is that you don't want users to get the impression of this feature as an embedded database. I think as the patch stands, it doesn't have such facility, so advertising it as an substitute for embedded database would be anyway inappropriate. The use case is to provide a standalone mode which will be useful for cases where today --single mode is required/used and I think documenting the feature that way is the right way to proceed. If this addresses your concern, then we can proceed to discuss solutions for other concerns like security? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers