Re: [HACKERS] SSI patch version 14

2011-02-02 Thread Heikki Linnakangas

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

2011-02-02 Thread Itagaki Takahiro
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

2011-02-02 Thread Dimitri Fontaine
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

2011-02-02 Thread Markus Wanner
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

2011-02-02 Thread Heikki Linnakangas

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

2011-02-02 Thread Heikki Linnakangas

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

2011-02-02 Thread Markus Wanner
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

2011-02-02 Thread Ibrar Ahmed
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

2011-02-02 Thread Magnus Hagander
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

2011-02-02 Thread Steve Singer

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

2011-02-02 Thread Ibrar Ahmed
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

2011-02-02 Thread Kevin Grittner
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

2011-02-02 Thread Heikki Linnakangas

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

2011-02-02 Thread Kevin Grittner
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

2011-02-02 Thread Heikki Linnakangas

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

2011-02-02 Thread Magnus Hagander
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

2011-02-02 Thread Dimitri Fontaine
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

2011-02-02 Thread Marko Tiikkaja

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)

2011-02-02 Thread Dimitri Fontaine
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

2011-02-02 Thread Kevin Grittner
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...

2011-02-02 Thread Bruce Momjian
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?

2011-02-02 Thread Andrew Dunstan


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

2011-02-02 Thread Heikki Linnakangas

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

2011-02-02 Thread Bruce Momjian
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

2011-02-02 Thread Heikki Linnakangas

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

2011-02-02 Thread David E. Wheeler
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]

2011-02-02 Thread Tim Bunce
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

2011-02-02 Thread Magnus Hagander
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

2011-02-02 Thread Dimitri Fontaine
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

2011-02-02 Thread David E. Wheeler
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]

2011-02-02 Thread Andrew Dunstan



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]

2011-02-02 Thread Tim Bunce
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

2011-02-02 Thread Dimitri Fontaine
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

2011-02-02 Thread David E. Wheeler
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

2011-02-02 Thread Kevin Grittner
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

2011-02-02 Thread Heikki Linnakangas

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

2011-02-02 Thread Kevin Grittner
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

2011-02-02 Thread Dimitri Fontaine
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

2011-02-02 Thread David E. Wheeler
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

2011-02-02 Thread Jeff Davis
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

2011-02-02 Thread Dimitri Fontaine
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

2011-02-02 Thread David E. Wheeler
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

2011-02-02 Thread David E. Wheeler
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

2011-02-02 Thread Dimitri Fontaine
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

2011-02-02 Thread Aidan Van Dyk
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

2011-02-02 Thread Simon Riggs
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

2011-02-02 Thread Peter Eisentraut
  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

2011-02-02 Thread Bruce Momjian
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

2011-02-02 Thread vaibhavkaushal123

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.

2011-02-02 Thread Vaibhav Kaushal
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

2011-02-02 Thread Peter Eisentraut
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

2011-02-02 Thread Peter Eisentraut
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

2011-02-02 Thread Kevin Grittner
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.

2011-02-02 Thread Mark Kirkwood

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

2011-02-02 Thread Robert Haas
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

2011-02-02 Thread Tom Lane
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

2011-02-02 Thread Bruce Momjian
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

2011-02-02 Thread Brendan Jurd
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

2011-02-02 Thread Bruce Momjian
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

2011-02-02 Thread Itagaki Takahiro
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

2011-02-02 Thread Robert Haas
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

2011-02-02 Thread Robert Haas
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

2011-02-02 Thread Bruce Momjian
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-02-02 Thread Robert Haas
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

2011-02-02 Thread Noah Misch
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

2011-02-02 Thread Robert Haas
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