Re: [HACKERS] SSI patch version 14
On 26.01.2011 13:36, Simon Riggs wrote: I looked at the patch to begin a review and immediately saw dtest. There's no docs to explain what it is, but a few comments fill me in a little more. Can we document that please? And/or explain why its an essential part of this commit? Could we keep it out of core, or if not, just commit that part separately? I notice the code is currently copyright someone else than PGDG. We still haven't really resolved this.. Looking at the dtest suite, I'd really like to have those tests in the PostgreSQL repository. However, I'd really like not to require python to run them, and even less dtester (no offense Markus) and the dependencies it has. I couldn't get it to run on my basic Debian system, clearly I'm doing something wrong, but it will be even harder for people on more exotic platforms. The tests don't seem very complicated, and they don't seem to depend much on the dtester features. How hard would it be to rewrite the test engine in C or perl? I'm thinking of defining each test in a simple text file, and write a small test engine to parse that. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER EXTENSION UPGRADE, v3
On Wed, Feb 2, 2011 at 03:21, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: PFA version 3 of the ALTER EXTENSION PATCH, cleaned and merged against recent HEAD and extension's branch from which I just produced the v30 patch. Excuse me for asking, but could you explain what is the purpose? Which is true, upgrade to 9.1 from past versions or upgrade from 9.1 to future versions? Also, how much advantage will we have compared with uninstall_MODULE.sql + CREATE EXTENSION? In my understanding, the patch does two things: 1. Add ALTER object SET EXTENSION 2. Add MODULE.upgrade.sql script for each contrib module #1 seems reasonable as a supplement for CREATE EXTENSION patch, but we might need not only attach but also detach. I guess #2 is much more difficult than expected because we might upgrade databases from older versions. Will we need upgrade script for each supported versions? -- if so, it would be nightmare... -- Itagaki Takahiro -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER EXTENSION UPGRADE, v3
Itagaki Takahiro itagaki.takah...@gmail.com writes: Excuse me for asking, but could you explain what is the purpose? Which is true, upgrade to 9.1 from past versions or upgrade from 9.1 to future versions? Also, how much advantage will we have compared with uninstall_MODULE.sql + CREATE EXTENSION? Both are true use cases and supported in the code. The goal is to be able to manage extensions upgrading. This is done by running a script the author provides. To know which script to run, you need to know the currently installed extension version, the available version, and determine from that the script filename. That's what the new control file options are about. Now that you can upgrade extensions to their next versions, what about migrating from an existing set of objects towards having an extension? This use case happens either when upgrading from pre-9.1 or when you're working on an in-house extension. At first it's not an extension, you just CREATE FUNCTION and CREATE VIEW. The day you decide to properly package it, you want to be able to do that without the hassle of DROP'ing all those objects that your production is depending on. In my understanding, the patch does two things: 1. Add ALTER object SET EXTENSION 2. Add MODULE.upgrade.sql script for each contrib module The patch also add new options in the control file so that it's possible to do ALTER EXTENSION foo UPGRADE;. That's the main goal. #1 seems reasonable as a supplement for CREATE EXTENSION patch, but we might need not only attach but also detach. I didn't think about detach, I'm not sure I see the use case… I guess #2 is much more difficult than expected because we might upgrade databases from older versions. Will we need upgrade script for each supported versions? -- if so, it would be nightmare... It's not about upgrading major versions, it's about upgrading extensions. The only time you will need to run the scripts in the patch is e.g. when upgrading the extension hstore from NULL to 1.0. Once done, hstore is registered as an extension, you're done. No need to redo that or maintain the upgrade script for 9.1 to 9.2. We will have to provide some other scripts when upgrade hstore from 1.0 to 1.1, whenever that happens (minor upgrades, major upgrades, etc). I hope to make the case clear enough… Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] SSI patch version 14
Heikki, On 02/02/2011 11:20 AM, Heikki Linnakangas wrote: I couldn't get it to run on my basic Debian system, clearly I'm doing something wrong, but it will be even harder for people on more exotic platforms. On Debian you only need 'python-twisted' and the dtester sources [1], IIRC. What issue did you run into? The tests don't seem very complicated, and they don't seem to depend much on the dtester features. How hard would it be to rewrite the test engine in C or perl? I've taken a quick look at a perl framework similar to twisted (POE). Doesn't seem too complicated, but I don't see the benefit of requiring one over the other. Another async, event-based framework that I've been playing with is asio (C++). I don't think it's feasible to write anything similar without basing on such a framework. (And twisted seems more mature and widespread (in terms of supported platforms) than POE, but that's probably just my subjective impression). However, the question here probably is whether or not you need all of the bells and whistles of dtester (as if there were that many) [2]. I'm thinking of defining each test in a simple text file, and write a small test engine to parse that. AFAIUI there are lots of possible permutations, so you don't want to have to edit files for each manually (several hundreds, IIRC). So using a scripting language to generate the permutations makes sense, IMO. Pretty much everything else that Kevin currently uses dtester for (i.e. initdb, running the postmaster, connecting with psql) is covered by the existing regression testing infrastructure already, I think. So it might be a matter of integrating the permutation generation and test running code into the existing infrastructure. Kevin? Regards Markus Wanner [1]: dtester project site: http://www.bluegap.ch/projects/dtester/ [2]: side note: Dtester mainly serves me to test Postgres-R, where the current regression testing infrastructure simply doesn't suffice. With dtester I tried to better structure the processes and services, their dependencies and the test environment. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSI patch version 14
On 02.02.2011 12:20, Heikki Linnakangas wrote: On 26.01.2011 13:36, Simon Riggs wrote: I looked at the patch to begin a review and immediately saw dtest. There's no docs to explain what it is, but a few comments fill me in a little more. Can we document that please? And/or explain why its an essential part of this commit? Could we keep it out of core, or if not, just commit that part separately? I notice the code is currently copyright someone else than PGDG. We still haven't really resolved this.. Looking at the dtest suite, I'd really like to have those tests in the PostgreSQL repository. However, I'd really like not to require python to run them, and even less dtester (no offense Markus) and the dependencies it has. I couldn't get it to run on my basic Debian system, clearly I'm doing something wrong, but it will be even harder for people on more exotic platforms. The tests don't seem very complicated, and they don't seem to depend much on the dtester features. How hard would it be to rewrite the test engine in C or perl? I'm thinking of defining each test in a simple text file, and write a small test engine to parse that. I took a stab at doing that. Attached, untar in the top source tree, and do cd src/test/isolation; make check. It's like installcheck, so it needs a postgres server to be running. Also available in my git repository at git://git.postgresql.org/git/users/heikki/postgres.git, branch serializable. I think this will work. This is a rough first version, but it already works. We can extend the test framework later if we need more features, but I believe this is enough for all the existing tests. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com isolationtester.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] SSI patch version 14
On 02.02.2011 14:48, Markus Wanner wrote: Heikki, On 02/02/2011 11:20 AM, Heikki Linnakangas wrote: I couldn't get it to run on my basic Debian system, clearly I'm doing something wrong, but it will be even harder for people on more exotic platforms. On Debian you only need 'python-twisted' and the dtester sources [1], IIRC. What issue did you run into? I installed dtester with: PYTHONPATH=~/pythonpath/lib/python2.6/site-packages/ python ./setup.py install --prefix=/home/heikki/pythonpath/ And that worked. But when I try to run Kevin's test suite, I get this: ~/git-sandbox/postgresql/src/test/regress (serializable)$ PYTHONPATH=~/pythonpath/lib/python2.6/site-packages/ make dcheck ./pg_dtester.py --temp-install --top-builddir=../../.. \ --multibyte= Postgres dtester suiteCopyright (c) 2004-2010, by Markus Wanner Traceback (most recent call last): File ./pg_dtester.py, line 1376, in module main(sys.argv[1:]) File ./pg_dtester.py, line 1370, in main runner = Runner(reporter=TapReporter(sys.stdout, sys.stderr, showTimingInfo=True), TypeError: __init__() got an unexpected keyword argument 'showTimingInfo' make: *** [dcheck] Virhe 1 At that point I had no idea what's wrong. PS. I tried python ./setup.py dtest, mentioned in the README, and it just said invalid command 'dtest'. But I presume the installation worked anyway. Pretty much everything else that Kevin currently uses dtester for (i.e. initdb, running the postmaster, connecting with psql) is covered by the existing regression testing infrastructure already, I think. So it might be a matter of integrating the permutation generation and test running code into the existing infrastructure. Kevin? Yeah, that was my impression too. (see my other post with WIP implementation of that) -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSI patch version 14
Heikki, On 02/02/2011 02:04 PM, Heikki Linnakangas wrote: TypeError: __init__() got an unexpected keyword argument 'showTimingInfo' make: *** [dcheck] Virhe 1 At that point I had no idea what's wrong. Hm.. looks like Kevin already uses the latest dtester code from git. You can either do that as well, or try to drop the 'showTimingInfo' argument from the call to Runner() in pg_dtester.py:1370. PS. I tried python ./setup.py dtest, mentioned in the README, and it just said invalid command 'dtest'. But I presume the installation worked anyway. Yeah, that's a gotcha in dtester. setup.py dtest requires dtester to be installed - a classic chicken and egg problem. I certainly need to look into this. Yeah, that was my impression too. (see my other post with WIP implementation of that) I'd like to get Kevin's opinion on that, but it certainly sounds like a pragmatic approach for now. Regards Markus Wanner -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] REVIEW: Determining client_encoding from client locale
Hi!, I have reviewed/tested this patch. OS = Linux ubuntu 2.6.35-22-generic #33-Ubuntu SMP Sun Sep 19 20:34:50 UTC 2010 i686 GNU/Linux PostgreSQL Version = Head (9.1devel) Patch gives the desired results(still testing), but couple of questions with this portion of the code. * if (conn-client_encoding_initial conn-client_encoding_initial[0]) { if (packet) strcpy(packet + packet_len, client_encoding); packet_len += strlen(client_encoding) + 1; if (packet) strcpy(packet + packet_len, conn-client_encoding_initial); packet_len += strlen(conn-client_encoding_initial) + 1; }* Is there any reason to check packet twice?. And suppose packet is null then we will not append client_encoding into the string but will increase the length(looks intentional! like in ADD_STARTUP_OPTION). In my point code should be like this *if (conn-client_encoding_initial conn-client_encoding_initial[0]) { if (packet) { strcpy(packet + packet_len, client_encoding); packet_len += strlen(client_encoding) + 1; strcpy(packet + packet_len, conn-client_encoding_initial); packet_len += strlen(conn-client_encoding_initial) + 1; } }* * * * * BTW why you have not used ADD_STARTUP_OPTION? I will test this patch on Windows and will send results. -- Ibrar Ahmed On Sun, Jan 30, 2011 at 10:56 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 29.01.2011 19:23, Peter Eisentraut wrote: Also, do we really need a new set of states for this..? I would have thought, just reading through the patch, that we could use the existing OPTION_SEND/OPTION_WAIT states.. Don't know. Maybe Heikki could comment; he wrote that initially. The other options send by the OPTION_SEND/WAIT machinery come from environment variables, there's a getenv() call where the SETENV_STATE_OPTION_SEND state is handled. It would be a bit awkward to shoehorn client_encoding into that. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Ibrar Ahmed
[HACKERS] Some more walsender metadata
Attached patch adds some more metadata to walsender results: * Adds the current xlog insert location as a third column in IDENTIFY_SYSTEM * Adds a resultset at the start of a base backup that contains the start xlog location (as returned by do_pg_start_backup) * Adds a resultset at the end of a base backup that contains the end xlog location (as returned by do_pg_stop_backup) These are all information that's interesting for the log receiver version of pg_basebackup (the changes to BASE_BACKUP) and the pg_streamrecv program (the IDENTIFY_SYSTEM) once integrated. I still haven't had the time to clean up those programs enough to submit as a patch yet (it has progressed past that early post from a month ago or so, but isn't done yet), but I wanted to make sure that the backend parts gets reviewed and added as soon as possible. And they're also quite trivial. That way if I don't find time to clean up the streaming mode receiver in time for 9.1, it can be maintained outside of the main tree until 9.2 - but the small backend changes would still be required. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index 73c05ed..5e9b5d4 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -1315,7 +1315,7 @@ The commands accepted in walsender mode are: listitem para Requests the server to identify itself. Server replies with a result - set of a single row, containing two fields: + set of a single row, containing three fields: /para para @@ -1344,6 +1344,19 @@ The commands accepted in walsender mode are: /para /listitem /varlistentry + + varlistentry + term + xlogpos + /term + listitem + para + Current xlog write location. Useful to get a known location in the + transaction log where streaming can start. + /para + /listitem + /varlistentry + /variablelist /para /listitem @@ -1520,15 +1533,16 @@ The commands accepted in walsender mode are: /variablelist /para para - When the backup is started, the server will first send a header in - ordinary result set format, followed by one or more CopyResponse - results, one for PGDATA and one for each additional tablespace other - than literalpg_default/ and literalpg_global/. The data in - the CopyResponse results will be a tar format (using ustar00 - extensions) dump of the tablespace contents. + When the backup is started, the server will first send two + ordinary result sets, followed by one or more CopyResponse + results. + /para + para + The first ordinary result set contains the starting position of the + backup, given in XLogRecPtr format as a single column in a single row. /para para - The header is an ordinary resultset with one row for each tablespace. + The second ordinary result set has one row for each tablespace. The fields in this row are: variablelist varlistentry @@ -1561,6 +1575,15 @@ The commands accepted in walsender mode are: /variablelist /para para + After the two regular result set, one or more CopyResponse results + will be sent, one for PGDATA and one for each additional tablespace other + than literalpg_default/ and literalpg_global/. The data in + the CopyResponse results will be a tar format (using ustar00 + extensions) dump of the tablespace contents. After the tar data is + complete, a final ordinary result set will be sent. + /para + + para The tar archive for the data directory and each tablespace will contain all files in the directories, regardless of whether they are productnamePostgreSQL/ files or other files added to the same @@ -1583,6 +1606,11 @@ The commands accepted in walsender mode are: Owner, group and file mode are set if the underlying filesystem on the server supports it. /para + para + Once all tablespaces have been sent, a final regular result set will + be sent. This result set contains the end position of the + backup, given in XLogRecPtr format as a single column in a single row. + /para /listitem /varlistentry /variablelist diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c index 29284a6..840d577 100644 --- a/src/backend/replication/basebackup.c +++ b/src/backend/replication/basebackup.c @@ -52,6 +52,7 @@ static void SendBackupHeader(List *tablespaces); static void base_backup_cleanup(int code, Datum arg); static void perform_base_backup(basebackup_options *opt, DIR *tblspcdir); static void parse_basebackup_options(List *options, basebackup_options *opt); +static void SendXlogRecPtrResult(XLogRecPtr ptr); /* * Size of each block sent into the tar stream for larger
Re: [HACKERS] pl/python explicit subtransactions
On 11-01-27 05:11 PM, Jan Urbański wrote: On 23/12/10 15:32, Jan Urbański wrote: Here's a patch implementing explicitly starting subtransactions mentioned in http://archives.postgresql.org/pgsql-hackers/2010-12/msg01991.php. It's an incremental patch on top of the spi-in-subxacts patch sent eariler. Updated to the spi-in-subxacts version sent earlier. Submission Review - The patch applies against master. Test updates are included. The patch doesn't included any documentation updates. The author did mention that he'll do these if it looks like the patch is going to be accepted. The plpython_subxact regression test you addded is failing on both python3 and 2.4 for me. It seems to be creating functions with the same name twice and the second time is failing with ERROR: function . already exists. I think this is just an issue with your expect files. Usability Review --- The patch implements a python context manager that allows plpython programs to control subtransactions with the python 'with' syntax. The patch implements what it describes. Using the subtransaction manager seems consistent with other examples of Python context managers. This feature seems useful for pl/python developers. The 'with' syntax was only officially added with python 2.6. I have confirmed that the patch does not break plpython going as far back as 2.5 and 2.4. I have no reason to think that earlier versions will be broken either, I just didn't test anything earlier than 2.4. I think this feature is useful for developing more complicated functions in pl/python and we don't have an existing way of managing savepoints from pl/python. The context manager approach seems consistent with how recent python versions deal with this type of thing in other areas. Feature Test No issues found. Code Review PLy_abort_open_subtransactions(...) line 1215: ereport(WARNING, (errmsg(Forcibly aborting a subtransaction that has not been exited))); Forcibly should be forcibly (lower case) Similarly in PLy_subxact_enter and PLy_subxact_exit a few PLy_exception_set calls start with an upper case character when I think we want it to be lower case. Other than that I don't see any issues. I am marking this as waiting for author since the documentation is still outstanding. -- Sent 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: Determining client_encoding from client locale
On Wed, Feb 2, 2011 at 5:22 AM, Ibrar Ahmed ibrar.ah...@gmail.com wrote: Hi!, I have reviewed/tested this patch. OS = Linux ubuntu 2.6.35-22-generic #33-Ubuntu SMP Sun Sep 19 20:34:50 UTC 2010 i686 GNU/Linux PostgreSQL Version = Head (9.1devel) Patch gives the desired results(still testing), but couple of questions with this portion of the code. * if (conn-client_encoding_initial conn-client_encoding_initial[0]) { if (packet) strcpy(packet + packet_len, client_encoding); packet_len += strlen(client_encoding) + 1; if (packet) strcpy(packet + packet_len, conn-client_encoding_initial); packet_len += strlen(conn-client_encoding_initial) + 1; }* Is there any reason to check packet twice?. And suppose packet is null then we will not append client_encoding into the string but will increase the length(looks intentional! like in ADD_STARTUP_OPTION). I got the point, why we are doing this, just to calculate the length. Sorry for this point. In my point code should be like this *if (conn-client_encoding_initial conn-client_encoding_initial[0]) { if (packet) { strcpy(packet + packet_len, client_encoding); packet_len += strlen(client_encoding) + 1; strcpy(packet + packet_len, conn-client_encoding_initial); packet_len += strlen(conn-client_encoding_initial) + 1; } }* * * * * BTW why you have not used ADD_STARTUP_OPTION? I will test this patch on Windows and will send results. -- Ibrar Ahmed On Sun, Jan 30, 2011 at 10:56 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 29.01.2011 19:23, Peter Eisentraut wrote: Also, do we really need a new set of states for this..? I would have thought, just reading through the patch, that we could use the existing OPTION_SEND/OPTION_WAIT states.. Don't know. Maybe Heikki could comment; he wrote that initially. The other options send by the OPTION_SEND/WAIT machinery come from environment variables, there's a getenv() call where the SETENV_STATE_OPTION_SEND state is handled. It would be a bit awkward to shoehorn client_encoding into that. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Ibrar Ahmed -- Ibrar Ahmed
Re: [HACKERS] SSI patch version 14
Heikki Linnakangas wrote: On 02.02.2011 12:20, Heikki Linnakangas wrote: The tests don't seem very complicated, and they don't seem to depend much on the dtester features. How hard would it be to rewrite the test engine in C or perl? I'm thinking of defining each test in a simple text file, and write a small test engine to parse that. I took a stab at doing that. Attached, untar in the top source tree, and do cd src/test/isolation; make check. It's like installcheck, so it needs a postgres server to be running. Also available in my git repository at git://git.postgresql.org/git/users/heikki/postgres.git, branch serializable. I think this will work. This is a rough first version, but it already works. We can extend the test framework later if we need more features, but I believe this is enough for all the existing tests. Thanks for this. It does seem the way to go. I can fill it out with the rest of the tests, unless you'd rather. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSI patch version 14
On 02.02.2011 16:11, Kevin Grittner wrote: Heikki Linnakangas wrote: On 02.02.2011 12:20, Heikki Linnakangas wrote: The tests don't seem very complicated, and they don't seem to depend much on the dtester features. How hard would it be to rewrite the test engine in C or perl? I'm thinking of defining each test in a simple text file, and write a small test engine to parse that. I took a stab at doing that. Attached, untar in the top source tree, and do cd src/test/isolation; make check. It's like installcheck, so it needs a postgres server to be running. Also available in my git repository at git://git.postgresql.org/git/users/heikki/postgres.git, branch serializable. I think this will work. This is a rough first version, but it already works. We can extend the test framework later if we need more features, but I believe this is enough for all the existing tests. Thanks for this. It does seem the way to go. I can fill it out with the rest of the tests, unless you'd rather. Thanks, please do. I converted the next test, receipt-report, grab that from my git repository first. It produces over two hundred permutations, so it's going to be a bit tedious to check the output for that, but I think it's still acceptable so that we don't need more complicated rules for what is accepted. IOW, we can just print the output of all permutations and diff, we won't need if step X ran before step Y, commit should succeed rules that the python version had. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSI patch version 14
Heikki Linnakangas wrote: I converted the next test, receipt-report, grab that from my git repository first. Will do. It produces over two hundred permutations, so it's going to be a bit tedious to check the output for that, but I think it's still acceptable so that we don't need more complicated rules for what is accepted. IOW, we can just print the output of all permutations and diff, we won't need if step X ran before step Y, commit should succeed rules that the python version had. During initial development, I was very glad to have the one-line- per-permutation summary; however, lately I've been wishing for more detail, such as is available with this approach. At least for the moment, what this provides is exactly what is most useful. If there is ever a major refactoring (versus incremental enhancements), it might be worth developing a way to filter the detailed input into the sort of summary we were getting from dtester, but we can cross that bridge when (and if) we come to it. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSI patch version 14
On 02.02.2011 16:27, Kevin Grittner wrote: Heikki Linnakangas wrote: It produces over two hundred permutations, so it's going to be a bit tedious to check the output for that, but I think it's still acceptable so that we don't need more complicated rules for what is accepted. IOW, we can just print the output of all permutations and diff, we won't need if step X ran before step Y, commit should succeed rules that the python version had. During initial development, I was very glad to have the one-line- per-permutation summary; however, lately I've been wishing for more detail, such as is available with this approach. At least for the moment, what this provides is exactly what is most useful. If there is ever a major refactoring (versus incremental enhancements), it might be worth developing a way to filter the detailed input into the sort of summary we were getting from dtester, but we can cross that bridge when (and if) we come to it. Ok, great. Let's just add comments to the test specs to explain which permutations should succeed and which should fail, then. And which ones fail because they're false positives. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Move WAL warning
When running pg_basebackup with -x to include all transaction log, the server will still throw a warning about xlog archiving if it's not enabled - that is completely irrelevant since pg_basebackup has included it already (and if it was gone, the base backup step itself will fail - actual error and not warning). This patch moves the warning from do_pg_base_backup to pg_base_backup, so it still shows when using the explicit function calls, but goes away when using pg_basebackup. Objections? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 66cc004..7d1a95e 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -8619,6 +8619,11 @@ pg_stop_backup(PG_FUNCTION_ARGS) snprintf(stopxlogstr, sizeof(stopxlogstr), %X/%X, stoppoint.xlogid, stoppoint.xrecoff); + + if (!XLogArchivingActive()) + ereport(NOTICE, +(errmsg(WAL archiving is not enabled; you must ensure that all required WAL segments are copied through other means to complete the backup))); + PG_RETURN_TEXT_P(cstring_to_text(stopxlogstr)); } @@ -8870,9 +8875,6 @@ do_pg_stop_backup(char *labelfile) ereport(NOTICE, (errmsg(pg_stop_backup complete, all required WAL segments have been archived))); } - else - ereport(NOTICE, -(errmsg(WAL archiving is not enabled; you must ensure that all required WAL segments are copied through other means to complete the backup))); /* * We're done. As a convenience, return the ending WAL location. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER EXTENSION UPGRADE, v3
Itagaki Takahiro itagaki.takah...@gmail.com writes: The latest extension might drop some functions. Then the upgrade script contains the DROP commands. I'm still not clear what upgrade means. if module authors wrote functions with C, they can just replace .so to upgrade. If with SQL or PL/pgSQL, they should execute CREATE OR REPLACE FUNCTION. When do you execute those statements? Certainly, you want the user to issue ALTER EXTENSION foo UPGRADE and be done with it. The patch seems useful to upgrade from NULL to 1.0, but I cannot imagine how it work for cases from 1.0 to higher versions. For example, if we have 3 versions of a module below: NULL unmanaged functions only v1EXTENSION support with an additional function v2EXTENSION support with another function. How do we write upgrading scripts for NULL=v1, NULL=v2, and v1=v2 ? Well, you write 3 scripts. Let's consider an example, the lo contrib, with its 3 objects: CREATE DOMAIN lo AS pg_catalog.oid; CREATE OR REPLACE FUNCTION lo_oid(lo) … CREATE OR REPLACE FUNCTION lo_manage() … Now, the upgrade script from version NULL to 1.0 is alter domain @extschema@.lo set extension lo; alter function @extschema@.lo_oid(lo) set extension lo; alter function @extschema@.lo_manage() set extension lo; The upgrade script from version 1.0 to 2.0 is, let's say: CREATE OR REPLACE FUNCTION @extschema@.lo_newfunc() … So the upgrade script from version NULL to 2.0 is: alter domain @extschema@.lo set extension lo; alter function @extschema@.lo_oid(lo) set extension lo; alter function @extschema@.lo_manage() set extension lo; CREATE OR REPLACE FUNCTION @extschema@.lo_newfunc() … If as an extension author you're kind enough to provide all those 3 scripts and the upgrade setup in the control file, then the user can issue ALTER EXTENSION lo UPGRADE; and have all your cases covered automatically. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] Transaction-scope advisory locks
On 2011-01-28 10:12 AM +0200, I wrote: I still didn't address the issue with pg_advisory_unlock_all() releasing transaction scoped locks, but I'm going to. .. and here's the patch. I'm not too confident with the code I added to storage/lmgr/lock.c, but it seems to be working. Earlier there was some discussion about adding regression tests for advisory locks. However, I don't see where they would fit in our current .sql files and adding a new one just for a few tests didn't seem right. Anyone have an idea where they should go or should I just add a new one? Regards, Marko Tiikkaja *** a/doc/src/sgml/func.sgml --- b/doc/src/sgml/func.sgml *** *** 14544,14634 SELECT (pg_stat_file('filename')).modification; literalfunctionpg_advisory_lock(parameterkey/ typebigint/)/function/literal /entry entrytypevoid/type/entry !entryObtain exclusive advisory lock/entry /row row entry literalfunctionpg_advisory_lock(parameterkey1/ typeint/, parameterkey2/ typeint/)/function/literal /entry entrytypevoid/type/entry !entryObtain exclusive advisory lock/entry /row row entry literalfunctionpg_advisory_lock_shared(parameterkey/ typebigint/)/function/literal /entry entrytypevoid/type/entry !entryObtain shared advisory lock/entry /row row entry literalfunctionpg_advisory_lock_shared(parameterkey1/ typeint/, parameterkey2/ typeint/)/function/literal /entry entrytypevoid/type/entry !entryObtain shared advisory lock/entry /row row entry literalfunctionpg_advisory_unlock(parameterkey/ typebigint/)/function/literal /entry entrytypeboolean/type/entry !entryRelease an exclusive advisory lock/entry /row row entry literalfunctionpg_advisory_unlock(parameterkey1/ typeint/, parameterkey2/ typeint/)/function/literal /entry entrytypeboolean/type/entry !entryRelease an exclusive advisory lock/entry /row row entry literalfunctionpg_advisory_unlock_all()/function/literal /entry entrytypevoid/type/entry !entryRelease all advisory locks held by the current session/entry /row row entry literalfunctionpg_advisory_unlock_shared(parameterkey/ typebigint/)/function/literal /entry entrytypeboolean/type/entry !entryRelease a shared advisory lock/entry /row row entry literalfunctionpg_advisory_unlock_shared(parameterkey1/ typeint/, parameterkey2/ typeint/)/function/literal /entry entrytypeboolean/type/entry !entryRelease a shared advisory lock/entry /row row entry literalfunctionpg_try_advisory_lock(parameterkey/ typebigint/)/function/literal /entry entrytypeboolean/type/entry !entryObtain exclusive advisory lock if available/entry /row row entry literalfunctionpg_try_advisory_lock(parameterkey1/ typeint/, parameterkey2/ typeint/)/function/literal /entry entrytypeboolean/type/entry !entryObtain exclusive advisory lock if available/entry /row row entry literalfunctionpg_try_advisory_lock_shared(parameterkey/ typebigint/)/function/literal /entry entrytypeboolean/type/entry !entryObtain shared advisory lock if available/entry /row row entry literalfunctionpg_try_advisory_lock_shared(parameterkey1/ typeint/, parameterkey2/ typeint/)/function/literal /entry entrytypeboolean/type/entry !entryObtain shared advisory lock if available/entry /row /tbody /tgroup --- 14544,14690 literalfunctionpg_advisory_lock(parameterkey/ typebigint/)/function/literal /entry entrytypevoid/type/entry !entryObtain exclusive session level advisory lock/entry /row row entry literalfunctionpg_advisory_lock(parameterkey1/ typeint/, parameterkey2/ typeint/)/function/literal /entry entrytypevoid/type/entry !entryObtain exclusive session level advisory lock/entry /row row entry literalfunctionpg_advisory_lock_shared(parameterkey/ typebigint/)/function/literal /entry entrytypevoid/type/entry !entryObtain shared session level advisory lock/entry /row row entry literalfunctionpg_advisory_lock_shared(parameterkey1/ typeint/, parameterkey2/ typeint/)/function/literal /entry entrytypevoid/type/entry !entryObtain shared session level advisory lock/entry
Re: [HACKERS] REVIEW: alter extension upgrade (patch v3)
Hi, Anssi Kääriäinen anssi.kaariai...@thl.fi writes: There is one make check error: test rules... FAILED I forgot to update the pg_available_extensions system view in the tests, but failed to do so in the extension's patch too, where Itagaki fixed it. Will fix if not beaten to it, in which case the fix for this very patch will fall down of rebasing against master… When building the contrib modules, make installcheck has one failure: test dblink ... FAILED Apparently your setup is not allowing dblink to connect… There is no upgrade rule for adminpack. If I am not mistaken, this could be defined as upgrade_from_all = '.* = adminpack.sql' as the adminpack.sql only contains create or replace statements. Right. Sorry about missing it. In fact the regexp .* will not match NULL, so you have to add a specific NULL case. dblink has upgrade rule upgrade_from_null = 'null = dblink.upgrade.sql' however, there is not file dblink.upgrade.sql. Same for btree_gin and possibly others. Those must have been missed in my git adding, I'm quite sure I have them on another computer than the one I'm using here, will fix later. Sorry about that. For example the int_aggregate.upgrade.sql uses this form: alter function @extschema@.int_array_enum(integer[]) set extension int_aggregate; while btree_gist has this (no @extschema@): alter function gbt_text_compress(internal) set extension btree_gist; I am not sure which one of them is correct. I think having extschema there is what is wanted, and if so, this should also be mentioned in the documentation. The @extschema@ is correct, sorry about that too. I've been pushing to finish all those upgrade scripts early enough :( Here's what says the comments in the code (which I've been working on a moon ago, with much less pressure and tiredness): * At upgrade time, it's highly possible that the script will need * to ALTER already existing objects, so we only offer support for * the placeholder @extschema@. So I think the code is ok but the upgrade scripts will need another round of care. In general, I think the upgrade rules and associated .upgrade.sql files should be revisited. I did not have time to go through them all. Will do. Thanks for the heads up. Might it be possible that there is a risk of attaching incompatible versions of objects to an extension? Say, you have contrib module foo which is loaded in 8.4 with \i. The module contains function bar, which is redefined in 9.1. When you upgrade from null, you will have the 8.4's version of function bar registered in the extension foo. That is, upgrade from null does not upgrade the objects in an extension, it just attaches them to the extension. As a result, when restoring from dump, you get back 9.1's version. What will happen if you have tables, indexes etc depending on the objects in the extension? Is it possible to create rules to upgrade these extensions easily? Let's not mix the mechanism and how we use it. What's in the patch is a way for the authors to provide for upgrade scripts. If 9.0 and 9.1 versions of the extension are different there's no reason for them to provide the same upgrade from null script. See my lo example on a previous mail to Itagaki. As the operator used for matching the version number is ~, the rule ' = foo.upgrade.sql' will match any version. This is just as specified, but if you have two upgrade rules: '9.1 = foo.upgrade.sql.1' and '9.1.1 = foo.upgrade.sql.2' the regexp for 9.1 will match also 9.1.1 . This could be quite surprising. Should the operator be such that exact match is required, that is ^ is inserted before the search string and $ after it? Also, should the ALTER EXTENSION UPGRADE give a notice: 'using rule upgrade_from_xxx'? Yeah, anchored matches might be a better choice here. Will wait for some more comments and do just that baring objections. As far as the notice is concerned, though, you have the information at the DEBUG1 level, I'm not sure about getting a notice message. That's useful for authors debugging their control files, not for users… While trying to fool the upgrade engine, I noticed that if you use absolute paths, you get the error: ERROR: script path not allowed DETAIL: Extension's script are expected in /usr/local/pgsql/share. Is this correct? Isn't the path /usr/local/pgsql/share/contrib Is it intentional that paths are allowed in the upgrade script name? You can use ../../foobar.upgrade.sql as the filename. Itagaki raised some concerns about the path management too, so I will follow what comes out of this in the upgrade patch too. All in all, so far the feature has been relatively easy and intuitive to use. My biggest concern is the upgrade_from_null, as specified, does not actually upgrade the objects, just attach them to the extension. I have no more time to test the patch.
Re: [HACKERS] SSI patch version 14
Heikki Linnakangas wrote: I converted the next test, receipt-report, grab that from my git repository first. It produces over two hundred permutations, so it's going to be a bit tedious to check the output for that, but I think it's still acceptable so that we don't need more complicated rules for what is accepted. I was a bit disconcerted to see 48 permutations fail when dcheck had been seeing 6; but it turned out to be simple to fix -- the third connection was declared READ ONLY in the dcheck version. Measuring false positives for the READ WRITE version is probably valuable, but we also want to test the READ ONLY optimizations. The two ids test has similar dynamics, so I'll change one or the other so we cover both scenarios. IOW, we can just print the output of all permutations and diff, we won't need if step X ran before step Y, commit should succeed rules that the python version had. I found two problems with this, and I'm not sure how to handle either. If we can figure out an approach, I'm happy to work on it. (1) We don't have much in the way of detail yet. I put a different detail message on each, so that there is more evidence, hopefully at least somewhat comprehensible to an educated user, about how the cancelled transaction fit into the dangerous pattern of access among transactions. Ultimately, I hope we can improve these messages to include such detail as table names in many circumstances, but that's not 9.1 material. What I did include, when it was easily available, was another xid involved in the conflict. These are not matching from one test to the next. (2) The NOTICE lines for implicit index creation pop up at odd times in the output, like in the middle of a SQL statement. It looks like these are piling up in a buffer somewhere and getting dumped into the output when the buffer fills. They are actually showing up at exactly the same point on each run, but I doubt that we can count on that for all platforms, and even if we could -- it's kinda ugly. Perhaps we should change the client logging level to suppress these? They're not really important here. So, I think (2) is probably easy, but I don't see how we can deal with (1) as easily. Suppress detail? Filter to change the xid number to some literal? -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PERFORM] Slow count(*) again...
Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: On 02/01/2011 05:47 PM, Bruce Momjian wrote: Tom Lane wrote: At this point what we've got is 25% of the runtime in nodeAgg.c overhead, and it's difficult to see how to get any real improvement without tackling that. Do we want a TODO about optimizing COUNT(*) to avoid aggregate processing overhead? Whether or not it's bad application design, it's ubiquitous, and we should make it work as best we can, IMNSHO. This often generates complaints about Postgres, and if we really plan for world domination this needs to be part of it. I don't think that saving ~25% on COUNT(*) runtime will help that at all. The people who complain about it expect it to be instantaneous. If this sort of hack were free, I'd be all for doing it anyway; but I'm concerned that adding tests to enable a fast path will slow down every other aggregate, or else duplicate a lot of code that we'll then have to maintain. OK, thank you. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Where are we on SQl-MED?
I'm trying to follow where we are on SQL-MED and it's difficult. We have patches on patches and working out what's current is by no means easy. Is there an available git repo I can pull from that contains the current state? That would be *so* much easier than trying to extract a non-trivial number of matches and apply them all in the right order. 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] SSI patch version 14
On 02.02.2011 17:52, Kevin Grittner wrote: I found two problems with this, and I'm not sure how to handle either. If we can figure out an approach, I'm happy to work on it. (1) We don't have much in the way of detail yet. I put a different detail message on each, so that there is more evidence, hopefully at least somewhat comprehensible to an educated user, about how the cancelled transaction fit into the dangerous pattern of access among transactions. Ultimately, I hope we can improve these messages to include such detail as table names in many circumstances, but that's not 9.1 material. What I did include, when it was easily available, was another xid involved in the conflict. These are not matching from one test to the next. (2) The NOTICE lines for implicit index creation pop up at odd times in the output, like in the middle of a SQL statement. It looks like these are piling up in a buffer somewhere and getting dumped into the output when the buffer fills. They are actually showing up at exactly the same point on each run, but I doubt that we can count on that for all platforms, and even if we could -- it's kinda ugly. Perhaps we should change the client logging level to suppress these? They're not really important here. So, I think (2) is probably easy, but I don't see how we can deal with (1) as easily. Suppress detail? Filter to change the xid number to some literal? Suppressing detail seems easiest. AFAICS all the variable parts are in errdetail. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] A postgres parser related question
Luis Ochoa wrote: Hi everyone, I just want to know something about postgresql parser, because I want to add a new feature for pgAdmin graphical query builder (GQB) that allow an user to create a query graphical model from a sql statment, and I just want to reuse postgres parser code (reuse this) to do the task of create an abstract tree like structure for the sql statment. I just remember that read at some place that I don't remember that the syntax and semantic checks are done in two different stages, and as I can understand, then is possible to just reuse only the parser stage in an easy way, but before even trying to do this task, I want some advices from people that know a lot more than me from postgres internals. Read the developer's FAQ for info on understanding the Postgres source code. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Move WAL warning
On 02.02.2011 16:36, Magnus Hagander wrote: When running pg_basebackup with -x to include all transaction log, the server will still throw a warning about xlog archiving if it's not enabled - that is completely irrelevant since pg_basebackup has included it already (and if it was gone, the base backup step itself will fail - actual error and not warning). This patch moves the warning from do_pg_base_backup to pg_base_backup, so it still shows when using the explicit function calls, but goes away when using pg_basebackup. For the sake of consistency, how about moving the pg_stop_backup complete, all required WAL segments have been archived notice too? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER EXTENSION UPGRADE, v3
On Feb 2, 2011, at 6:45 AM, Dimitri Fontaine wrote: Well, you write 3 scripts. Let's consider an example, the lo contrib, with its 3 objects: CREATE DOMAIN lo AS pg_catalog.oid; CREATE OR REPLACE FUNCTION lo_oid(lo) … CREATE OR REPLACE FUNCTION lo_manage() … Now, the upgrade script from version NULL to 1.0 is alter domain @extschema@.lo set extension lo; alter function @extschema@.lo_oid(lo) set extension lo; alter function @extschema@.lo_manage() set extension lo; The upgrade script from version 1.0 to 2.0 is, let's say: CREATE OR REPLACE FUNCTION @extschema@.lo_newfunc() … So the upgrade script from version NULL to 2.0 is: alter domain @extschema@.lo set extension lo; alter function @extschema@.lo_oid(lo) set extension lo; alter function @extschema@.lo_manage() set extension lo; CREATE OR REPLACE FUNCTION @extschema@.lo_newfunc() … If as an extension author you're kind enough to provide all those 3 scripts and the upgrade setup in the control file, then the user can issue ALTER EXTENSION lo UPGRADE; and have all your cases covered automatically. As an extension author, I can't emphasize enough how much I hate the redundancy of this approach. 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] Optimize PL/Perl function argument passing [PATCH]
On Mon, Jan 31, 2011 at 02:22:37PM -0500, Andrew Dunstan wrote: On 01/15/2011 12:31 AM, Alex Hunsaker wrote: On Tue, Dec 7, 2010 at 07:24, Tim Buncetim.bu...@pobox.com wrote: Changes: Sets the local $_TD via C instead of passing an extra argument. So functions no longer start with our $_TD; local $_TD = shift; Pre-extend stack for trigger arguments for slight performance gain. Passes installcheck. Cool, surprisingly in the non trigger case I saw up to an 18% speedup. That's great. The trigger case remained about the same, I suppose im I/O bound. Find attached a v2 with some minor fixes, If it looks good to you Ill mark this as Ready for Commit. Changes: - move up a declaration to make it c90 safe - avoid using tg_trigger before it was initialized - only extend the stack to the size we need (there was + 1 which unless I am missing something was needed because we used to push $_TD on the stack, but we dont any more) This looks pretty good. But why are we bothering to keep $prolog at all any more, if all we're going to pass it is PL_sv_no all the time? Maybe we'll have a use for it in the future, but right now we don't appear to unless I'm missing something. PostgreSQL::PLPerl::NYTProf would break if it was removed, so I'd rather it wasn't. I could work around that if there's an easy way for perl code to tell what version of PostgreSQL. If there isn't I think it would be worth adding. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Move WAL warning
On Wed, Feb 2, 2011 at 17:43, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 02.02.2011 16:36, Magnus Hagander wrote: When running pg_basebackup with -x to include all transaction log, the server will still throw a warning about xlog archiving if it's not enabled - that is completely irrelevant since pg_basebackup has included it already (and if it was gone, the base backup step itself will fail - actual error and not warning). This patch moves the warning from do_pg_base_backup to pg_base_backup, so it still shows when using the explicit function calls, but goes away when using pg_basebackup. For the sake of consistency, how about moving the pg_stop_backup complete, all required WAL segments have been archived notice too? Well, it goes out as a NOTICE, so by default it doesn't show.. But yeah, for code-consistency it makes sense. Like so, then. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 66cc004..d7559b8 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -8619,6 +8619,14 @@ pg_stop_backup(PG_FUNCTION_ARGS) snprintf(stopxlogstr, sizeof(stopxlogstr), %X/%X, stoppoint.xlogid, stoppoint.xrecoff); + + if (XLogArchivingActive()) + ereport(NOTICE, +(errmsg(pg_stop_backup complete, all required WAL segments have been archived))); + else + ereport(NOTICE, +(errmsg(WAL archiving is not enabled; you must ensure that all required WAL segments are copied through other means to complete the backup))); + PG_RETURN_TEXT_P(cstring_to_text(stopxlogstr)); } @@ -8866,13 +8874,7 @@ do_pg_stop_backup(char *labelfile) but the database backup will not be usable without all the WAL segments.))); } } - - ereport(NOTICE, -(errmsg(pg_stop_backup complete, all required WAL segments have been archived))); } - else - ereport(NOTICE, -(errmsg(WAL archiving is not enabled; you must ensure that all required WAL segments are copied through other means to complete the backup))); /* * We're done. As a convenience, return the ending WAL location. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER EXTENSION UPGRADE, v3
David E. Wheeler da...@kineticode.com writes: On Feb 2, 2011, at 6:45 AM, Dimitri Fontaine wrote: Well, you write 3 scripts. Let's consider an example, the lo contrib, with its 3 objects: CREATE DOMAIN lo AS pg_catalog.oid; CREATE OR REPLACE FUNCTION lo_oid(lo) … CREATE OR REPLACE FUNCTION lo_manage() … Now, the upgrade script from version NULL to 1.0 is alter domain @extschema@.lo set extension lo; alter function @extschema@.lo_oid(lo) set extension lo; alter function @extschema@.lo_manage() set extension lo; The upgrade script from version 1.0 to 2.0 is, let's say: CREATE OR REPLACE FUNCTION @extschema@.lo_newfunc() … So the upgrade script from version NULL to 2.0 is: alter domain @extschema@.lo set extension lo; alter function @extschema@.lo_oid(lo) set extension lo; alter function @extschema@.lo_manage() set extension lo; CREATE OR REPLACE FUNCTION @extschema@.lo_newfunc() … If as an extension author you're kind enough to provide all those 3 scripts and the upgrade setup in the control file, then the user can issue ALTER EXTENSION lo UPGRADE; and have all your cases covered automatically. As an extension author, I can't emphasize enough how much I hate the redundancy of this approach. Well, fair enough I suppose. Or it would be if you gave me an alternative that provides a simpler way to support those 3 upgrades. Of course if that's too much for you, you can also choose to only support upgrades one versions at a time and provide only two scripts. Note also that I don't recall of any proposal on the table that would help with that situation, so I'm all ears. Regards. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] ALTER EXTENSION UPGRADE, v3
On Feb 2, 2011, at 9:03 AM, Dimitri Fontaine wrote: Well, fair enough I suppose. Or it would be if you gave me an alternative that provides a simpler way to support those 3 upgrades. I did: a naming convention with upgrade scripts that have the version number in them. You rejected it. Of course if that's too much for you, you can also choose to only support upgrades one versions at a time and provide only two scripts. Note also that I don't recall of any proposal on the table that would help with that situation, so I'm all ears. http://archives.postgresql.org/pgsql-hackers/2011-01/msg00296.php 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] Optimize PL/Perl function argument passing [PATCH]
On 02/02/2011 11:45 AM, Tim Bunce wrote: But why are we bothering to keep $prolog at all any more, if all we're going to pass it isPL_sv_no all the time? Maybe we'll have a use for it in the future, but right now we don't appear to unless I'm missing something. PostgreSQL::PLPerl::NYTProf would break if it was removed, so I'd rather it wasn't. I could work around that if there's an easy way for perl code to tell what version of PostgreSQL. If there isn't I think it would be worth adding. Not really. It might well be good to add but that needs to wait for another time. I gather you're plugging in a replacement for mkfunc? For now I'll add a comment to the code saying why it's there. 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] arrays as pl/perl input arguments [PATCH]
I'm sorry I'm late to this party. I haven't been keeping up with pgsql-hackers. I'd kind'a hoped that this functionality could be tied into extending PL/Perl to handle named arguments. That way the perl variables corresponding to the named arguments could be given references without breaking any code. Some observations on the current code (based on a quick skim): - I'd like to see the conversion function exposed as a builtin $ref = decode_array_literal({...}); - Every existing plperl function that takes arrays is going to get slower due to the overhead of parsing the string and allocating the array and all its elements. - Some of those functions may not use the array at all and some may simply pass it on as an argument to another function. - Making the conversion lazy would be a big help. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER EXTENSION UPGRADE, v3
David E. Wheeler da...@kineticode.com writes: On Feb 2, 2011, at 9:03 AM, Dimitri Fontaine wrote: Well, fair enough I suppose. Or it would be if you gave me an alternative that provides a simpler way to support those 3 upgrades. I did: a naming convention with upgrade scripts that have the version number in them. You rejected it. I'm sorry, I'm not following. You're proposing to pick one file or another depending on its name. You're not proposing to have less than three files to handle three upgrade setups. You still have to produce the exact same file set. The only difference is that the core code, in your proposal, has to know what is a version number and where to find it in the file names, whereas in mine the core code does not have to assume anything at all about what version numbers look like. Nor to know how do they compare. Oh, and in my current proposal and code, the author can reuse the same file more than once for some upgrade setups, too. Of course if that's too much for you, you can also choose to only support upgrades one versions at a time and provide only two scripts. Note also that I don't recall of any proposal on the table that would help with that situation, so I'm all ears. http://archives.postgresql.org/pgsql-hackers/2011-01/msg00296.php I see there no solution to your reaction here. Please take the time to tell us more about what exactly it is that you hated, and how to make it lovely. We won't make any progress with your current commenting style. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] ALTER EXTENSION UPGRADE, v3
On Feb 2, 2011, at 9:19 AM, Dimitri Fontaine wrote: I see there no solution to your reaction here. Please take the time to tell us more about what exactly it is that you hated, and how to make it lovely. We won't make any progress with your current commenting style. Here is your example of the two upgrade scripts: Now, the upgrade script from version NULL to 1.0 is alter domain @extschema@.lo set extension lo; alter function @extschema@.lo_oid(lo) set extension lo; alter function @extschema@.lo_manage() set extension lo; The upgrade script from version 1.0 to 2.0 is, let's say: CREATE OR REPLACE FUNCTION @extschema@.lo_newfunc() … So the upgrade script from version NULL to 2.0 is: alter domain @extschema@.lo set extension lo; alter function @extschema@.lo_oid(lo) set extension lo; alter function @extschema@.lo_manage() set extension lo; CREATE OR REPLACE FUNCTION @extschema@.lo_newfunc() … They are identical except for the extra line in the second one. If I had, say 15 different versions of an extension, then I'd have 15 upgrade scripts. That's fine. But in your plan, the script to upgrade from version 1 to version 15 would have all the same code as the v14 script, plus any additional. The v14 script would have everything in v13. v13 would have everything in v12. With no support for the equivalent of psql's \i, that's extremely redundant and a huge PITA to maintain. Hence my hate. My proposal would also have 15 upgrade scripts, but each one would only upgrade from the previous one. So to upgrade from v1 to v15, UPGRADE EXTENSION would run all of them. So v15 would only need to have deltas from v14. V14 would need only deltas from v13. Etc. The number of upgrade script files is not the problem I have. It's the redundant content of those files that raises my ire. If there was some way to get something like \i in your approach, that would satisfy me, as it would be only an extra line (at most) in each script. And then, as you note, the core wouldn't need the complexity of understanding version numbers, which I agree would be beneficial. 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] SSI patch version 14
Heikki Linnakangas wrote: Suppressing detail seems easiest. AFAICS all the variable parts are in errdetail. I pushed that with some work on the tests. If you could review the changes to isolationtester.c to confirm that they look sane, I'd appreciate it. http://git.postgresql.org/gitweb?p=users/kgrittn/postgres.git;a=commitdiff;h=2e0254b82a62ead25311f545b0626c97f9ac35b4#patch6 If that part is right, it shouldn't take me very long to finish the specs and capture the expected results. I really appreciate you putting this testing framework together. This is clearly the right way to do it, although we also clearly don't want this test in the check target at the root directory, because of the run time. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSI patch version 14
On 02.02.2011 19:36, Kevin Grittner wrote: Heikki Linnakangas wrote: Suppressing detail seems easiest. AFAICS all the variable parts are in errdetail. I pushed that with some work on the tests. If you could review the changes to isolationtester.c to confirm that they look sane, I'd appreciate it. http://git.postgresql.org/gitweb?p=users/kgrittn/postgres.git;a=commitdiff;h=2e0254b82a62ead25311f545b0626c97f9ac35b4#patch6 If that part is right, it shouldn't take me very long to finish the specs and capture the expected results. Looks good. A comment would be in order to explain why we only print the primary message. (yeah, I know I didn't comment the tester code very well myself) -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSI patch version 14
Heikki Linnakangas wrote: A comment would be in order to explain why we only print the primary message. Done: http://git.postgresql.org/gitweb?p=users/kgrittn/postgres.git;a=commitdiff;h=ddeea22db06ed763b39f716b86db248008a3aa92 -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER EXTENSION UPGRADE, v3
David E. Wheeler da...@kineticode.com writes: They are identical except for the extra line in the second one. If I had, say 15 different versions of an extension, then I'd have 15 upgrade scripts. That's fine. But in your plan, the script to upgrade from version 1 to version 15 would have all the same code as the v14 script, plus any additional. The v14 script would have everything in v13. v13 would have everything in v12. With no support for the equivalent of psql's \i, that's extremely redundant and a huge PITA to maintain. Hence my hate. That's easy enough to manage in your Makefile, really: upgrade.null-v15.sql: upgrade.v14.sql upgrade.v15.sql cat upgrade.v14.sql upgrade.v15.sql $@ There's a difference between what you maintain and what you ship. My proposal would also have 15 upgrade scripts, but each one would only upgrade from the previous one. So to upgrade from v1 to v15, UPGRADE EXTENSION would run all of them. So v15 would only need to have deltas from v14. V14 would need only deltas from v13. Etc. What if you can reuse the later script for upgrading from any previous version, like when the extension only contains CREATE OR REPLACE statements (functions only extension, like adminpack). I don't see benefits in your proposal. The number of upgrade script files is not the problem I have. It's the redundant content of those files that raises my ire. If there was some way to get something like \i in your approach, that would satisfy me, as it would be only an extra line (at most) in each script. And then, as you note, the core wouldn't need the complexity of understanding version numbers, which I agree would be beneficial. It all comes down to the benefits. I don't see any in your proposal. That might be just me though. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] ALTER EXTENSION UPGRADE, v3
On Feb 2, 2011, at 10:04 AM, Dimitri Fontaine wrote: David E. Wheeler da...@kineticode.com writes: They are identical except for the extra line in the second one. If I had, say 15 different versions of an extension, then I'd have 15 upgrade scripts. That's fine. But in your plan, the script to upgrade from version 1 to version 15 would have all the same code as the v14 script, plus any additional. The v14 script would have everything in v13. v13 would have everything in v12. With no support for the equivalent of psql's \i, that's extremely redundant and a huge PITA to maintain. Hence my hate. That's easy enough to manage in your Makefile, really: upgrade.null-v15.sql: upgrade.v14.sql upgrade.v15.sql cat upgrade.v14.sql upgrade.v15.sql $@ Sure, if you know Make really well. But then I need to add a line to the Makefile for every bloody upgrade script. Gross. There's a difference between what you maintain and what you ship. Yes. My proposal would also have 15 upgrade scripts, but each one would only upgrade from the previous one. So to upgrade from v1 to v15, UPGRADE EXTENSION would run all of them. So v15 would only need to have deltas from v14. V14 would need only deltas from v13. Etc. What if you can reuse the later script for upgrading from any previous version, like when the extension only contains CREATE OR REPLACE statements (functions only extension, like adminpack). I don't understand the question. I don't see benefits in your proposal. The benefit is reduced redundancy. It all comes down to the benefits. I don't see any in your proposal. That might be just me though. Could be. 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] WIP: RangeTypes
On Sun, 2011-01-30 at 17:14 -0500, Tom Lane wrote: Jeff Davis pg...@j-davis.com writes: On Sun, 2011-01-30 at 02:55 +, Thom Brown wrote: postgres=# select '[18,20]'::numrange @ 19; ERROR: operator does not exist: numrange @ integer LINE 1: select '[18,20]'::numrange @ 19; ^ HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts. It's because it doesn't know the type on the right side, and assumes it's an int4. Well, yeah, it is an int4. The question ought to be phrased why does the parser fail to promote the int4 to numeric?. There might be some excuse for an operator is not unique here, but I don't understand the above failure --- it should be able to use an implicit coercion from int4 to numeric. The problem exists for arrays, as well, so I think this is just a limitation of the type system. Regards, Jeff Davis postgres=# select ARRAY[1.4,1.5,1.6]::numeric[] || 5.0; ?column? --- {1.4,1.5,1.6,5.0} (1 row) postgres=# select ARRAY[1.4,1.5,1.6]::numeric[] || 5; ERROR: operator does not exist: numeric[] || integer LINE 1: select ARRAY[1.4,1.5,1.6]::numeric[] || 5; ^ HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER EXTENSION UPGRADE, v3
David E. Wheeler da...@kineticode.com writes: upgrade.null-v15.sql: upgrade.v14.sql upgrade.v15.sql cat upgrade.v14.sql upgrade.v15.sql $@ Sure, if you know Make really well. But then I need to add a line to the Makefile for every bloody upgrade script. Gross. Either one line in the Makefile or a new file with the \i equivalent lines, that would maybe look like: SELECT pg_execute_sql_file('upgrade.v14.sql'); SELECT pg_execute_sql_file('upgrade.v15.sql'); So well… I don't see how you've made it less gross here. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] ALTER EXTENSION UPGRADE, v3
On Feb 2, 2011, at 10:14 AM, Aidan Van Dyk wrote: My concern with this approach (upgrade is forced through all intermetiary versions) is that the shared libray now for version 15 *has* to have all the intermediary compatibility for *all* versions in it. So it has to have functions with all symbols so the CREATE ... staements for all previous 15 versions can succeed. With having the $old - $new scripts, the new .so only needs to have functions enough that the DROPs work, and the new CREATE... work. Yeah, so that's another argument for some sort of include syntax, instead, so the upgrade scripts can include other scripts as appropriate. 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] ALTER EXTENSION UPGRADE, v3
On Feb 2, 2011, at 10:22 AM, Dimitri Fontaine wrote: Either one line in the Makefile or a new file with the \i equivalent lines, that would maybe look like: SELECT pg_execute_sql_file('upgrade.v14.sql'); SELECT pg_execute_sql_file('upgrade.v15.sql'); So well… I don't see how you've made it less gross here. I suppose it depends on whether or not you prefer SQL to make. I know where my preferences are. 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] ALTER EXTENSION UPGRADE, v3
David E. Wheeler da...@kineticode.com writes: On Feb 2, 2011, at 10:14 AM, Aidan Van Dyk wrote: My concern with this approach (upgrade is forced through all intermetiary versions) is that the shared libray now for version 15 *has* to have all the intermediary compatibility for *all* versions in it. So it has to have functions with all symbols so the CREATE ... staements for all previous 15 versions can succeed. With having the $old - $new scripts, the new .so only needs to have functions enough that the DROPs work, and the new CREATE... work. Yeah, so that's another argument for some sort of include syntax, instead, so the upgrade scripts can include other scripts as appropriate. That's just the opposite, really. Consider in-house extensions where you perfectly know that you will only upgrade from the previous version. There you only want to ship one upgrade script. Anyway, it's high time that we see some other votes, I think both of us explained only too many times what their own preferences are in terms of what tools to use to maintain and package script files, and how. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] ALTER EXTENSION UPGRADE, v3
On Wed, Feb 2, 2011 at 12:31 PM, David E. Wheeler da...@kineticode.com wrote: They are identical except for the extra line in the second one. If I had, say 15 different versions of an extension, then I'd have 15 upgrade scripts. That's fine. But in your plan, the script to upgrade from version 1 to version 15 would have all the same code as the v14 script, plus any additional. The v14 script would have everything in v13. v13 would have everything in v12. With no support for the equivalent of psql's \i, that's extremely redundant and a huge PITA to maintain. Hence my hate. My proposal would also have 15 upgrade scripts, but each one would only upgrade from the previous one. So to upgrade from v1 to v15, UPGRADE EXTENSION would run all of them. So v15 would only need to have deltas from v14. V14 would need only deltas from v13. Etc. My concern with this approach (upgrade is forced through all intermetiary versions) is that the shared libray now for version 15 *has* to have all the intermediary compatibility for *all* versions in it. So it has to have functions with all symbols so the CREATE ... staements for all previous 15 versions can succeed. With having the $old - $new scripts, the new .so only needs to have functions enough that the DROPs work, and the new CREATE... work. -- Aidan Van Dyk Create like a god, ai...@highrise.ca command like a king, http://www.highrise.ca/ work like a slave. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Some more walsender metadata
On Wed, 2011-02-02 at 14:26 +0100, Magnus Hagander wrote: Attached patch adds some more metadata to walsender results: * Adds the current xlog insert location as a third column in IDENTIFY_SYSTEM * Adds a resultset at the start of a base backup that contains the start xlog location (as returned by do_pg_start_backup) * Adds a resultset at the end of a base backup that contains the end xlog location (as returned by do_pg_stop_backup) These are all information that's interesting for the log receiver version of pg_basebackup (the changes to BASE_BACKUP) and the pg_streamrecv program (the IDENTIFY_SYSTEM) once integrated. I still haven't had the time to clean up those programs enough to submit as a patch yet (it has progressed past that early post from a month ago or so, but isn't done yet), but I wanted to make sure that the backend parts gets reviewed and added as soon as possible. And they're also quite trivial. That way if I don't find time to clean up the streaming mode receiver in time for 9.1, it can be maintained outside of the main tree until 9.2 - but the small backend changes would still be required. These things seem like subsidiary items to your other work, so they should be part of 9.1. I'd like to make a complete set of changes for this release, rather than wait another year. -- Simon Riggs http://www.2ndQuadrant.com/books/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python SPI in subtransactions
You mentioned that you were going to add a few paragraphs to the documentation saying that you can now actually catch SPI errors. I haven't seen that yet. Yeah, I'm procrastinating the doc writing part ;) Will spit something out by the end of the (CET) day. Done, added a small example in the Database Access section. Committed. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Simplify pg_upgrade executable checks
The attached patch simplifies pg_upgrade's checking of executables, per suggestion from Robert Haas. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/contrib/pg_upgrade/exec.c b/contrib/pg_upgrade/exec.c index cce40e4..3ad9dbb 100644 *** a/contrib/pg_upgrade/exec.c --- b/contrib/pg_upgrade/exec.c *** *** 15,22 static void check_data_dir(const char *pg_data); static void check_bin_dir(ClusterInfo *cluster); ! static int check_exec(const char *dir, const char *cmdName); ! static const char *validate_exec(const char *path); /* --- 15,21 static void check_data_dir(const char *pg_data); static void check_bin_dir(ClusterInfo *cluster); ! static void validate_exec(const char *dir, const char *cmdName); /* *** check_data_dir(const char *pg_data) *** 160,217 static void check_bin_dir(ClusterInfo *cluster) { ! check_exec(cluster-bindir, postgres); ! check_exec(cluster-bindir, pg_ctl); ! check_exec(cluster-bindir, pg_resetxlog); if (cluster == new_cluster) { /* these are only needed in the new cluster */ ! check_exec(cluster-bindir, pg_config); ! check_exec(cluster-bindir, psql); ! check_exec(cluster-bindir, pg_dumpall); } } /* - * check_exec() - * - * Checks whether either of the two command names (cmdName and alternative) - * appears to be an executable (in the given directory). If dir/cmdName is - * an executable, this function returns 1. If dir/alternative is an - * executable, this function returns 2. If neither of the given names is - * a valid executable, this function returns 0 to indicated failure. - */ - static int - check_exec(const char *dir, const char *cmdName) - { - char path[MAXPGPATH]; - const char *errMsg; - - snprintf(path, sizeof(path), %s/%s, dir, cmdName); - - if ((errMsg = validate_exec(path)) == NULL) - return 1;/* 1 - first alternative OK */ - else - pg_log(PG_FATAL, check for %s failed - %s\n, cmdName, errMsg); - - return 0; /* 0 - neither alternative is acceptable */ - } - - - /* * validate_exec() * * validate path as an executable file - * returns 0 if the file is found and no error is encountered. - * -1 if the regular file path does not exist or cannot be executed. - * -2 if the file is otherwise valid but cannot be read. */ ! static const char * ! validate_exec(const char *path) { struct stat buf; #ifdef WIN32 /* Win32 requires a .exe suffix for stat() */ char path_exe[MAXPGPATH + sizeof(EXE_EXT) - 1]; --- 159,190 static void check_bin_dir(ClusterInfo *cluster) { ! validate_exec(cluster-bindir, postgres); ! validate_exec(cluster-bindir, pg_ctl); ! validate_exec(cluster-bindir, pg_resetxlog); if (cluster == new_cluster) { /* these are only needed in the new cluster */ ! validate_exec(cluster-bindir, pg_config); ! validate_exec(cluster-bindir, psql); ! validate_exec(cluster-bindir, pg_dumpall); } } /* * validate_exec() * * validate path as an executable file */ ! static void ! validate_exec(const char *dir, const char *cmdName) { + char path[MAXPGPATH]; struct stat buf; + snprintf(path, sizeof(path), %s/%s, dir, cmdName); + #ifdef WIN32 /* Win32 requires a .exe suffix for stat() */ char path_exe[MAXPGPATH + sizeof(EXE_EXT) - 1]; *** validate_exec(const char *path) *** 229,238 * Ensure that the file exists and is a regular file. */ if (stat(path, buf) 0) ! return getErrorText(errno); if (!S_ISREG(buf.st_mode)) ! return not an executable file; /* * Ensure that the file is both executable and readable (required for --- 202,213 * Ensure that the file exists and is a regular file. */ if (stat(path, buf) 0) ! pg_log(PG_FATAL, check for %s failed - %s\n, ! cmdName, getErrorText(errno)); if (!S_ISREG(buf.st_mode)) ! pg_log(PG_FATAL, check for %s failed - not an executable file\n, ! cmdName); /* * Ensure that the file is both executable and readable (required for *** validate_exec(const char *path) *** 240,254 */ #ifndef WIN32 if (access(path, R_OK) != 0) - return can't read file (permission denied); - if (access(path, X_OK) != 0) - return can't execute (permission denied); - return NULL; #else if ((buf.st_mode S_IRUSR) == 0) ! return can't read file (permission denied); if ((buf.st_mode S_IXUSR) == 0) - return can't execute (permission denied); - return NULL; #endif } --- 215,231 */ #ifndef WIN32 if (access(path, R_OK) != 0) #else if ((buf.st_mode S_IRUSR) == 0) ! #endif ! pg_log(PG_FATAL, check for %s failed - cannot read file (permission denied)\n, ! cmdName); ! ! #ifndef WIN32 ! if (access(path, X_OK) != 0) ! #else if
[HACKERS] Elephants
Hi ,I have just shown my support for India's forests. Please see the mail below and support our forests at http://www.greenpeace.org/india/en/What-We-Do/Stop-Climate-Change/Quit-Coal/support-the-forests/ Regards, Dear , The dangerous forest versus coal tussle has claimed its first victim â the endangered Indian elephant. The Chhattisgarh government has scrapped plans for an elephant reserve in order to allow the same forests to be mined for coal. [1] Last year the environment and forest ministry defined regions rich in forest cover and biodiversity as No-Go areas for mining. [2] The coal ministry wants the government to scrap this classification. [3] A Group of Ministers (GoM) constituted to resolve this will take a decision later in the year. [4] These No-Go areas are among the last remaining forests in our country. They are home to wildlife such as tigers, leopards and elephants, and crucial to the livelihoods of millions. [5] If mining is allowed here, we can bid good bye to all of these. We need to pick our sides for this epic match. Big ministries backed by private lobbies are siding with coal â and against the forests. Can you show your support for the elephants and our forests? http://www.greenpeace.org/india/en/What-We-Do/Stop-Climate-Change/Quit-Coal/support-the-forests/ This is the first preparatory step for our massive fight to save our forests and the communities who depend on them. The government needs to know that millions of Indians want their forests protected and declared No-Go for mining. Instead of increasing its dependence on coal the government needs to invest in exploring renewable energy options. [6] Once destroyed, these forests will never get resurrected even with the best reforestation efforts. Clearing forests will only hasten climate change. Coal mining not only destroys forests and displaces communities but also contaminates nearby rivers and water sources.[7] India's remaining forests and wildlife and communities must be protected from mining. Show your support for the elephants and their forests now. http://www.greenpeace.org/india/en/What-We-Do/Stop-Climate-Change/Quit-Coal/support-the-forests/ Thanks a billion! http://timesofindia.indiatimes.com/india/Chhattisgarh-govt-scraps-tusker-reserve-plan-to-please-miners/articleshow/7294639.cms 2. 35% of India's coal mining areas are in 'no go' zones, DNA, March 27, 2010 http://www.dnaindia.com/india/report_35pct-of-india-s-coal-mining-areas-are-in-no-go-zones_1363884 3. Allow mining in 90% no-go zones, Hindusthan Times, October 29 2010 http://www.hindustantimes.com/Allow-mining-in-90-no-go-zones/Article1-619206.aspx 4. GoM will look into sensitive 'no-go' mining areas, Deccan Herald, January 13, 2011 http://www.deccanherald.com/content/128848/cabinet-decides-set-up-gom.html 5. Forests will rescue India, says Jairam Ramesh, new.rediff.com, August 11, 200 http://news.rediff.com/slide-show/2009/aug/11/slide-show-1-forests-are-indias-rescuer-says-jairam-ramesh.htm 6. Energy Revolution, Greenpeace, March 23, 2009 http://www.greenpeace.org/india/Global/india/report/2009/3/energy-revolution.pdf
[HACKERS] Apologizing about the ELEPHANTS email.
Hello all, I am sorry for sending an email about elephants to the hackers list. It was accidental. This mailing list is the only mailing list which I am subscribed to. Most others are on a different email address. Actually I was using some website which said that it was trying to save trees and forests. Being a nature loves, I used the website service to pull down my contacts and send them email about it. I never knew that Gmail had added this mailing list channel email address in the contact list (because I never did this manually). The email was a mistake from my side. I was not aware that this would happen. Since postgres has 'Elephants' as its LOGO / ICON, I apologize in particular about that email and request the moderators / admins to kindly forgive me let me stay in the list. I sincerely apologize for the mistake. I was trying to send email to my friends only. Will be more aware the next time. - Vaibhav (*_*)
Re: [HACKERS] Per-column collation, the finale
On lör, 2011-01-29 at 02:52 -0500, Noah Misch wrote: I'm reviewing this patch as part of CommitFest 2011-01. Thank you very much for this thorough review. This no longer applies cleanly against git master, so I've done my testing against 52713d0. I will send an updated patch soon, when I have figured out the issues discussed below. If you go about supporting non-GNU libc systems, you may want to look at the gnulib locale module[2] for a starting point. I don't have a Mac system, but I'll see about figuring this out. The new documentation is helpful. It would be useful to document the implications of marking your user-defined type COLLATABLE. As best I can tell, you should only set COLLATABLE if functions that would be expected to respect collation, particularly members of a B-tree operator family, do check the fmgr collation. Otherwise, the user might specify collations for data of your type and be surprised to get no differentiated behavior. Are there other considerations? Also, should implementers of COLLATABLE types observe any restrictions on when to respect the collation? For example, is equality permitted to be sensitive to collation? I improved the documentation in the CREATE TYPE man page about this. Note, however, that the questions you raise are not new. There already is a set of types that observe locale information in a set of associated functions. The answers are already hidden somewhere; this feature just exposes them in a different way. Over time, we may want to document more of this, but at the moment it's not clear how much to actually expose. On that note, the following is accepted; should it be? CREATE TABLE parent (key text COLLATE sv_SE); CREATE UNIQUE INDEX ON parent(key COLLATE id_ID); CREATE TABLE child (key text COLLATE tr_TR REFERENCES parent(key)); Does the system expect any invariants to hold on the platform implementation of collations? For example, that they be consistent with equality in some way? If so, would it be practical and/or helpful to provide a tool for testing a given platform locale for conformance? We discussed this previously in this thread. Currently, collation support does not affect equality. Values are only equal if they are bytewise equal. That's why the above is currently OK, but it should probably be restricted in the future. I'll make a note about it. pg_attribute entries for an index currently retain the attcollation of the table columns. They should have the collation of the index columns. At that point, pg_index.indcollation would exist for optimization only. Fixed. Though there's no reason it ought to happen with the first commit, it would be really excellent for make check to detect when the platform has sufficient support to run the collation tests (HAVE_LOCALE_T, UTF8, sv_SE and tr_TR locales). Manual execution is fine for something like numeric_big, but this is so platform-sensitive that testing it widely is important. I solicited ideas about this a while ago, but no one came up with a solution. If we do get Mac or Windows support, it would be a good time to revisit this and devise a platform-independent approach, if possible. It would likewise be nice to have a way to display the collation of an arbitrary expression. A merger of pg_typeof and format_type would cover that. Agreed. I have a follow-up patch pending for that. Four call sites gain code like this (example is from varstr_cmp): +#ifdef HAVE_LOCALE_T + locale_tmylocale = pg_newlocale_from_collation(collid); +#else + check_default_collation(collid); +#endif ... +#ifdef HAVE_LOCALE_T + result = strcoll_l(a1p, a2p, mylocale); +#else result = strcoll(a1p, a2p); +#endif Under !HAVE_LOCALE_T, we could define a locale_t, a pg_newlocale_from_collation that merely calls check_default_collation, #define strcoll_l(a, b, c) strcoll(a, b), etc. I can see six TODO sites with similar needs, and the abstraction would make the mainline code significantly cleaner. Even so, it might not pay off. Thoughts? I had considered that, but I'm mainly hesitant about introducing a fake locale_t type into the public namespace. Maybe it should be wrapped into a pg_locale_t; then we can do whatever we want. Couldn't check_default_collation be a no-op except on assert-only builds? It's currently only called in !HAVE_LOCALE_T branches, in which case initdb will not have added non-default collations. CREATE COLLATION will presumably be made to fail unconditionally on such builds, too. Unless you register the HAVE_LOCALE_T state in pg_control, you need to be prepared to deal with schemas that contain nondefault collations even though the server binary doesn't support it. wchar2char and char2wchar take a collation argument, but they only use it to Assert(!lc_ctype_is_c(collation)). Maybe that's best, but it seems weird. Yeah, it's
Re: [HACKERS] pl/python explicit subtransactions
On tor, 2010-12-23 at 15:32 +0100, Jan Urbański wrote: with plpy.subxact(): plpy.execute(insert into t values (1)) plpy.execute(insert into t values (2)) plpy.execute(ooops) Looks pretty cool, but maybe s/subxact/subtransaction/. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSI patch version 14
Heikki Linnakangas wrote: On 02.02.2011 19:36, Kevin Grittner wrote: If that part is right, it shouldn't take me very long to finish the specs and capture the expected results. Looks good. First cut on the rest of it pushed. I'll want to go over it again to confirm, but I think all dcheck testing is now replicated there. I didn't eliminate dtester/dcheck code yet, in case either of us wanted to compare, but it can go any time now. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Apologizing about the ELEPHANTS email.
On 03/02/11 10:08, Vaibhav Kaushal wrote: Since postgres has 'Elephants' as its LOGO / ICON, I apologize in particular about that email and request the moderators / admins to kindly forgive me let me stay in the list. I sincerely apologize for the mistake. This is probably the best list to accidentally send mail about saving *elephants* to :-) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER EXTENSION UPGRADE, v3
On Wed, Feb 2, 2011 at 12:31 PM, David E. Wheeler da...@kineticode.com wrote: They are identical except for the extra line in the second one. If I had, say 15 different versions of an extension, then I'd have 15 upgrade scripts. That's fine. But in your plan, the script to upgrade from version 1 to version 15 would have all the same code as the v14 script, plus any additional. The v14 script would have everything in v13. v13 would have everything in v12. With no support for the equivalent of psql's \i, that's extremely redundant and a huge PITA to maintain. Hence my hate. Stepping back from the implementation details and file naming conventions a bit, it seems to me that when you do schema upgrades, there are basically three possible things you might want to put in the upgrade script: 1. SQL statements that you want to execute unconditionally, such as (1a) CREATE OR REPLACE FUNCTION on something that has a compatible signature in every prior release in which it exists, or (1b) CREATE TABLE on a table that was added in the most recent release. 2. SQL statements that you want to execute if the version we're upgrading *from* is older than X. For example, CREATE TABLE on a table that was added in version 6 should be executed if we're coming from a version less than 6, and skipped otherwise. 3. SQL statements that you want to execute if the version we're upgrading *from* is between X and Y. This is less common, but you sometimes need it. For example, in version 6 you added a table, but by version 13 it wasn't needed any more so you removed it. The upgrade script for version 17 should drop the table if we're coming from a version between 6 and 12 (if we're coming from pre-6, it was never created to begin with, and we don't want to drop an unrelated table with the same name, and if we're coming from 13-16, it either never existed or, depending on the history, some previous upgrade dropped it). So how could we provide this functionality? Dimitri's approach is simple in concept, but it potentially requires a LOT of bookkeeping when an extension has been around for a while, to make sure that all of the upgrade files contain exactly the right combinations of stuff. I've managed schema upgrades that went through dozens of versions, and making sure that you can correctly upgrade from every previous version to v48 is definitely going to be a challenge. David's approach makes that a little simpler in some ways, but I think it falls down pretty badly on point #3. I'd actually be inclined to just have ONE upgrade file and invent some kind of meta-language for controlling which statements get executed. Just to pick a syntax that everyone will probably hate: [..] -- unconditional stuff [..6] -- stuff to do if coming from pre-7 [..] -- some more unconditional stuff [6..12] -- stuff to do if coming from between 6 and 12 [..] -- a few more unconditional things You might all be either scoffing right now or laughing so hard there are tears running down your face, but in my not insignificant experience that's what real schema upgrade scripts need to cope with in real-world situations, so I hereby pre-reject any comments of the form that should never be necessary in real life because... and/or for that to be necessary you'd have to have done the following bat-shit stupid thing. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER EXTENSION UPGRADE, v3
Aidan Van Dyk ai...@highrise.ca writes: My concern with this approach (upgrade is forced through all intermetiary versions) is that the shared libray now for version 15 *has* to have all the intermediary compatibility for *all* versions in it. So it has to have functions with all symbols so the CREATE ... staements for all previous 15 versions can succeed. Bear in mind though that the compatibility stubs only have to be stubs; the C function needn't do anything except perhaps throw elog(ERROR). This doesn't seem very onerous to me. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] LIKE, CHAR(), and trailing spaces
I found a little LIKE/CHAR() surprise --- below is a table and query against a CHAR(10) field: test= CREATE TABLE test (x char(10)); CREATE TABLE test= INSERT INTO test values ('hi'); INSERT 0 1 test= SELECT * FROM test WHERE x = 'hi'; x hi (1 row) The above works because both sides are converted to 'bpchar'; explain shows that: test= EXPLAIN SELECT * FROM test WHERE x = 'hi'; QUERY PLAN -- Seq Scan on test (cost=0.00..33.12 rows=9 width=14) Filter: (x = 'hi'::bpchar) ^^ (2 rows) The following does not work: test= SELECT * FROM test WHERE x LIKE 'hi'; x --- (0 rows) It seems LIKE is considering the trailing CHAR(10) field spaces as significant, even though our documentations says: Values of type typecharacter/type are physically padded with spaces to the specified width replaceablen/, and are stored and displayed that way. However, the padding spaces are treated as semantically insignificant. Trailing spaces are -- disregarded when comparing two values of type typecharacter/type, and they will be removed when converting a typecharacter/type value to one of the other string types. Note that trailing spaces emphasisare/ semantically significant in typecharacter varying/type and typetext/type values. It says trailing spaces are not significant for character comparisons --- the real question is whether LIKE is a comparison. Obvioiusly '=' is a comparison, but the system does not treat LIKE as a comparison in terms of trailing spaces. Is that desired behavior? I did an EXPLAIN on the query and found '~~' was being used and 'hi' was being converted to text: test= explain select * from test where x like 'hi'; QUERY PLAN -- Seq Scan on test (cost=0.00..33.12 rows=9 width=14) Filter: (x ~~ 'hi'::text) ^^ (2 rows) so I then checked psql \do to see what operators there were for ~~: test= \do ~~ List of operators Schema | Name | Left arg type | Right arg type | Result type | Description +--+---++-+- pg_catalog | ~~ | bytea | bytea | boolean | matches LIKE expression -- pg_catalog | ~~ | character | text | boolean | matches LIKE expression pg_catalog | ~~ | name | text | boolean | matches LIKE expression pg_catalog | ~~ | text | text | boolean | matches LIKE expression (4 rows) The one marked matches the arguments so it seems the comparison being done is not character and character, but character and text. I realize trim() could be used to get the desired behavior, but is our behavior consistent? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] LIKE, CHAR(), and trailing spaces
On 3 February 2011 10:54, Bruce Momjian br...@momjian.us wrote: It seems LIKE is considering the trailing CHAR(10) field spaces as significant, even though our documentations says: -- snip -- It says trailing spaces are not significant for character comparisons --- the real question is whether LIKE is a comparison. Obvioiusly '=' is a comparison, but the system does not treat LIKE as a comparison in terms of trailing spaces. Is that desired behavior? Interesting. I would have to say that from the user point of view, LIKE is definitely a comparison, and if the rest of the operators on bpchar ignore whitespace then LIKE ought to as well. Is the situation the same for regex matches (~ operators)? 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] LIKE, CHAR(), and trailing spaces
Brendan Jurd wrote: On 3 February 2011 10:54, Bruce Momjian br...@momjian.us wrote: It seems LIKE is considering the trailing CHAR(10) field spaces as significant, even though our documentations says: -- snip -- It says trailing spaces are not significant for character comparisons --- the real question is whether LIKE is a comparison. ?Obvioiusly '=' is a comparison, but the system does not treat LIKE as a comparison in terms of trailing spaces. ?Is that desired behavior? Interesting. I would have to say that from the user point of view, LIKE is definitely a comparison, and if the rest of the operators on bpchar ignore whitespace then LIKE ought to as well. Is the situation the same for regex matches (~ operators)? Yes, I think so: test= SELECT 'a'::char(10) ~ 'a$'; ?column? -- f (1 row) test= SELECT 'a'::char(10) ~ 'a *$'; ?column? -- t (1 row) -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] log_checkpoints and restartpoint
On Mon, Jan 31, 2011 at 05:17, Robert Haas robertmh...@gmail.com wrote: The attached patch changes LogCheckpointEnd so that it logs the number of WAL files created/deleted/recycled even in restartpoint. This patch looks good to me. For now, I'm marking it Ready for Committer. Absent objections, I will also commit it. I don't have any objections for the patch, but we might also need to add description about restartpoints into log_checkpoints option. http://developer.postgresql.org/pgdocs/postgres/runtime-config-logging.html#RUNTIME-CONFIG-LOGGING-WHAT -- Itagaki Takahiro -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] log_checkpoints and restartpoint
On Wed, Feb 2, 2011 at 8:16 PM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: On Mon, Jan 31, 2011 at 05:17, Robert Haas robertmh...@gmail.com wrote: The attached patch changes LogCheckpointEnd so that it logs the number of WAL files created/deleted/recycled even in restartpoint. This patch looks good to me. For now, I'm marking it Ready for Committer. Absent objections, I will also commit it. I don't have any objections for the patch, but we might also need to add description about restartpoints into log_checkpoints option. http://developer.postgresql.org/pgdocs/postgres/runtime-config-logging.html#RUNTIME-CONFIG-LOGGING-WHAT Good idea. Committed, with an appropriate (I hope) doc change. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Error code for terminating connection due to conflict with recovery
On Tue, Feb 1, 2011 at 3:17 AM, Simon Riggs si...@2ndquadrant.com wrote: ERRCODE_DATABASE_DROPPED 57P04 looks best So I guess the only remaining issue is whether we should distinguish the error message text, as well as the error codes. Tom was in favor of that upthread, and I am too. Right now we have: else if (RecoveryConflictPending RecoveryConflictRetryable) { pgstat_report_recovery_conflict(RecoveryConflictReason); ereport(FATAL, (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), errmsg(terminating connection due to conflict with recovery), errdetail_recovery_conflict())); } else if (RecoveryConflictPending) { /* Currently there is only one non-retryable recovery conflict */ Assert(RecoveryConflictReason == PROCSIG_RECOVERY_CONFLICT_DATABASE); pgstat_report_recovery_conflict(RecoveryConflictReason); ereport(FATAL, (errcode(ERRCODE_DATABASE_DROPPED), errmsg(terminating connection due to conflict with recovery), errdetail_recovery_conflict())); } The simplest thing to do seems to be to make the second one read terminating connection because the database has been dropped. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] compiler warning
I am seeing the following compiler warning for the past few days: basebackup.c:213: warning: variable `ptr' might be clobbered by `longjmp' or `vfork' and I see this comment in the file: /* * Actually do a base backup for the specified tablespaces. * * This is split out mainly to avoid complaints about variable might be * clobbered by longjmp from stupider versions of gcc. */ Seems that isn't working as expected. I am using: gcc version 2.95.3 20010315 (release) with -O1. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] sepgsql contrib module
2011/1/27 KaiGai Kohei kai...@ak.jp.nec.com: (2011/01/27 22:26), Robert Haas wrote: 2011/1/27 KaiGai Koheikai...@ak.jp.nec.com: - Error messages become obtaining %m, when the error was originated from the libselinux interfaces. It will provides DBA a hint why interactions with SELinux does not work well. No space before the : %m, please. Also this word looks like a typo: seuciryt The attached patch eliminated spaces before : %m, and fixed up the typo. Committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Per-column collation, the finale
On Wed, Feb 02, 2011 at 11:20:44PM +0200, Peter Eisentraut wrote: On l??r, 2011-01-29 at 02:52 -0500, Noah Misch wrote: The new documentation is helpful. It would be useful to document the implications of marking your user-defined type COLLATABLE. As best I can tell, you should only set COLLATABLE if functions that would be expected to respect collation, particularly members of a B-tree operator family, do check the fmgr collation. Otherwise, the user might specify collations for data of your type and be surprised to get no differentiated behavior. Are there other considerations? Also, should implementers of COLLATABLE types observe any restrictions on when to respect the collation? For example, is equality permitted to be sensitive to collation? I improved the documentation in the CREATE TYPE man page about this. Note, however, that the questions you raise are not new. There already is a set of types that observe locale information in a set of associated functions. The answers are already hidden somewhere; this feature just exposes them in a different way. Over time, we may want to document more of this, but at the moment it's not clear how much to actually expose. That's a helpful way to think about it. On that note, the following is accepted; should it be? CREATE TABLE parent (key text COLLATE sv_SE); CREATE UNIQUE INDEX ON parent(key COLLATE id_ID); CREATE TABLE child (key text COLLATE tr_TR REFERENCES parent(key)); Does the system expect any invariants to hold on the platform implementation of collations? For example, that they be consistent with equality in some way? If so, would it be practical and/or helpful to provide a tool for testing a given platform locale for conformance? We discussed this previously in this thread. Currently, collation support does not affect equality. Values are only equal if they are bytewise equal. That's why the above is currently OK, but it should probably be restricted in the future. I'll make a note about it. I see that this is true for texteq, bpchareq and citext_eq. However, nothing stops a user from creating a COLLATABLE type and making the = operator for its B-tree operator class sensitive to collation, right? Could that break any system assumptions? That is, do we need to document that user-defined hash and B-tree operator classes must ignore locale in their = operators? Though there's no reason it ought to happen with the first commit, it would be really excellent for make check to detect when the platform has sufficient support to run the collation tests (HAVE_LOCALE_T, UTF8, sv_SE and tr_TR locales). Manual execution is fine for something like numeric_big, but this is so platform-sensitive that testing it widely is important. I solicited ideas about this a while ago, but no one came up with a solution. If we do get Mac or Windows support, it would be a good time to revisit this and devise a platform-independent approach, if possible. As a rough idea, we could introduce the notion of a skip condition SQL command for a pg_regress test file. When the command throws an error, pg_regress skips that file. For these tests, the skip condition would resemble: SELECT 'foo' COLLATE sv_SE, 'bar' COLLATE tr_TR Four call sites gain code like this (example is from varstr_cmp): +#ifdef HAVE_LOCALE_T + locale_tmylocale = pg_newlocale_from_collation(collid); +#else + check_default_collation(collid); +#endif ... +#ifdef HAVE_LOCALE_T + result = strcoll_l(a1p, a2p, mylocale); +#else result = strcoll(a1p, a2p); +#endif Under !HAVE_LOCALE_T, we could define a locale_t, a pg_newlocale_from_collation that merely calls check_default_collation, #define strcoll_l(a, b, c) strcoll(a, b), etc. I can see six TODO sites with similar needs, and the abstraction would make the mainline code significantly cleaner. Even so, it might not pay off. Thoughts? I had considered that, but I'm mainly hesitant about introducing a fake locale_t type into the public namespace. Maybe it should be wrapped into a pg_locale_t; then we can do whatever we want. True. There are advantages and disadvantages to both. No strong opinion here. Couldn't check_default_collation be a no-op except on assert-only builds? It's currently only called in !HAVE_LOCALE_T branches, in which case initdb will not have added non-default collations. CREATE COLLATION will presumably be made to fail unconditionally on such builds, too. Unless you register the HAVE_LOCALE_T state in pg_control, you need to be prepared to deal with schemas that contain nondefault collations even though the server binary doesn't support it. Makes sense. [local] test=# SELECT prosrc COLLATE id_ID, count(*) FROM pg_proc GROUP BY prosrc; -- worked ... [local] test=# SELECT prosrc
Re: [HACKERS] sepgsql contrib module
On Wed, Feb 2, 2011 at 11:43 PM, Robert Haas robertmh...@gmail.com wrote: Committed. I did some more polishing of the documentation as well. It would be good to have some buildfarm coverage of this code. Can we find anyone brave enough to set up a buildfarm critter using --with-selinux? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers