Re: [HACKERS] Hostnames in pg_hba.conf

2010-02-12 Thread Bart Samwel
On Fri, Feb 12, 2010 at 02:31, Mark Mielke m...@mark.mielke.cc wrote:

  But once there, it seems clear that packing hostnames or netmasks onto one
 line is just ugly and hard to manage. I'd like to see this extended to any
 of the many ways to allow hostnames to be specified one per line. For
 example:

 set tool_servers {
 127.0.0.1/32
 ::1/128
 1.2.3.4/32
 1.2.3.5/32
 }

 host DATABASE USER $tool_servers md5

 The above features easy parsing capability.

 Of course, then I'll ask for the ability to simplify specifying multiple
 databases:

 set databases {
 db1
 db2
 }

 set users {
 user1
 user2
 }

 host $databases $users $tool_servers md5

 Sorry... :-)


Definitely sounds useful! But I do now see that this is entirely orthogonal
to what I was trying to do -- which means I don't have to do anything about
it. :-)


  I think wildcards are interesting, but I have yet to see an actual use
 case other than it's cool and very generalized. In my mind (tell me if I'm
 wrong), the most common type of PostgreSQL authentication setup is within a
 local network within an organization. There, you either authorize an entire
 subnet (the entire server park or all client PCs) or you authorize
 specific hosts (single IP address). The wildcard case is for replacing the
 first case, but for that case, subnets are usually just fine. I'm trying to
 target the second case here.


 The user case would be an organization with nodes all over the IP space,
 that wants to manage configuration from a single place. DNS would be that
 single place of choice. If moves trust from trust the netmasks to be kept
 up-to-date to trust that DNS will be kept up-to-date. Since DNS has
 important reasons to be up-to-date, it's a pretty safe bet that DNS is equal
 or more up-to-date than pg_hba.conf hard coded netmasks. It makes sense, but
 it can be a later use case. It doesn't have to be in version 1.


DNS is preferred to subnets in that regard, definitely. But again, that
points to the per-hostname route, and it's not a use case for the wildcard
route (unless people explicitly choose to organize their DNS hierarchy so
that they can use it for PostgreSQL authorization -- doubtful.)

Cheers,
Bart


Re: [HACKERS] review: More frame options in window functions

2010-02-12 Thread Dimitri Fontaine
Tom Lane t...@sss.pgh.pa.us writes:
 I think there's the associativity property of operators that we might
 want to have someday, in order for the planner to know some more about
 joins on A = B then on B = C, or replace with  if you will.

 We already do know about that, at least in the case of =.  The reason it
 doesn't do transitive  deductions is not lack of information but doubt
 that it's worth the cycles to try.

Ok. I just remember about some mails here about the problem of
reordering [LEFT] JOINS when we can, but I can't remember if it's really
tied to associativity or some other thing.

Searching the archives ain't helping me refresh those memories. So it
seems the case for an extended opclass infrastructure, or a new side
one, is between thin an non-existent yet?

Regards,
-- 
dim

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


Re: [HACKERS] Parameter name standby_mode

2010-02-12 Thread Fujii Masao
On Fri, Feb 12, 2010 at 4:59 PM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 Agreed. I've changed it now so that if primary_conninfo is not set, it
 doesn't try to establish a streaming connection. If you want to get the
 connection information from environment variables, you can use
 primary_conninfo=''.

OK, you win. I would live with primary_conninfo=''.

And you need to change the document, recovery.conf.sample and
so on.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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


[HACKERS] Thank you Command Prompt

2010-02-12 Thread Devrim GÜNDÜZ
Hi,

A bit offtopic, but...

For who do not read Planet PostgreSQL: I am no longer working for
Command Prompt as of today:

http://people.planetpostgresql.org/devrim/index.php?/archives/33-Bye-bye-Command-Prompt..html
http://people.planetpostgresql.org/devrim/index.php?/archives/34-F.A.Q.-about-latest-news.html

I'd like to thank Command Prompt publicly for the great support over the
years to my community projects, and more. I wish them the best in the
future.

Regards,
-- 
Devrim GÜNDÜZ, RHCE
Command Prompt - http://www.CommandPrompt.com 
devrim~gunduz.org, devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr
http://www.gunduz.org  Twitter: http://twitter.com/devrimgunduz


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


Re: [HACKERS] Parameter name standby_mode

2010-02-12 Thread Joachim Wieland
On Fri, Feb 12, 2010 at 8:59 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 Agreed. I've changed it now so that if primary_conninfo is not set, it
 doesn't try to establish a streaming connection. If you want to get the
 connection information from environment variables, you can use
 primary_conninfo=''.

Why not just remove the default:

If no primary_conninfo variable is set explicitly in the configuration
file, check the environment variables. If the environment variable is
not set, don't try to establish a connection.

?

Joachim

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


[HACKERS] Streaming Replication docs

2010-02-12 Thread Heikki Linnakangas
One glaring issue with the current documentation layout is that the
documentation for the various options in recovery.conf is split in two
places. The old options, restore_command, recovery_target_* and so forth
are in section 24.3.3.1 Recovery Settings, while the new streaming
replication related options are in 25.3.1 Setup [of streaming
replication].

It's pretty clear that we should have a single place that documents all
the recovery.conf options. I suggest that we move the 24.3.3.1 Recovery
 Settings section after High Availability, Load Balancing, and
Replication, and add the new streaming replication related options
there. The previous sections would still briefly describe the most
important settings and give examples, but the new section would be the
authoritative reference page for recovery.conf.

So the new layout would be:

III. Server Administration

...
21. Managing Databases
22. Localization
23. Routine Database Maintenance Tasks
24. Backup and Restore
25. High Availability, Load Balancing, and Replication
*26. Recovery Configuration *
27. Monitoring Database Activity
28. Monitoring Disk Usage
29. Reliability and the Write-Ahead Log
30. Regression Tests

Thoughts, better ideas?


Another change I'd like to make is to document the new built-in way (ie.
standby_mode='on', restore_command='cp ...', primary_conninfo not set)
as the primary way of implementing file-based log shipping. Using
pg_standby or similar scripts that do the waiting would still be
documented, but I'd like to de-emphasize it, moving it into an
Alternative way to implement File-based log shipping section. The
description would go along the lines of An alternative way to implement
File-based log shipping is to leave standby_mode='false', and use a
restore_command that waits until the next WAL file arrives. This offers
more flexibility and control over the process. ...

The reason for that is that it would make the documentation flow better
with Streaming Replication. In a nutshell, there would be three steps to
set up a full streaming replication system:

1. Set up WAL archiving in master (section 24.3 Continuous Archiving
and Point-In-Time Recovery)
2. Use the WAL archive to implement file-based log shipping (Section
25.2 File-based Log Shipping)
3. Add streaming replication to the file-based log shipping (Section
25.3 Streaming Replication) setup.

Each section could then simply refer to the previous step: first set up
... as described in section  The new way of setting up file-based
log shipping is a little bit easier than pg_standby to set up anyway
(not that pg_standby is hard either), so it would be good to describe
the simpler method first anyway.

-- 
  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] Parameter name standby_mode

2010-02-12 Thread Heikki Linnakangas
Joachim Wieland wrote:
 On Fri, Feb 12, 2010 at 8:59 AM, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com wrote:
 Agreed. I've changed it now so that if primary_conninfo is not set, it
 doesn't try to establish a streaming connection. If you want to get the
 connection information from environment variables, you can use
 primary_conninfo=''.
 
 Why not just remove the default:
 
 If no primary_conninfo variable is set explicitly in the configuration
 file, check the environment variables. If the environment variable is
 not set, don't try to establish a connection.

The environment variables in question are the libpq environment
variables like PGHOST, PGPORT. The server shouldn't need to know about
them. Besides, there'd still be the corner case that you really want to
use the built-in defaults, ie. connect to a server running in the same
host at the default port, so you'd not set any environment variables either.

-- 
  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] Confusion over Python drivers {license}

2010-02-12 Thread Federico Di Gregorio
On 12/02/2010 01:00, Jeff Davis wrote:
 * I tried installing psycopg2-2.0.13 and the build system apparently
 doesn't support python3.1, so I assume that psycopg2 doesn't support
 python3 at all.

python3 was almost completely supported some months ago but then I had
to fix some bugs and almost 99% of psycopg users are still with python2
so the python2 branch is in a better shape. But most of the work is done
so, after the next release, I'll start porting changes to the python3
branch (master on git) and finish the work.

federico

-- 
Federico Di Gregorio   f...@initd.org
 Viva il pollo! Viva il pollo! -- sisterconfusion
 Continuo a preferire il coniglio -- vodka



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] TCP keepalive support for libpq

2010-02-12 Thread Peter Geoghegan
 peter.geoghega...@gmail.com wrote:
 Why hasn't libpq had keepalives for years?

 I guess that it's because keepalive doesn't work as expected
 in some cases. For example, if the network outage happens
 before a client sends some packets, keepalive doesn't work,
 then it would have to wait for a long time until it detects
 the outage. This is the specification of linux kernel. So
 a client should not have excessive expectations of keepalive,
 and should have another timeout like QueryTimeout of JDBC.

In my experience, the problems described are common when using libpq
over any sort of flaky connection, which I myself regularly do (not
just with Slony, but with a handheld wi-fi PDT application, where
libpq is used without any wrapper). The slony docs say it takes about
2 hours for the problem to correct itself, but I have found that it
may take a lot longer, perhaps because I have a hybrid Linux/Windows
Slony cluster.

 keepalive doesn't work,
 then it would have to wait for a long time until it detects
 the outage.

I'm not really sure what you mean. In this scenario, would it take as
long as it would have taken had keepalives not been used?

I strongly welcome anything that can ameliorate these problems, which
are probably not noticed by the majority of users, but are a real
inconvenience when they do arise.

Regards,
Peter Geoghegan

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


Re: [HACKERS] testing cvs HEAD - HS/SR - cannot stat

2010-02-12 Thread Heikki Linnakangas
Ok, I committed a patch to reduce the chatter a bit:

Fujii Masao wrote:
 On Fri, Feb 5, 2010 at 3:58 AM, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com wrote:
 LOG:  starting archive recovery
 
 This is reported even if restore_command is not given and the WAL files are
 never restored from the archive. We should get rid of this in that case?

Yeah, removed.

 LOG:  restore_command = 'cp /home/hlinnaka/pgsql.cvshead/walarchive/%f %p'
 LOG:  standby_mode = 'true'
 LOG:  primary_conninfo = 'host=localhost port=5432 user=rep_user 
 password=reppass'
 LOG:  trigger_file = '/tmp/standby-trigger'
 Do we really need to echo all the lines in recovery.conf? That might be
 interesting information, but perhaps it could be condensed and worded
 more nicely.
 
 It's OK for me to move them from LOG to DEBUG.

Done.

 cp: cannot stat 
 `/home/hlinnaka/pgsql.cvshead/walarchive/00010007': No such 
 file or directory
 This is the noise line Josh started this thread with.
 
 Agreed. But the messages other than ENOENT that restore_command emits
 might be useful.
 
 LOG:  automatic recovery in progress
 Ok, so what?
 
 Seems unnecessary.

I replaced this with a more informative message that says either:

entering standby mode, or
starting point-in-time recovery to XID %u, or
starting archive recovery

 LOG:  initializing recovery connections
 Seems like unnecessary noise, doesn't mean anything to a DBA.
 
 Agreed.

Demoted to DEBUG1.

 LOG:  redo starts at 0/700140C
 I guess this could be useful debug information sometimes.
 
 Agreed.

I left this as it was.

 LOG:  consistent recovery state reached at 0/700142C
 It's nice to know that it has reached consistency, but was there any way
 to know before this line that it hadn't been reached yet? Perhaps the
 redo starts line should mention that consistency hasn't been reached yet.
 
 But redo might restart from the consistent database if the standby
 server was shut down after it reached the consistent status.

Yeah, it would be nice to say whether the database is already consistent
or not, but I've left this alone for now.

 Now, should we print a line when we connect to the master successfully?
 Seems like useful information.
 
 Agreed.
 
 Then there's the LOG lines whenever a file is restored successfully from
 archive; are the necessary anymore, now that you can connect to the
 standby and use pg_last_xlog_replay_location() to poll its status?
 
 How about moving those messages from LOG to DEBUG?

Didn't touch these yet.

-- 
  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] Streaming Replication docs

2010-02-12 Thread Fujii Masao
On Fri, Feb 12, 2010 at 6:14 PM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 So the new layout would be:

 III. Server Administration

    ...
    21. Managing Databases
    22. Localization
    23. Routine Database Maintenance Tasks
    24. Backup and Restore
    25. High Availability, Load Balancing, and Replication
 *    26. Recovery Configuration *
    27. Monitoring Database Activity
    28. Monitoring Disk Usage
    29. Reliability and the Write-Ahead Log
    30. Regression Tests

 Thoughts, better ideas?

+1

 Another change I'd like to make is to document the new built-in way (ie.
 standby_mode='on', restore_command='cp ...', primary_conninfo not set)
 as the primary way of implementing file-based log shipping. Using
 pg_standby or similar scripts that do the waiting would still be
 documented, but I'd like to de-emphasize it, moving it into an
 Alternative way to implement File-based log shipping section. The
 description would go along the lines of An alternative way to implement
 File-based log shipping is to leave standby_mode='false', and use a
 restore_command that waits until the next WAL file arrives. This offers
 more flexibility and control over the process. ...

+1

We might need to add the following code of pg_standby into the core,
to prefer it for many cases.

 #ifdef WIN32

   /*
* Windows 'cp' sets the final file size before the 
 copy is
* complete, and not yet ready to be opened by 
 pg_standby. So we
* wait for sleeptime secs before attempting to 
 restore. If that
* is not enough, we will rely on the retry/holdoff 
 mechanism.
* GNUWin32's cp does not have this problem.
*/
   pg_usleep(sleeptime * 100L);
 #endif

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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


Re: [HACKERS] Parameter name standby_mode

2010-02-12 Thread Dimitri Fontaine
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
 There's yet another mode that would be useful with hot standby: start up
 the standby, but don't poll the archive and don't try to connect to the
 master. Kind of 'paused' mode. Simon had functions to do that and more
 in the original hot standby patch.

And having the pause/resume functions would lower the need for perfect
conflict resolution too. When you want to run this huge reporting query
set and not get interrupted, pause the standby. Afterward, resume it.

Of course, while paused, it's not a good HA standby anymore, but you
just did pause it, so you're not surprised, right?

 I've been thinking that this would work with just the three options we
 have now:

I like that, because it exposes exactly the code logic, and it is not
complex enough to merit being hidden from the users. Also, you depend on
understanding how the server really works to setup a trustworthy HA
solution, so exposing the very used concepts is a win.

 primary_conninfo (string) specifies a connection string to use to
 connect to the master. If not given, don't try to connect.

Would it be possible to expose that at the SQL level, so that you can
easily check in scripts what master you're a slave of? Think nagios
cascading alerts or topology graphs, etc.

Regards,
-- 
dim

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


Re: [HACKERS] TCP keepalive support for libpq

2010-02-12 Thread Fujii Masao
On Fri, Feb 12, 2010 at 6:40 PM, Peter Geoghegan
peter.geoghega...@gmail.com wrote:
 keepalive doesn't work,
 then it would have to wait for a long time until it detects
 the outage.

 I'm not really sure what you mean. In this scenario, would it take as
 long as it would have taken had keepalives not been used?

Please see the following threads.
http://archives.postgresql.org/pgsql-bugs/2006-08/msg00098.php
http://lkml.indiana.edu/hypermail/linux/kernel/0508.2/0757.html

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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


Re: [HACKERS] Streaming Replication docs

2010-02-12 Thread Heikki Linnakangas
Fujii Masao wrote:
 We might need to add the following code of pg_standby into the core,
 to prefer it for many cases.
 
 #ifdef WIN32

  /*
   * Windows 'cp' sets the final file size before the 
 copy is
   * complete, and not yet ready to be opened by 
 pg_standby. So we
   * wait for sleeptime secs before attempting to 
 restore. If that
   * is not enough, we will rely on the retry/holdoff 
 mechanism.
   * GNUWin32's cp does not have this problem.
   */
  pg_usleep(sleeptime * 100L);
 #endif

That's actually a bit questionable, always has been even in pg_standby.
It adds a constant 1 s delay to the recovery each WAL file, which
effectively rate-limits the WAL recovery to 16MB per second. I think we
should rather add a warning to the docs, suggesting the copy-then-rename
method on Windows.

-- 
  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] Streaming Replication docs

2010-02-12 Thread Fujii Masao
On Fri, Feb 12, 2010 at 7:15 PM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 That's actually a bit questionable, always has been even in pg_standby.
 It adds a constant 1 s delay to the recovery each WAL file, which
 effectively rate-limits the WAL recovery to 16MB per second. I think we
 should rather add a warning to the docs, suggesting the copy-then-rename
 method on Windows.

Right. Adding a warning to the docs looks enough.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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


Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]

2010-02-12 Thread Tim Bunce
On Fri, Feb 12, 2010 at 12:22:28AM -0500, Robert Haas wrote:
 On Wed, Feb 3, 2010 at 6:41 PM, Andrew Dunstan and...@dunslane.net wrote:
  Tom Lane wrote:
  Andrew Dunstan and...@dunslane.net writes:
 
  %_SHARED has been around for several years now, and if there are genuine
  security concerns about it ISTM they would apply today, regardless of 
  these
  patches.
 
  Yes.  I am not at all happy about inserting nonstandard permissions
  checks into GUC assign hooks --- they are not really meant for that
  and I think there could be unexpected consequences.  Without a serious
  demonstration of a real problem that didn't exist before, I'm not in
  favor of it.
 
  Agreed.
 
  I think a more reasonable answer is just to add a documentation note
  pointing out that %_SHARED should be considered insecure in a multi-user
  database.
 
  Seems fair.
 
  What I was actually wondering about, however, is the extent to which
  the semantics of Perl code could be changed from an on_init hook ---
  is there any equivalent of changing search_path or otherwise creating
  trojan-horse code that might be executed unexpectedly?  And if so is
  there any point in trying to guard against it?  AIUI there isn't
  anything that can be done in on_init that couldn't be done in somebody
  else's function anyhow.
 
  The user won't be able to do anything in the on_init hook that they could
  not do from a plperl function anyway. In fact less, as SPI is not being made
  available.
 
  Plperl functions can't load arbitrary modules at all, and can't modify
  perl's equivalent of search_path. So I think the plain answer to your wonder
  ins no, it can't happen.
 
  There has been discussion in the past about allowing plperl functions access
  to certain modules. There is some sense in doing this, as it would help to
  avoid having to write plperlu for perfectly innocuous operations. But the
  list of allowed modules would have to be carefully controlled in something
  like a SIGHUP setting. We're certainly not going to allow such decisions to
  be made by anyone but the DBA.
 
 The discussion on this seems to have died off.  Andrew, do you have
 something you're planning to commit for this?  I think we're kind of
 down to the level of bikeshedding here...

Following this thread I posted an updated patch:
http://archives.postgresql.org/message-id/20100205134044.go52...@timac.local

which Alex Hunsaker reviewed (and marked Ready For Committer) in
http://archives.postgresql.org/pgsql-hackers/2010-02/msg00391.php

Andrew Dunstan also reviewed it and highlighted some docs issues in
http://archives.postgresql.org/pgsql-hackers/2010-02/msg00488.php

I replied in http://archives.postgresql.org/pgsql-hackers/2010-02/msg00515.php
and posted a further patch update to reflect the needed doc changes in
http://archives.postgresql.org/message-id/20100208204213.gh1...@timac.local

That updated patch is in the commitfest
https://commitfest.postgresql.org/action/patch_view?id=271

From talking to Andrew via IM I understand he's very busy, and also that
he'll be traveling on Sunday and Monday.

I certainly hope he can commit this patch today (along with the next
patch, which is also marked ready for committer) but if not, is there
someone else who could?

What happens to patches marked 'ready for committer' after the
commitfest ends?

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] Parameter name standby_mode

2010-02-12 Thread Fujii Masao
On Fri, Feb 12, 2010 at 4:04 PM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 Fujii Masao wrote:
 On Fri, Feb 12, 2010 at 3:19 PM, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com wrote:
 Fujii Masao wrote:
 But if we fail in restoring the archived WAL file, standby_mode = on
 *always* tries to start streaming replication.
 Hmm, somehow I thought it doesn't if you don't set primary_conninfo. I
 think that's the way it should work, ie. if primary_conninfo is not set,
 don't launch walreceiver but just keep trying to restore from the archive.

 Yeah, even if primary_conninfo is not given, the standby tries to invoke
 walreceiver by using the another connection settings (environment variables
 or defaults). This is intentional behavior, and would make the setup of SR
 easier. So I'd like to leave it be.

 You could do primary_conninfo='' for that.

 Maybe we should have two options, streaming_mode='on' and
 primary_conninfo='...'.

It looks better for me to extend the standby_mode:
For example, standby_mode = 'streaming' or 'archive'.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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


Re: [HACKERS] Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL

2010-02-12 Thread Simon Riggs
On Fri, 2010-02-12 at 14:38 +0900, Fujii Masao wrote:
 On Thu, Feb 11, 2010 at 11:22 PM, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com wrote:
  Simon Riggs wrote:
  Might it not be simpler to add a parameter onto pg_standby?
  We send %s to tell pg_standby the standby_mode of the server which is
  calling it so it can decide how to act in each case.
 
  That would work too, but it doesn't seem any simpler to me. On the contrary.
 
 Agreed.
 
 There could be three kinds of SR configurations. Let's think of them 
 separately.
 
 (1) SR without restore_command
 (2) SR with 'cp'
 (3) SR with pg_standby

Thanks for the explanation.

 (1) is the straightforward configuration. In this case the standby replays 
 only
 the WAL files in pg_xlog directory, and starts SR when it has found the 
 invalid
 record or been able to find no more WAL file. Then if SR is terminated for 
 some
 reasons, the standby would periodically try to connect to the primary and 
 start
 SR again. If you choose this, you don't need to care about the problem 
 discussed
 on the thread.
 
 In the (2) case the standby replays the WAL files in not only pg_xlog but also
 the archive, and starts SR when it has found the invalid record or been able 
 to
 find no more WAL file. If the archive is shared between the primary and the
 standby, the standby might restore the partial WAL file being archived 
 (copied)
 by the primary. This could happen because 'cp' is not an atomic operation.
 
 Currently when the standby finds the WAL file whose file size is less than 
 16MB,
 it emits the FATAL error. This is the problem that I presented upthread. That 
 is
 undesirable behavior, so I proposed to just treat that case the same as if no
 more WAL file is found. If so, the standby can start SR instead of emitting 
 the
 FATAL error. (2) is useful configuration as described in Heikki's
 commig message.
 http://archives.postgresql.org/pgsql-committers/2010-01/msg00395.php

 (3) was unexpected configuration (at least for me). This would work fine as a
 *file-based* log shipping but not SR. Since pg_standby doesn't return when no
 more WAL file is found in the archive (i.e., it waits until the next complete
 WAL file is available), SR will never start. OTOH, since pg_standby treats the
 partial file as nonexistence, the problem discussed on this thread doesn't
 happen.

When we refer to pg_standby we mean any script. pg_standby is just a
reference implementation of a useful script. The discussion shouldn't
really focus on pg_standby, nor should we think of it as a different
approach. My original question was whether we are seeking to remove
pg_standby and, if so, have we implemented all of the technical features
that pg_standby provides? Worryingly the answer seems to be Yes and No.
I don't care if we get rid of pg_standby as long as we retain all the
things it does. *Losing* features is not acceptable.

 Questions:
 (A) Is my proposal for (2) reasonable? For me, Yes.
 (B) Should we allow (3) to work as streaming replication? In fact, we should
 create the new mode that makes pg_standby return as soon as it doesn't 
 find
 a complete WAL file in the archive? I agree with Heikki, i.e., don't think
 it's worth doing. Though pg_standby already has the capability to remove 
 the
 old WAL files, we would still need the cron job that removes them
 periodically
 because pg_standby is not executed during SR is running normally.

Yes, I realised that myself overnight and was going to raise this point
with you today. 

In 8.4 it is pg_standby that was responsible for clearing down the
archive, which is why I suggested using pg_standby for that again. I
agree that will not work. The important thing is not pg_standby but that
we have a valid mechanism for clearing down the archive.

If (2) is a fully supported replication method then we cannot rely on
the existence of an external cron job to clear down the archive. Most
importantly, that cron job would not know the point up to which to clear
down the archive, the information given to pg_standby by %r.

So I suggest that you have a new action that gets called after every
checkpoint to clear down the archive. It will remove all files from the
archive prior to %r. We can implement that as a sequence of unlink()s
from within the server, or we can just call a script to do it. I prefer
the latter approach. However we do it, we need something initiated by
the server to maintain the archive and stop it from overflowing. 

-- 
 Simon Riggs   www.2ndQuadrant.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] Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL

2010-02-12 Thread Heikki Linnakangas
Simon Riggs wrote:
 In 8.4 it is pg_standby that was responsible for clearing down the
 archive, which is why I suggested using pg_standby for that again. I
 agree that will not work. The important thing is not pg_standby but that
 we have a valid mechanism for clearing down the archive.

Good point. When streaming from the master, the standby doesn't call
restore_command, and restore_command doesn't get a chance to clean up
old files.

 So I suggest that you have a new action that gets called after every
 checkpoint to clear down the archive. It will remove all files from the
 archive prior to %r. We can implement that as a sequence of unlink()s
 from within the server, or we can just call a script to do it. I prefer
 the latter approach. However we do it, we need something initiated by
 the server to maintain the archive and stop it from overflowing. 

+1

-- 
  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] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]

2010-02-12 Thread Andrew Dunstan



Tim Bunce wrote:

That updated patch is in the commitfest
https://commitfest.postgresql.org/action/patch_view?id=271

From talking to Andrew via IM I understand he's very busy, and also that
he'll be traveling on Sunday and Monday.

I certainly hope he can commit this patch today (along with the next
patch, which is also marked ready for committer) but if not, is there
someone else who could?
  


Working on it now.


What happens to patches marked 'ready for committer' after the
commitfest ends?


  


It's not the end of the world. But anyway, this will make the cut, and 
possibly your other patch too.


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] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]

2010-02-12 Thread Andrew Dunstan



Robert Haas wrote:

The discussion on this seems to have died off.  Andrew, do you have
something you're planning to commit for this?  I think we're kind of
down to the level of bikeshedding here...


  


There is documentation in Tim's patch I am working on right now. I don't 
think anything else is needed.


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] Confusion over Python drivers

2010-02-12 Thread Tom Lane
Andrew McNamara andr...@object-craft.com.au writes:
 The solution is to write the query in an unambiguous way:
 SELECT $1::date + 1;

 You are missing the point: this is not about what types the SQL execution
 sees. It is about making sure the correct recv function is applied to
 the binary parameter data.

Indeed, and the above locution does set that.

regards, tom lane

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


Re: [HACKERS] review: More frame options in window functions

2010-02-12 Thread Tom Lane
Dimitri Fontaine dfonta...@hi-media.com writes:
 Searching the archives ain't helping me refresh those memories. So it
 seems the case for an extended opclass infrastructure, or a new side
 one, is between thin an non-existent yet?

Yeah, I don't immediately see anything that would justify going to
that level of effort.  Adding +/- as support functions for btree
seems like the thing to do.

regards, tom lane

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


Re: [HACKERS] Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL

2010-02-12 Thread Simon Riggs
On Fri, 2010-02-12 at 12:54 +, Simon Riggs wrote:

 So I suggest that you have a new action that gets called after every
 checkpoint to clear down the archive. It will remove all files from the
 archive prior to %r. We can implement that as a sequence of unlink()s
 from within the server, or we can just call a script to do it. I prefer
 the latter approach. However we do it, we need something initiated by
 the server to maintain the archive and stop it from overflowing. 

Attached patch implements pg_standby for use as an
archive_cleanup_command, reusing existing code with new -a option.

e.g.

archive_cleanup_command = 'pg_standby -a -d /mnt/server/archiverdir %r'

Happy to add the archive_cleanup_command into main server as well, if
you like. Won't take long.

-- 
 Simon Riggs   www.2ndQuadrant.com
Index: contrib/pg_standby/pg_standby.c
===
RCS file: /home/sriggs/pg/REPOSITORY/pgsql/contrib/pg_standby/pg_standby.c,v
retrieving revision 1.27
diff -c -r1.27 pg_standby.c
*** contrib/pg_standby/pg_standby.c	4 Nov 2009 12:51:30 -	1.27
--- contrib/pg_standby/pg_standby.c	12 Feb 2010 14:26:42 -
***
*** 53,58 
--- 53,59 
  int			keepfiles = 0;		/* number of WAL files to keep, 0 keep all */
  int			maxretries = 3;		/* number of retries on restore command */
  bool		debug = false;		/* are we debugging? */
+ bool		cleanup_only = false;		/* -a option new for 9.0 */
  bool		need_cleanup = false;		/* do we need to remove files from
  		 * archive? */
  
***
*** 243,249 
  	/*
  	 * Work out name of prior file from current filename
  	 */
! 	if (nextWALFileType == XLOG_DATA)
  	{
  		int			rc;
  		DIR		   *xldir;
--- 244,250 
  	/*
  	 * Work out name of prior file from current filename
  	 */
! 	if (cleanup_only || nextWALFileType == XLOG_DATA)
  	{
  		int			rc;
  		DIR		   *xldir;
***
*** 334,340 
  		 * these files from archive. This shouldn't happen, but better safe
  		 * than sorry.
  		 */
! 		if (strcmp(restartWALFileName, nextWALFileName)  0)
  			return false;
  
  		strcpy(exclusiveCleanupFileName, restartWALFileName);
--- 335,341 
  		 * these files from archive. This shouldn't happen, but better safe
  		 * than sorry.
  		 */
! 		if (!cleanup_only  strcmp(restartWALFileName, nextWALFileName)  0)
  			return false;
  
  		strcpy(exclusiveCleanupFileName, restartWALFileName);
***
*** 512,518 
  static void
  usage(void)
  {
! 	printf(%s allows PostgreSQL warm standby servers to be configured.\n\n, progname);
  	printf(Usage:\n);
  	printf(  %s [OPTION]... ARCHIVELOCATION NEXTWALFILE XLOGFILEPATH [RESTARTWALFILE]\n, progname);
  	printf(\n
--- 513,519 
  static void
  usage(void)
  {
! 	printf(%s allows PostgreSQL standby servers to be configured.\n\n, progname);
  	printf(Usage:\n);
  	printf(  %s [OPTION]... ARCHIVELOCATION NEXTWALFILE XLOGFILEPATH [RESTARTWALFILE]\n, progname);
  	printf(\n
***
*** 520,526 
--- 521,534 
  		 restore_command = 'pg_standby [OPTION]... ARCHIVELOCATION %%f %%p %%r'\n
  		   e.g.\n
  		 restore_command = 'pg_standby -l /mnt/server/archiverdir %%f %%p %%r'\n);
+ 	printf(  %s -a ARCHIVELOCATION RESTARTWALFILE\n, progname);
+ 	printf(\n
+ 		with main intended use as an archive_cleanup_command in the recovery.conf:\n
+ 		 archive_cleanup_command = 'pg_standby -a ARCHIVELOCATION %%r'\n
+ 		   e.g.\n
+ 		 archive_cleanup_command = 'pg_standby -a -d /mnt/server/archiverdir %%r'\n);
  	printf(\nOptions:\n);
+ 	printf(  -a cleans archive only\n);
  	printf(  -c copies file from archive (default)\n);
  	printf(  -d generate lots of debugging output (testing only)\n);
  	printf(  -k NUMFILESTOKEEP  if RESTARTWALFILE not used, removes files prior to limit\n
***
*** 595,604 
  	(void) signal(SIGQUIT, sigquit_handler);
  #endif
  
! 	while ((c = getopt(argc, argv, cdk:lr:s:t:w:)) != -1)
  	{
  		switch (c)
  		{
  			case 'c':			/* Use copy */
  restoreCommandType = RESTORE_COMMAND_COPY;
  break;
--- 603,615 
  	(void) signal(SIGQUIT, sigquit_handler);
  #endif
  
! 	while ((c = getopt(argc, argv, acdk:lr:s:t:w:)) != -1)
  	{
  		switch (c)
  		{
+ 			case 'a':			/* Cleanup only */
+ cleanup_only = true;
+ break;
  			case 'c':			/* Use copy */
  restoreCommandType = RESTORE_COMMAND_COPY;
  break;
***
*** 684,734 
  		exit(2);
  	}
  
! 	if (optind  argc)
! 	{
! 		nextWALFileName = argv[optind];
! 		optind++;
! 	}
! 	else
  	{
! 		fprintf(stderr, %s: use %%f to specify nextWALFileName\n, progname);
! 		fprintf(stderr, Try \%s --help\ for more information.\n, progname);
! 		exit(2);
  	}
  
  	if (optind  argc)
  	{
! 		xlogFilePath = argv[optind];
  		optind++;
  	}
! 	else
  	{
! 		fprintf(stderr, %s: use %%p to specify xlogFilePath\n, progname);
  		fprintf(stderr, Try \%s 

[HACKERS] Re: [COMMITTERS] pgsql: Reduce the chatter to the log when starting a standby server.

2010-02-12 Thread Simon Riggs
On Fri, 2010-02-12 at 09:49 +, Heikki Linnakangas wrote:
 Log Message:
 ---
 Reduce the chatter to the log when starting a standby server. Don't
 echo all the recovery.conf options. Don't emit the initializing
 recovery connections message, which doesn't mean anything to a user.
 Remove the starting archive recovery message and replace the 
 automatic recovery in progress message with a more informative message
 saying whether the server is doing PITR, normal archive recovery, or
 standby mode.

Not happy with these changes without discussion.

* entering standby mode isn't a more informative message. Two people
have already said on-list that standby mode name might need to be
changed. More informative, for me, would be something like entering
streaming replication mode and having a parameter called replication
would also make that clearer.

* If you change the HS startup messages you need to change the docs also

-- 
 Simon Riggs   www.2ndQuadrant.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] Parameter name standby_mode

2010-02-12 Thread Tom Lane
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
 Joachim Wieland wrote:
 If no primary_conninfo variable is set explicitly in the configuration
 file, check the environment variables. If the environment variable is
 not set, don't try to establish a connection.

 The environment variables in question are the libpq environment
 variables like PGHOST, PGPORT. The server shouldn't need to know about
 them.

Even more to the point is that some of them, like PGPORT, are highly
likely to be set in a server's environment to point to the server
itself.  It would be extremely dangerous to automatically try to start
replication just because we find those set.  In fact, I would argue that
we should fix things so that any such variables inherited from the
server environment are intentionally *NOT* used for making SR
connections.

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] Re: [COMMITTERS] pgsql: Reduce the chatter to the log when starting a standby server.

2010-02-12 Thread Heikki Linnakangas
Simon Riggs wrote:
 * entering standby mode isn't a more informative message. Two people
 have already said on-list that standby mode name might need to be
 changed.

Well, I'm all ears for better suggestions.

 More informative, for me, would be something like entering
 streaming replication mode and having a parameter called replication
 would also make that clearer.

That doesn't accurately describe what the standby_mode setting does. It
doesn't imply streaming replication. It means that the server doesn't
end recovery when it reaches end of WAL, but keeps trying.

 * If you change the HS startup messages you need to change the docs also

Thanks, fixed.

-- 
  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] Re: [COMMITTERS] pgsql: Reduce the chatter to the log when starting a standby server.

2010-02-12 Thread Robert Haas
On Fri, Feb 12, 2010 at 9:55 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 That doesn't accurately describe what the standby_mode setting does. It
 doesn't imply streaming replication. It means that the server doesn't
 end recovery when it reaches end of WAL, but keeps trying.

I think I'm going to add my name to the list of people who are unhappy
with standby_mode as a parameter name.  I really like the design of
the feature, but the name is just not that clear.  You can't read that
parameter name and immediately know what it's trying to do.
Furthermore, if you're wanting to use pg_standby, you might be
forgiven for thinking that you should set standby_mode = on; but in
fact that's exactly the wrong thing to do.

One possibility that occurs to me is that we could call it something
like integrated_standby; but I'm not attached to that.

...Robert

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


Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]

2010-02-12 Thread Robert Haas
On Fri, Feb 12, 2010 at 5:40 AM, Tim Bunce tim.bu...@pobox.com wrote:
 What happens to patches marked 'ready for committer' after the
 commitfest ends?

We talk about it and figure out what to do.  It'll be some combination
of (1) last-minute commits,  (2) postponing things to 9.1, and (3)
rejecting things we don't ever want.

...Robert

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


Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]

2010-02-12 Thread Robert Haas
On Fri, Feb 12, 2010 at 8:56 AM, Andrew Dunstan and...@dunslane.net wrote:
 There is documentation in Tim's patch I am working on right now. I don't
 think anything else is needed.

Cool.

...Robert

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


Re: [HACKERS] Re: [COMMITTERS] pgsql: Reduce the chatter to the log when starting a standby server.

2010-02-12 Thread Heikki Linnakangas
Robert Haas wrote:
 Furthermore, if you're wanting to use pg_standby, you might be
 forgiven for thinking that you should set standby_mode = on; but in
 fact that's exactly the wrong thing to do.

Yeah, I think that's the main weakness of the name standby_mode. It's
pretty descriptive otherwise, we call that mode of operation standby
everywhere, and always have.

I'm not sure I dare to say this out loud after Simon's previous
outburst, but removing or renaming pg_standby would help with that...

 One possibility that occurs to me is that we could call it something
 like integrated_standby; but I'm not attached to that.

Anything with the word 'standby' in it suffers from the same problem,
maybe not as badly but still.

-- 
  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] Re: [COMMITTERS] pgsql: Reduce the chatter to the log when starting a standby server.

2010-02-12 Thread Robert Haas
On Fri, Feb 12, 2010 at 10:27 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 Robert Haas wrote:
 Furthermore, if you're wanting to use pg_standby, you might be
 forgiven for thinking that you should set standby_mode = on; but in
 fact that's exactly the wrong thing to do.

 Yeah, I think that's the main weakness of the name standby_mode. It's
 pretty descriptive otherwise, we call that mode of operation standby
 everywhere, and always have.

 I'm not sure I dare to say this out loud after Simon's previous
 outburst, but removing or renaming pg_standby would help with that...

Not really.  People aren't going to forget about it just because we
remove it from CVS HEAD.

 One possibility that occurs to me is that we could call it something
 like integrated_standby; but I'm not attached to that.

 Anything with the word 'standby' in it suffers from the same problem,
 maybe not as badly but still.

Well, let's come up with something else then.

...Robert

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


Re: [HACKERS] Hostnames in pg_hba.conf

2010-02-12 Thread Peter Eisentraut
On tor, 2010-02-11 at 14:13 +0100, Bart Samwel wrote:
 I've been working on a patch to add hostname support to pg_hba.conf.
 It's not ready for public display yet, but I would just like to run a
 couple of issues / discussion points past everybody.

It might be good to review Apache's hostname-based access control:
http://httpd.apache.org/docs/2.2/mod/mod_authz_host.html#allow


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


Re: [HACKERS] Re: [COMMITTERS] pgsql: Reduce the chatter to the log when starting a standby server.

2010-02-12 Thread Fujii Masao
On Sat, Feb 13, 2010 at 12:28 AM, Robert Haas robertmh...@gmail.com wrote:
 Well, let's come up with something else then.

continuous_recovery ?

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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


Re: [HACKERS] Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL

2010-02-12 Thread Fujii Masao
On Fri, Feb 12, 2010 at 10:10 PM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 So I suggest that you have a new action that gets called after every
 checkpoint to clear down the archive. It will remove all files from the
 archive prior to %r. We can implement that as a sequence of unlink()s
 from within the server, or we can just call a script to do it. I prefer
 the latter approach. However we do it, we need something initiated by
 the server to maintain the archive and stop it from overflowing.

 +1

If we leave executing the remove_command to the bgwriter, the restartpoint
might not happen unfortunately for a long time. To prevent that situation, the
archiver should execute the command, I think. Thought?

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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


Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs

2010-02-12 Thread Boszormenyi Zoltan
Robert Haas írta:
 On Mon, Feb 8, 2010 at 5:53 AM, Boszormenyi Zoltan z...@cybertec.at wrote:
   
 New patch is attached with the discussed changes.
 

 This looks OK to me now, but it lacks docs.

 I'll set it to Waiting on Author.

 ...Robert
   

I added a small change to protocol.sgml for the
CommandComplete message describing the change.
Is it enough?

Best regards,
Zoltán Böszörményi

-- 
Bible has answers for everything. Proof:
But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil. (Matthew 5:37) - basics of digital technology.
May your kingdom come - superficial description of plate tectonics

--
Zoltán Böszörményi
Cybertec Schönig  Schönig GmbH
http://www.postgresql.at/

diff -dcrpN pgsql.orig/doc/src/sgml/protocol.sgml pgsql/doc/src/sgml/protocol.sgml
*** pgsql.orig/doc/src/sgml/protocol.sgml	2010-02-03 11:12:23.0 +0100
--- pgsql/doc/src/sgml/protocol.sgml	2010-02-12 16:44:18.0 +0100
*** CommandComplete (B)
*** ,2227 
--- ,2234 
 /para
  
 para
+ For a commandSELECT [INTO]/command and 
+ commandCREATE TABLE ... AS query/command commands,
+ the tag is literalSELECT replaceablerows/replaceable/literal
+ where replaceablerows/replaceable is the number of rows retrieved.
+/para
+ 
+para
  For a commandMOVE/command command, the tag is
  literalMOVE replaceablerows/replaceable/literal where
  replaceablerows/replaceable is the number of rows the
diff -dcrpN pgsql.orig/src/backend/tcop/pquery.c pgsql/src/backend/tcop/pquery.c
*** pgsql.orig/src/backend/tcop/pquery.c	2010-01-03 12:54:25.0 +0100
--- pgsql/src/backend/tcop/pquery.c	2010-02-08 11:46:33.0 +0100
*** ProcessQuery(PlannedStmt *plan,
*** 205,211 
  		switch (queryDesc-operation)
  		{
  			case CMD_SELECT:
! strcpy(completionTag, SELECT);
  break;
  			case CMD_INSERT:
  if (queryDesc-estate-es_processed == 1)
--- 205,212 
  		switch (queryDesc-operation)
  		{
  			case CMD_SELECT:
! snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! 		 SELECT %u, queryDesc-estate-es_processed);
  break;
  			case CMD_INSERT:
  if (queryDesc-estate-es_processed == 1)
*** PortalRun(Portal portal, long count, boo
*** 714,719 
--- 715,721 
  		  char *completionTag)
  {
  	bool		result;
+ 	uint32		nprocessed;
  	ResourceOwner saveTopTransactionResourceOwner;
  	MemoryContext saveTopTransactionContext;
  	Portal		saveActivePortal;
*** PortalRun(Portal portal, long count, boo
*** 776,814 
  		switch (portal-strategy)
  		{
  			case PORTAL_ONE_SELECT:
- (void) PortalRunSelect(portal, true, count, dest);
- 
- /* we know the query is supposed to set the tag */
- if (completionTag  portal-commandTag)
- 	strcpy(completionTag, portal-commandTag);
- 
- /* Mark portal not active */
- portal-status = PORTAL_READY;
- 
- /*
-  * Since it's a forward fetch, say DONE iff atEnd is now true.
-  */
- result = portal-atEnd;
- break;
- 
  			case PORTAL_ONE_RETURNING:
  			case PORTAL_UTIL_SELECT:
  
  /*
   * If we have not yet run the command, do so, storing its
!  * results in the portal's tuplestore.
   */
! if (!portal-holdStore)
  	FillPortalStore(portal, isTopLevel);
  
  /*
   * Now fetch desired portion of results.
   */
! (void) PortalRunSelect(portal, true, count, dest);
  
! /* we know the query is supposed to set the tag */
  if (completionTag  portal-commandTag)
! 	strcpy(completionTag, portal-commandTag);
  
  /* Mark portal not active */
  portal-status = PORTAL_READY;
--- 778,812 
  		switch (portal-strategy)
  		{
  			case PORTAL_ONE_SELECT:
  			case PORTAL_ONE_RETURNING:
  			case PORTAL_UTIL_SELECT:
  
  /*
   * If we have not yet run the command, do so, storing its
!  * results in the portal's tuplestore. Do this only for the
!  * PORTAL_ONE_RETURNING and PORTAL_UTIL_SELECT cases.
   */
! if ((portal-strategy != PORTAL_ONE_SELECT)  (!portal-holdStore))
  	FillPortalStore(portal, isTopLevel);
  
  /*
   * Now fetch desired portion of results.
   */
! nprocessed = PortalRunSelect(portal, true, count, dest);
  
! /*
!  * If the portal result contains a command tag and the caller
!  * gave us a pointer to store it, copy it. Patch the SELECT
!  * tag to also provide the rowcount.
!  */
  if (completionTag  portal-commandTag)
! {
! 	if (strcmp(portal-commandTag, SELECT) == 0)
! 		snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! 		SELECT %u, nprocessed);
! 	else
! 		strcpy(completionTag, portal-commandTag);
! }
  
  /* Mark portal not active */
  portal-status = PORTAL_READY;
*** PortalRunMulti(Portal portal, bool isTop
*** 1318,1337 

Re: [HACKERS] Re: Faster CREATE DATABASE by delaying fsync (was 8.4.1 ubuntu karmic slow createdb)

2010-02-12 Thread Robert Haas
On Wed, Feb 10, 2010 at 9:27 PM, Andres Freund and...@anarazel.de wrote:
 On Monday 08 February 2010 05:53:23 Robert Haas wrote:
 On Sun, Feb 7, 2010 at 10:09 PM, Alvaro Herrera

 alvhe...@commandprompt.com wrote:
  Andres Freund escribió:
  I personally think the fsync on the directory should be added to the
  stable branches - other opinions?
  If wanted I can prepare patches for that.
 
  Yeah, it seems there are two patches here -- one is the addition of
  fsync_fname() and the other is the fsync_prepare stuff.

 Andres, you want to take a crack at splitting this up?
 I hope I didnt duplicate Gregs work, but I didnt hear back from him, so...

 Everything 8.1 is hopeless because cp is used there... I didnt see it worth
 to replace that. The patch applies cleanly for 8.1 to 8.4 and survives the
 regression tests

 Given pg's heavy commit model I didnt see a point to split the patch for 9.0
 as well...

I'd probably argue for committing this patch to both HEAD and the
back-branches, and doing a second commit with the remaining stuff for
HEAD only, but I don't care very much.

Greg Stark, have you managed to get your access issues sorted out?  If
you like, I can do the actual commit on this one.

...Robert

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


Re: [HACKERS] review: More frame options in window functions

2010-02-12 Thread Alvaro Herrera
Tom Lane escribió:
 Dimitri Fontaine dfonta...@hi-media.com writes:
  Searching the archives ain't helping me refresh those memories. So it
  seems the case for an extended opclass infrastructure, or a new side
  one, is between thin an non-existent yet?
 
 Yeah, I don't immediately see anything that would justify going to
 that level of effort.  Adding +/- as support functions for btree
 seems like the thing to do.

Would it work to use a fake access method instead?  If we add it to
btree, will we be able to backtrack and move that to a separate catalog
if we ever determine that it would have been better?

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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


Re: [HACKERS] TCP keepalive support for libpq

2010-02-12 Thread Marko Kreen
On 2/11/10, Tollef Fog Heen tollef.fog.h...@collabora.co.uk wrote:
  | I disagree. I have clients who have problems with leftover client 
 connections
  | due to server host failures. They do not write apps in C. For a non-default
  | change to be effective we would need to have all the client drivers, eg 
 JDBC,
  | psycopg, DBD-DBI, and the apps like psql make changes to turn it on. Adding
  | this option as a non-default will not really help.


  FWIW, this is my case.  My application uses psycopg, which provides no
  way to get access to the underlying socket.  Sure, I could hack my way
  around this, but from the application writer's point of view, I have a
  connection that I expect to stay around and be reliable.  Whether that
  connection is over a UNIX socket, a TCP socket or something else is
  something I would rather not have to worry about; it feels very much
  like an abstraction violation.

FYI, psycopg does support setting keepalive on fd:

  
http://github.com/markokr/skytools-dev/blob/master/python/skytools/psycopgwrapper.py#L105


The main problem with generic keepalive support is the inconsistencies
between OS'es.  I see 3 ways to handle it:

1) Let user set it on libpq's fd, as now.
2) Give option to set keepalive=on/off, but no timeouts
3) Support all 3 parameters (keepidle, keepintvl, keepcnt)
 and ignore parameters not supported by OS.  Details here:
   http://www.kernel.org/doc/man-pages/online/pages/man7/tcp.7.html
 Linux supports all 3, Windows 2, BSDs only keepidle.

I would prefer 3) or 1) to 2).

-- 
marko

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


Re: [HACKERS] Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL

2010-02-12 Thread Heikki Linnakangas
Fujii Masao wrote:
 On Fri, Feb 12, 2010 at 10:10 PM, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com wrote:
 So I suggest that you have a new action that gets called after every
 checkpoint to clear down the archive. It will remove all files from the
 archive prior to %r. We can implement that as a sequence of unlink()s
 from within the server, or we can just call a script to do it. I prefer
 the latter approach. However we do it, we need something initiated by
 the server to maintain the archive and stop it from overflowing.
 +1
 
 If we leave executing the remove_command to the bgwriter, the restartpoint
 might not happen unfortunately for a long time. 

Are you thinking of a scenario where remove_command gets stuck, and
prevents bgwriter from performing restartpoints while it's stuck? You
have trouble if restore_command gets stuck like that as well, so I think
we can require that the remove_command returns in a reasonable period of
time, ie. in a few minutes.

 To prevent that situation, the
 archiver should execute the command, I think. Thought?

The archiver isn't running in standby, so that's not going to work. And
it's not connected to shared memory either, so it doesn't know what the
latest restartpoint is.

-- 
  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] review: More frame options in window functions

2010-02-12 Thread Tom Lane
Alvaro Herrera alvhe...@commandprompt.com writes:
 Tom Lane escribió:
 Yeah, I don't immediately see anything that would justify going to
 that level of effort.  Adding +/- as support functions for btree
 seems like the thing to do.

 Would it work to use a fake access method instead?

Then you'd have to duplicate all the information in a btree opclass;
*and* teach all the stuff that knows about btree to know about fakeam
instead.  Doesn't seem like there's any win there.

 If we add it to
 btree, will we be able to backtrack and move that to a separate catalog
 if we ever determine that it would have been better?

Backwards compatibility with existing user-type definitions is one big
reason to *not* try to pull ORDER BY information out of btree.

regards, tom lane

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


Re: [HACKERS] TCP keepalive support for libpq

2010-02-12 Thread Euler Taveira de Oliveira
Marko Kreen escreveu:
 3) Support all 3 parameters (keepidle, keepintvl, keepcnt)
  and ignore parameters not supported by OS.
 
+1. AFAIR, we already do that for the backend.


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

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


Re: [HACKERS] Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL

2010-02-12 Thread Dimitri Fontaine
Simon Riggs si...@2ndquadrant.com writes:
 Attached patch implements pg_standby for use as an
 archive_cleanup_command, reusing existing code with new -a option.

 Happy to add the archive_cleanup_command into main server as well, if
 you like. Won't take long.

Would it be possible to have the server do the cleanup (and the cp for
that matter) on its own or use a script?

Either have archive_cleanup = on and either an commented out (empty)
archive_cleanup_command and the same for the restore_command, or a
special magic value, like 'postgresql' to activate the internal one.

This way we have both a zero config working default setup and the
flexibility to adapt to any archiving solution our user might already be
using.

A default archive_command would be nice too. Like you give it a
directory name or a rsync path and it does the basic right thing.

I'm not sure my detailed approach is the right one, but the birdview is
to have the simple case really simple to setup, and the complex cases
still possible. Default included archive, restore and cleanup commands
would be awesome.

Oh, and the basic simple case actually is with a remote standby. Without
NFS or the like, no shared mount point for archiving. 

Regards,
-- 
dim

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


Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs

2010-02-12 Thread Robert Haas
On Fri, Feb 12, 2010 at 10:48 AM, Boszormenyi Zoltan z...@cybertec.at wrote:
 I added a small change to protocol.sgml for the
 CommandComplete message describing the change.
 Is it enough?

Ah, I didn't even see that that section needed to be updated.  Good
catch.  I'd suggest the following wording:

For a commandSELECT/command or commandCREATE TABLE AS/command
command, the tag is SELECT rows... [and the rest as you have it]

We should probably also retitle that section from Retrieving Result
Information for Other Commands to Retrieving Other Result
Information and adjust the text of the opening sentence similarly.

Also I think you need to update the docs for PQcmdtuples to mention
that it not works for SELECT and CTAS.

...Robert

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


Re: [HACKERS] review: More frame options in window functions

2010-02-12 Thread Tom Lane
Hitoshi Harada umi.tan...@gmail.com writes:
 [ more_frame_options patch ]

Committed after rather extensive revisions.

I removed the RANGE value PRECEDING/FOLLOWING support as per discussion,
which was probably a good thing anyway from an incremental-development
standpoint; it made it easier to see what was going on in nodeWindowAgg.

I'm not terribly happy with the changes you made in WinGetFuncArgInPartition
and WinGetFuncArgInFrame to force the window function mark to not go
past frame start in some modes.  Not only is that pretty ugly, but I
think it can mask bugs in window functions: it's an error for a window
function to fetch a row before what it has set its mark to be, but in
some cases that wouldn't be detected because of this change.  I think
it would be better to revert those changes and find another method of
protecting fetches needed to determine the frame head.  One idea is
to create a separate read pointer that tracks the frame head whenever
actual fetches of the frame head might be needed by update_frameheadpos.
I committed it without changing that, but I think this should be
revisited before trying to add the RANGE value PRECEDING/FOLLOWING
options, because those will substantially expand the number of cases
where that hack affects the behavior.

I found one actual bug, which was indeed exposed by the submitted
regression tests:

SELECT sum(unique1) over (rows between 1 following and 3 following),
unique1, four
FROM tenk1 WHERE unique1  10;
 sum | unique1 | four 
-+-+--
   9 |   4 |0
  16 |   2 |2
  23 |   1 |1
  22 |   6 |2
  16 |   9 |1
  15 |   8 |0
  10 |   5 |1
   7 |   3 |3
   0 |   7 |3
   0 |   0 |0
(10 rows)

The last row's SUM ought to be null because there are no rows left in
the frame.  The cause of the bug was that update_frameheadpos was
forcing frameheadpos to not go past the last partition row, but this
has to be allowed so that the frame can be empty at need.

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] Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL

2010-02-12 Thread Greg Stark
so I from by like having the server doing the cleanup because it down by
necessarily have the while picture. it down nt know of it is the only
replica reading these log files our if the site policy is to keep them for
disaster recovery purposes.

I like having this as an return val command though. Or alternately now that
we have read-only replicas you could implement this by polling the slaves
and asking them what log position they are up to.

greg

On 12 Feb 2010 17:25, Dimitri Fontaine dfonta...@hi-media.com wrote:

Simon Riggs si...@2ndquadrant.com writes:

 Attached patch implements pg_standby for use as an
 archive_cleanup_command, reusing existing cod...

 Happy to add the archive_cleanup_command into main server as well, if
 you like. Won't take long
Would it be possible to have the server do the cleanup (and the cp for
that matter) on its own or use a script?

Either have archive_cleanup = on and either an commented out (empty)
archive_cleanup_command and the same for the restore_command, or a
special magic value, like 'postgresql' to activate the internal one.

This way we have both a zero config working default setup and the
flexibility to adapt to any archiving solution our user might already be
using.

A default archive_command would be nice too. Like you give it a
directory name or a rsync path and it does the basic right thing.

I'm not sure my detailed approach is the right one, but the birdview is
to have the simple case really simple to setup, and the complex cases
still possible. Default included archive, restore and cleanup commands
would be awesome.

Oh, and the basic simple case actually is with a remote standby. Without
NFS or the like, no shared mount point for archiving.

Regards,
--
dim


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subs...


Re: [HACKERS] Streaming Replication docs

2010-02-12 Thread Josh Berkus
On 2/12/10 1:14 AM, Heikki Linnakangas wrote:
 It's pretty clear that we should have a single place that documents all
 the recovery.conf options. I suggest that we move the 24.3.3.1 Recovery
  Settings section after High Availability, Load Balancing, and
 Replication, and add the new streaming replication related options
 there. The previous sections would still briefly describe the most
 important settings and give examples, but the new section would be the
 authoritative reference page for recovery.conf.

I'd also suggest that we should have a cross-reference from Server
Runtime Settings in Administration.

--Josh Berkus

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


Re: [HACKERS] knngist patch support

2010-02-12 Thread Robert Haas
2010/2/11 Oleg Bartunov o...@sai.msu.su:
 On Thu, 11 Feb 2010, Robert Haas wrote:

 On Thu, Feb 11, 2010 at 3:00 AM, Oleg Bartunov o...@sai.msu.su wrote:

 version I saw hadn't any documentation whatever.  It's not committable
 on documentation grounds alone, even if everybody was satisfied about
 the code.

 well, there is enough documentation to review patch.

 Where is there any documentation at all?  There are no changes to doc/
 at all; no README; and not even a lengthy comment block anywhere that
 I saw.  Nor did the email in which the patch was submitted clearly lay
 out the design of the feature.

 Well, initial knngist announce
 http://archives.postgresql.org/pgsql-hackers/2009-11/msg01547.php
 isn't enough to review ? We made test data available to reproduce results,
 see http://www.sai.msu.su/~megera/wiki/2009-11-25
 We are here and open to any reviewer's question.

Well, just for example, that doesn't document the changes you made to
the opclass infrastructure, and the reasons for those decisions.
Actually, I've been working on an email on that topic but haven't
gotten around to finishing it.  There's some description of the
overall goal of the feature but not much about how you got there.  Is
it enough to review?  Perhaps, but it would certainly be nice to have
some more discussion of the overall design.  IMHO, anyway.

...Robert

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


Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs

2010-02-12 Thread Boszormenyi Zoltan
Robert Haas írta:
 On Fri, Feb 12, 2010 at 10:48 AM, Boszormenyi Zoltan z...@cybertec.at wrote:
   
 I added a small change to protocol.sgml for the
 CommandComplete message describing the change.
 Is it enough?
 

 Ah, I didn't even see that that section needed to be updated.  Good
 catch.  I'd suggest the following wording:

 For a commandSELECT/command or commandCREATE TABLE AS/command
 command, the tag is SELECT rows... [and the rest as you have it]

 We should probably also retitle that section from Retrieving Result
 Information for Other Commands to Retrieving Other Result
 Information and adjust the text of the opening sentence similarly.

 Also I think you need to update the docs for PQcmdtuples to mention
 that it not works for SELECT and CTAS.
   

Ok, I will update libpq.sgml where this section resides.
What's a CTA, btw? Do you mean CTE, a.k.a. Common Table Expression?

Thanks,
Zoltán Böszörményi

-- 
Bible has answers for everything. Proof:
But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil. (Matthew 5:37) - basics of digital technology.
May your kingdom come - superficial description of plate tectonics

--
Zoltán Böszörményi
Cybertec Schönig  Schönig GmbH
http://www.postgresql.at/


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


Re: [HACKERS] Streaming Replication docs

2010-02-12 Thread Josh Berkus
Heikki,

Crossing this thread over to pgsql-docs, where I think it actually belongs.

In addition to the changes you've proposed, one thing our docs could
really use is a single reference page which we could go to for all of
the .conf files.  Right now, you need to rely on postgresql.org doc
search in order to find, for example, pg_hba.conf.

I think it would be good to put into server administration somewhere a
single page called Configuration Files which references:
postgresql.conf
pg_hba.conf
recovery.conf
pg_ident.conf
... hmmm, am I missing one?

--Josh Berkus

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


Re: [HACKERS] Streaming Replication docs

2010-02-12 Thread Joshua D. Drake
On Fri, 2010-02-12 at 10:22 -0800, Josh Berkus wrote:
 Heikki,
 
 Crossing this thread over to pgsql-docs, where I think it actually belongs.
 
 In addition to the changes you've proposed, one thing our docs could
 really use is a single reference page which we could go to for all of
 the .conf files.  Right now, you need to rely on postgresql.org doc
 search in order to find, for example, pg_hba.conf.
 
 I think it would be good to put into server administration somewhere a
 single page called Configuration Files which references:
   postgresql.conf
   pg_hba.conf
   recovery.conf
   pg_ident.conf
   ... hmmm, am I missing one?


Seems that should go... under Reference

Joshua D. Drake


 
 --Josh Berkus
 


-- 
PostgreSQL.org Major Contributor
Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564
Consulting, Training, Support, Custom Development, Engineering
Respect is earned, not gained through arbitrary and repetitive use or Mr. or 
Sir.


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


Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs

2010-02-12 Thread Boszormenyi Zoltan
Robert Haas írta:
 On Fri, Feb 12, 2010 at 10:48 AM, Boszormenyi Zoltan z...@cybertec.at wrote:
   
 I added a small change to protocol.sgml for the
 CommandComplete message describing the change.
 Is it enough?
 

 Ah, I didn't even see that that section needed to be updated.  Good
 catch.  I'd suggest the following wording:

 For a commandSELECT/command or commandCREATE TABLE AS/command
 command, the tag is SELECT rows... [and the rest as you have it]

 We should probably also retitle that section from Retrieving Result
 Information for Other Commands to Retrieving Other Result
 Information and adjust the text of the opening sentence similarly.

 Also I think you need to update the docs for PQcmdtuples to mention
 that it not works for SELECT and CTAS.
   

I mentioned the WITH command, instead of CTEs.

Best regards,
Zoltán Böszörményi

-- 
Bible has answers for everything. Proof:
But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil. (Matthew 5:37) - basics of digital technology.
May your kingdom come - superficial description of plate tectonics

--
Zoltán Böszörményi
Cybertec Schönig  Schönig GmbH
http://www.postgresql.at/

diff -dcrpN pgsql.orig/doc/src/sgml/libpq.sgml pgsql/doc/src/sgml/libpq.sgml
*** pgsql.orig/doc/src/sgml/libpq.sgml	2010-02-05 12:20:10.0 +0100
--- pgsql/doc/src/sgml/libpq.sgml	2010-02-12 19:27:08.0 +0100
*** typedef struct {
*** 2869,2880 
/sect2
  
sect2 id=libpq-exec-nonselect
!titleRetrieving Result Information for Other Commands/title
  
 para
! These functions are used to extract information from
! structnamePGresult/structname objects that are not
! commandSELECT/ results.
 /para
  
 variablelist
--- 2869,2879 
/sect2
  
sect2 id=libpq-exec-nonselect
!titleRetrieving Other Result Information/title
  
 para
! These functions are used to extract other information from
! structnamePGresult/structname objects.
 /para
  
 variablelist
*** typedef struct {
*** 2925,2936 
 This function returns a string containing the number of rows
 affected by the acronymSQL/ statement that generated the
 structnamePGresult/. This function can only be used following
!the execution of an commandINSERT/, commandUPDATE/,
!commandDELETE/, commandMOVE/, commandFETCH/, or
!commandCOPY/ statement, or an commandEXECUTE/ of a
!prepared query that contains an commandINSERT/,
!commandUPDATE/, or commandDELETE/ statement.  If the
!command that generated the structnamePGresult/ was anything
 else, functionPQcmdTuples/ returns an empty string. The caller
 should not free the return value directly. It will be freed when
 the associated structnamePGresult/ handle is passed to
--- 2924,2935 
 This function returns a string containing the number of rows
 affected by the acronymSQL/ statement that generated the
 structnamePGresult/. This function can only be used following
!the execution of a commandSELECT/, commandWITH/,
!commandINSERT/, commandUPDATE/, commandDELETE/,
!commandMOVE/, commandFETCH/, or commandCOPY/ statement,
!or an commandEXECUTE/ of a prepared query that contains an
!commandINSERT/, commandUPDATE/, or commandDELETE/ statement.
!If the command that generated the structnamePGresult/ was anything
 else, functionPQcmdTuples/ returns an empty string. The caller
 should not free the return value directly. It will be freed when
 the associated structnamePGresult/ handle is passed to
diff -dcrpN pgsql.orig/doc/src/sgml/protocol.sgml pgsql/doc/src/sgml/protocol.sgml
*** pgsql.orig/doc/src/sgml/protocol.sgml	2010-02-03 11:12:23.0 +0100
--- pgsql/doc/src/sgml/protocol.sgml	2010-02-12 19:17:24.0 +0100
*** CommandComplete (B)
*** ,2227 
--- ,2233 
 /para
  
 para
+ For a commandSELECT/command or commandCREATE TABLE AS/command
+ command, the tag is literalSELECT replaceablerows/replaceable/literal
+ where replaceablerows/replaceable is the number of rows retrieved.
+/para
+ 
+para
  For a commandMOVE/command command, the tag is
  literalMOVE replaceablerows/replaceable/literal where
  replaceablerows/replaceable is the number of rows the
diff -dcrpN pgsql.orig/src/backend/tcop/pquery.c pgsql/src/backend/tcop/pquery.c
*** pgsql.orig/src/backend/tcop/pquery.c	2010-01-03 12:54:25.0 +0100
--- pgsql/src/backend/tcop/pquery.c	2010-02-08 11:46:33.0 +0100
*** ProcessQuery(PlannedStmt *plan,
*** 205,211 
  		switch (queryDesc-operation)
  		{
  			case CMD_SELECT:
! strcpy(completionTag, SELECT);
  break;
  			case CMD_INSERT:
  if (queryDesc-estate-es_processed 

[HACKERS] logtrigger issue in PostgreSQL HEAD

2010-02-12 Thread Christopher Browne
Something has evidently changed of late (not quite sure when) which
causes the Slony-I log triggers to break when working against PostreSQL
HEAD.

A more-or-less simplest case is demonstrated thus:

ch...@dba2:/mnt/PostgreSQL/dbs psql slonyregress1
Line style is ascii.
psql (8.5devel)
Type help for help.

slonyregress1=# \d table1
 Table public.table1
 Column |  Type   |  Modifiers  
+-+-
 id | integer | not null default nextval('table1_id_seq'::regclass)
 data   | text| 
Indexes:
table1_pkey PRIMARY KEY, btree (id)
Referenced by:
TABLE table2 CONSTRAINT table2_table1_id_fkey FOREIGN KEY (table1_id) 
REFERENCES table1(id) ON UPDATE CASCADE ON DELETE CASCADE
Triggers:
_slony_regress1_logtrigger_1 AFTER INSERT OR DELETE OR UPDATE ON table1 FOR 
EACH ROW EXECUTE PROCEDURE _slony_regress1.logtrigger('_slony_regress1', '1', 
'kv')

slonyregress1=# delete from table1 where id = 1;
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
! \q

There are no notable compiler complaints about
src/backend/slony_funcs.c, so the mismatch must be at a subtler level
than that of an up-front API change for something.

I suppose a next step might be to kick off gdb against the backend when
processing this.
-- 
output = (cbbrowne @ ca.afilias.info)
Christopher Browne
Bother,  said Pooh,  Eeyore, ready  two photon  torpedoes  and lock
phasers on the Heffalump, Piglet, meet me in transporter room three

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


Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs

2010-02-12 Thread Robert Haas
On Fri, Feb 12, 2010 at 1:22 PM, Boszormenyi Zoltan z...@cybertec.at wrote:
 Robert Haas írta:
 On Fri, Feb 12, 2010 at 10:48 AM, Boszormenyi Zoltan z...@cybertec.at 
 wrote:

 I added a small change to protocol.sgml for the
 CommandComplete message describing the change.
 Is it enough?


 Ah, I didn't even see that that section needed to be updated.  Good
 catch.  I'd suggest the following wording:

 For a commandSELECT/command or commandCREATE TABLE AS/command
 command, the tag is SELECT rows... [and the rest as you have it]

 We should probably also retitle that section from Retrieving Result
 Information for Other Commands to Retrieving Other Result
 Information and adjust the text of the opening sentence similarly.

 Also I think you need to update the docs for PQcmdtuples to mention
 that it not works for SELECT and CTAS.


 Ok, I will update libpq.sgml where this section resides.
 What's a CTA, btw? Do you mean CTE, a.k.a. Common Table Expression?

Sorry, CTAS = CREATE TABLE AS SELECT.

...Robert

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


Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs

2010-02-12 Thread Boszormenyi Zoltan
Robert Haas írta:
 On Fri, Feb 12, 2010 at 1:22 PM, Boszormenyi Zoltan z...@cybertec.at wrote:
   
 Robert Haas írta:
 
 On Fri, Feb 12, 2010 at 10:48 AM, Boszormenyi Zoltan z...@cybertec.at 
 wrote:

   
 I added a small change to protocol.sgml for the
 CommandComplete message describing the change.
 Is it enough?

 
 Ah, I didn't even see that that section needed to be updated.  Good
 catch.  I'd suggest the following wording:

 For a commandSELECT/command or commandCREATE TABLE AS/command
 command, the tag is SELECT rows... [and the rest as you have it]

 We should probably also retitle that section from Retrieving Result
 Information for Other Commands to Retrieving Other Result
 Information and adjust the text of the opening sentence similarly.

 Also I think you need to update the docs for PQcmdtuples to mention
 that it not works for SELECT and CTAS.

   
 Ok, I will update libpq.sgml where this section resides.
 What's a CTA, btw? Do you mean CTE, a.k.a. Common Table Expression?
 

 Sorry, CTAS = CREATE TABLE AS SELECT.
   

Okay, new patch is attached. Please read the docs changes, and comment.

Thanks,
Zoltán Böszörményi

-- 
Bible has answers for everything. Proof:
But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil. (Matthew 5:37) - basics of digital technology.
May your kingdom come - superficial description of plate tectonics

--
Zoltán Böszörményi
Cybertec Schönig  Schönig GmbH
http://www.postgresql.at/

diff -dcrpN pgsql.orig/doc/src/sgml/libpq.sgml pgsql/doc/src/sgml/libpq.sgml
*** pgsql.orig/doc/src/sgml/libpq.sgml	2010-02-05 12:20:10.0 +0100
--- pgsql/doc/src/sgml/libpq.sgml	2010-02-12 19:53:36.0 +0100
*** typedef struct {
*** 2869,2880 
/sect2
  
sect2 id=libpq-exec-nonselect
!titleRetrieving Result Information for Other Commands/title
  
 para
! These functions are used to extract information from
! structnamePGresult/structname objects that are not
! commandSELECT/ results.
 /para
  
 variablelist
--- 2869,2879 
/sect2
  
sect2 id=libpq-exec-nonselect
!titleRetrieving Other Result Information/title
  
 para
! These functions are used to extract other information from
! structnamePGresult/structname objects.
 /para
  
 variablelist
*** typedef struct {
*** 2925,2936 
 This function returns a string containing the number of rows
 affected by the acronymSQL/ statement that generated the
 structnamePGresult/. This function can only be used following
!the execution of an commandINSERT/, commandUPDATE/,
!commandDELETE/, commandMOVE/, commandFETCH/, or
!commandCOPY/ statement, or an commandEXECUTE/ of a
!prepared query that contains an commandINSERT/,
!commandUPDATE/, or commandDELETE/ statement.  If the
!command that generated the structnamePGresult/ was anything
 else, functionPQcmdTuples/ returns an empty string. The caller
 should not free the return value directly. It will be freed when
 the associated structnamePGresult/ handle is passed to
--- 2924,2935 
 This function returns a string containing the number of rows
 affected by the acronymSQL/ statement that generated the
 structnamePGresult/. This function can only be used following
!the execution of a commandSELECT/, commandCREATE TABLE AS/,
!commandINSERT/, commandUPDATE/, commandDELETE/,
!commandMOVE/, commandFETCH/, or commandCOPY/ statement,
!or an commandEXECUTE/ of a prepared query that contains an
!commandINSERT/, commandUPDATE/, or commandDELETE/ statement.
!If the command that generated the structnamePGresult/ was anything
 else, functionPQcmdTuples/ returns an empty string. The caller
 should not free the return value directly. It will be freed when
 the associated structnamePGresult/ handle is passed to
diff -dcrpN pgsql.orig/doc/src/sgml/protocol.sgml pgsql/doc/src/sgml/protocol.sgml
*** pgsql.orig/doc/src/sgml/protocol.sgml	2010-02-03 11:12:23.0 +0100
--- pgsql/doc/src/sgml/protocol.sgml	2010-02-12 19:17:24.0 +0100
*** CommandComplete (B)
*** ,2227 
--- ,2233 
 /para
  
 para
+ For a commandSELECT/command or commandCREATE TABLE AS/command
+ command, the tag is literalSELECT replaceablerows/replaceable/literal
+ where replaceablerows/replaceable is the number of rows retrieved.
+/para
+ 
+para
  For a commandMOVE/command command, the tag is
  literalMOVE replaceablerows/replaceable/literal where
  replaceablerows/replaceable is the number of rows the
diff -dcrpN pgsql.orig/src/backend/tcop/pquery.c pgsql/src/backend/tcop/pquery.c
*** pgsql.orig/src/backend/tcop/pquery.c	2010-01-03 

Re: [HACKERS] logtrigger issue in PostgreSQL HEAD

2010-02-12 Thread Alvaro Herrera
Christopher Browne wrote:

 There are no notable compiler complaints about
 src/backend/slony_funcs.c, so the mismatch must be at a subtler level
 than that of an up-front API change for something.
 
 I suppose a next step might be to kick off gdb against the backend when
 processing this.

Yeah, let's see a backtrace.  Maybe we need to break an API somewhere so
that this becomes more obvious at compile time.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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


Re: [HACKERS] Re: [COMMITTERS] pgsql: Reduce the chatter to the log when starting a standby server.

2010-02-12 Thread Richard Huxton

On 12/02/10 15:37, Fujii Masao wrote:

On Sat, Feb 13, 2010 at 12:28 AM, Robert Haasrobertmh...@gmail.com  wrote:

Well, let's come up with something else then.


continuous_recovery ?


One problem with the otherwise entirely wonderful HS/SR pairing is the 
whole business of the config parameters. They feel too bottom-up. 
Individually, each one makes sense but if you look at them on a page 
they don't say master/slave replication to me.


What about something like:

# Primary
archive_mode = producer
archive_producer_command = 'cp %p .../%f'
max_consumers= 5


# Standby
archive_mode = producer, consumer
archive_producer_command = 'cp %p .../%f'
archive_consumer_command = 'cp %p .../%f'
consume_from = 'host=... user=...'

Three other points that struck me:
1. Why have a separate recovery.conf file rather than just put the 
commands inline? We can use the include directive to have them in a 
separate file if required.
2. Why have a finish.replication file, rather than SELECT 
pg_finish_replication()?


--
  Richard Huxton
  Archonet Ltd

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


Re: [HACKERS] Re: [COMMITTERS] pgsql: Reduce the chatter to the log when starting a standby server.

2010-02-12 Thread Dimitri Fontaine
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
 Robert Haas wrote:
 Furthermore, if you're wanting to use pg_standby, you might be
 forgiven for thinking that you should set standby_mode = on; but in
 fact that's exactly the wrong thing to do.

 Yeah, I think that's the main weakness of the name standby_mode. It's
 pretty descriptive otherwise, we call that mode of operation standby
 everywhere, and always have.

 I'm not sure I dare to say this out loud after Simon's previous
 outburst, but removing or renaming pg_standby would help with that...

Well seen from here it's quite logical: when replaying WALs, you are
either in recovery mode and the server gets back as soon as possible, or
you are setting up a standby server, which will keep recovering until
told to stop doing so.

Now you have 2 main options for keeping your server in standby mode,
either the integrated one (standby_mode = on) or another one. If you
choose to have your standby state managed by an external tool, of course
the first thing to do is tell the server not to maintain itself the
state, so you switch standby_mode to off.

Then you can either use the included contrib pg_standby to achieve the
result, or some other solution, such as Skytools and walmgr.py or CMD
pitrtools.

The fact that the parameter and the external script share the name is a
hint that they're competing for solving the same problem (in different
ways).

Regards,
-- 
dim

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


Re: [HACKERS] Testing with concurrent sessions

2010-02-12 Thread Kevin Grittner
[off-list to avoid distracting from the 9.0 wrap-up effort]
 
Markus Wanner mar...@bluegap.ch wrote:
 Quoting Kevin Grittner kevin.gritt...@wicourts.gov:
 I strongly encourage you to set that up on git.postgresql.org.
 
 I'm about to provide git repositories for Postgres-R anyway, so
 I've setup two projects on git.postgres-r.org:
 
 dtester: that's the driver/harness code
 postgres-dtests: a Postgres clone with the dtester patch applied -
 this is based on the Postgres git repository, so you can easily
 switch between Postgres branches.
 
I just got to the point of having what appears to be a working but
poorly optimized version of serializable transactions, so it is
critical that I create a good set of tests to confirm correct
behavior and monitor for regressions as I optimize.  I see that
you've been working on dtester recently -- should I grab what you've
got here, stick with 0.1, or do you want to package something?  If I
should pull from your git, any hints on the best git statements to
merge that in are welcome, I'm still rather new to git, and I tend
not to get things right on a first try without some hints.  :-/
 
Thanks again for this tool!
 
-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] Testing with concurrent sessions

2010-02-12 Thread Kevin Grittner
I wrote:
 [off-list to avoid distracting from the 9.0 wrap-up effort]
 
Arg.  I really didn't mean that to go to the list.  :-(
 
-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] [PATCH] Provide rowcount for utility SELECTs

2010-02-12 Thread Robert Haas
On Fri, Feb 12, 2010 at 1:55 PM, Boszormenyi Zoltan z...@cybertec.at wrote:
 Okay, new patch is attached. Please read the docs changes, and comment.

I like it.  I'll mark it as Ready for Committer.

...Robert

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


Re: [HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]

2010-02-12 Thread Andrew Dunstan



Alex Hunsaker wrote:

in plc_safe_ok.pl
+use vars qw($PLContainer $SafeClass @EvalInSafe @ShareIntoSafe);

Is there some reason not to use my here?  The only reason I can think
of is you want the *_init gucs to be able to stick things into
@ShareIntoSafe and friends?  And if so should we document that
somewhere (or was that in an earlier patch that i missed?)?
  

It's a step towards providing an interface to give the DBA control over
what gets loaded into the Safe compartment and what gets shared with it.

I opted to not try to define a formal documented interface because I
felt it was likely to be a source of controversy and/or bikeshedding.
This late in the game I didn't want to take that chance.

I'd rather a few brave souls get some experience with it as-as, and collect
some good use-cases, before proposing a formal documented API.



Fine with me.

  


I'm not sure it's fine with me.

I'm a bit inclined to commit the piece of this patch that concerns the 
warnings pragma, and leave the rest for the next release, unless you can 
convince me real fast that we're not opening a back door here. If we're 
going to allow DBAs to add things to the Safe container, it's going to 
be up front or not at all, as far as I'm concerned.


I hope I haven't misunderstood what's going on here.

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] knngist patch support

2010-02-12 Thread Oleg Bartunov

On Fri, 12 Feb 2010, Robert Haas wrote:


2010/2/11 Oleg Bartunov o...@sai.msu.su:
 On Thu, 11 Feb 2010, Robert Haas wrote:

 On Thu, Feb 11, 2010 at 3:00 AM, Oleg Bartunov o...@sai.msu.su wrote:

 version I saw hadn't any documentation whatever. =A0It's not committab=
le
 on documentation grounds alone, even if everybody was satisfied about
 the code.

 well, there is enough documentation to review patch.

 Where is there any documentation at all? =A0There are no changes to doc/
 at all; no README; and not even a lengthy comment block anywhere that
 I saw. =A0Nor did the email in which the patch was submitted clearly lay
 out the design of the feature.

 Well, initial knngist announce
 http://archives.postgresql.org/pgsql-hackers/2009-11/msg01547.php
 isn't enough to review ? We made test data available to reproduce results=
,
 see http://www.sai.msu.su/~megera/wiki/2009-11-25
 We are here and open to any reviewer's question.

Well, just for example, that doesn't document the changes you made to
the opclass infrastructure, and the reasons for those decisions.
Actually, I've been working on an email on that topic but haven't
gotten around to finishing it.  There's some description of the
overall goal of the feature but not much about how you got there.  Is
it enough to review?  Perhaps, but it would certainly be nice to have
some more discussion of the overall design.  IMHO, anyway.


This is not fair,Robert. Everything was discussed in -hackers.I assume reviewer
should follow discussion at least, he is a member of our community. Mailing 
list archive was/is/will our the best knowledge base. For example, regarding

changes in the opclass infrastructure you complain, you can see your reply
to Teodor's message 
http://archives.postgresql.org/message-id/603c8f070912292255u30e1983bi22ed5778bd2ce...@mail.gmail.com
which contains description of amcanorderbyop flag.

Frankly, I think we see here limit of our resources. Let me explain this.
We splitted patch by several parts - 2 parts are about contrib modules
(rather trivial), 1 - is a gist changes, 1 - planner changes and 1 part - 
some proc changes. The most serious are gist and planner patches. We develop

GiST for many years and know almost everything there and could say that we're
responsible for GiST. I don't know if anybody from -hackers could review our
patch for planner better than Tom, but he is busy and will be busy.
So, any serious feature, which touch planner doomed to be rejected because of
lack of reviewer.

We tried to find compromise for 9.0 (Tom suggests contrib module), but all
variants are ugly and bring incompatibility in future. If there are no hackers
willing/capable to review our patches, then, please,  help us  how to save
neighbourhood search without incompatibility in future.


Regards,
Oleg
_
Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru),
Sternberg Astronomical Institute, Moscow University, Russia
Internet: o...@sai.msu.su, http://www.sai.msu.su/~megera/
phone: +007(495)939-16-83, +007(495)939-23-83

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


Re: [HACKERS] knngist patch support

2010-02-12 Thread Robert Haas
On Fri, Feb 12, 2010 at 3:44 PM, Oleg Bartunov o...@sai.msu.su wrote:
 This is not fair,Robert. Everything was discussed in -hackers.I assume
 reviewer
 should follow discussion at least, he is a member of our community. Mailing
 list archive was/is/will our the best knowledge base.

Dude, there's no fair or unfair here; I'm just telling you what I
think.  There are not six people who follow this mailing list more
closely than I do, and when I started looking at this patch it took me
two hours to figure out why you made those changes to the opclass
machinery.  It's not documented anywhere either in the patch or in the
email in which the patch was submitted.  That made it hard for me.  If
that makes me stupid, then I'm stupid, but then probably there are a
lot of stupid people around here.

 For example, regarding
 changes in the opclass infrastructure you complain, you can see your reply
 to Teodor's message
 http://archives.postgresql.org/message-id/603c8f070912292255u30e1983bi22ed5778bd2ce...@mail.gmail.com
 which contains description of amcanorderbyop flag.

OK, so in one email message that is not the email in which you
submitted the patch there is one sentence of explanation that I failed
to remember six weeks later when I looked at the patch.

 Frankly, I think we see here limit of our resources. Let me explain this.
 We splitted patch by several parts - 2 parts are about contrib modules
 (rather trivial), 1 - is a gist changes, 1 - planner changes and 1 part -
 some proc changes. The most serious are gist and planner patches. We develop
 GiST for many years and know almost everything there and could say that
 we're
 responsible for GiST. I don't know if anybody from -hackers could review our
 patch for planner better than Tom, but he is busy and will be busy.
 So, any serious feature, which touch planner doomed to be rejected because
 of
 lack of reviewer.

I do think resource limitations play a role.  For at least as long as
I have been involved in the community, we have relied very heavily on
Tom Lane to review nearly every patch and commit more than half of
them.  As our group of developers grows, that is unsustainable, which
I believe to be part of the reason that -core just created four new
committers; as well as part of the reason for the CommitFest process,
which tries to enlist non-committer reviewers.  But none of us are Tom
Lane.  The part of your patch that took me two hours to figure out
probably would have taken Tom twenty minutes  (maybe less).  But even
if we assume that I am as smart as Tom Lane and that I will spend as
much time working on PostgreSQL as Tom Lane, both of which may well be
false, I won't know the code base as well as he does now until ~2020.

The only way we can get past that is, first, by splitting up the work
across as many people as possible, and second, by making it as easy as
possible for the reviewer to understand what the code is about.  And
at least if the reviewer is me, *documentation helps*.

I'm going to respond to the part of this email that's about moving
this patch forward with a separate email.

...Robert

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


Re: [HACKERS] knngist patch support

2010-02-12 Thread Robert Haas
On Fri, Feb 12, 2010 at 3:44 PM, Oleg Bartunov o...@sai.msu.su wrote:
 We tried to find compromise for 9.0 (Tom suggests contrib module), but all
 variants are ugly and bring incompatibility in future. If there are no
 hackers
 willing/capable to review our patches, then, please,  help us  how to save
 neighbourhood search without incompatibility in future.

Here's what I've gathered from my read through this patch.  Let me
know if it's right:

In CVS HEAD, if an AM is marked amcanorder, that means that the index
defines some kind of intrinsic order over the tuples so that, from a
given starting point, it makes sense to talk about scanning either
forward or backward.  GIST indices don't have an intrinsic notion of
ordering, but the structure of the index does lend itself to finding
index tuples that are nearby to some specified point.  So this patch
wants to introduce the notion of an AM that is marked amcanorderbyop
to accelerate queries that order by indexed column op constant.

To do that, we need some way of recognizing which operators are
candidates for this optimization.  This patch implements that by
putting the relevant operators into the operator class.  That requires
relaxing the rule that operator class operators must return bool; so
instead when the AM is marked amcanorderbyop we allow the operator
class operators to return any type with a default btree operator
class.

Does that sound right?

Tom remarked in another email that he wasn't too happy with the
opclass changes.  They seem kind of grotty to me, too, but I don't
immediately have a better idea.  My fear is that there may be places
in the code that rely on opclass operators only ever returning bool,
and that changing that may break things.  It also feels like allowing
non-bool-returning opclass members only for this one specific case is
kind of a hack: is this an instance of some more general problem that
we ought to be solving in some more general way?  Not sure.

In any case, it seems to me that figuring out how we're going to solve
the problem of marking the operators (or otherwise identifying the
expression trees) that are index-optimizable is the key issue for this
patch.  If we can agree on that, the rest should be a SMOP.

...Robert

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


Re: [HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]

2010-02-12 Thread Alex Hunsaker
On Fri, Feb 12, 2010 at 13:42, Andrew Dunstan and...@dunslane.net wrote:


 Alex Hunsaker wrote:

 in plc_safe_ok.pl
 +use vars qw($PLContainer $SafeClass @EvalInSafe @ShareIntoSafe);
 ...the *_init gucs to be able to stick things into
 @ShareIntoSafe and friends?

 I'm not sure it's fine with me.

 I'm a bit inclined to commit the piece of this patch that concerns the
 warnings pragma,

I think a sizable portion of the patch can be dropped if you do the
above.  Namely the whole double init protection that got added in the
last round.

 and leave the rest for the next release, unless you can
 convince me real fast that we're not opening a back door here. If we're
 going to allow DBAs to add things to the Safe container, it's going to be up
 front or not at all, as far as I'm concerned.

I think backdoor is a  bit extreme.   Yes it could allow people who
can set the plperl.*_init functions to muck with the safe.  As an
admin I could also do that by setting plperl.on_init and overloading
subs in the Safe namespace or switching out Safe.pm.

Anyway reasons I can come up for it are:
-its general so we can add other modules/pragmas easily in the future
-helps with the plperl/plperlu all or nothing situation
-starts to flesh out how an actual exposed (read documented) interface
should look for 9.1

... I know Tim mentioned David as having some use cases (cc'ed)

So my $0.2 is I don't have any strong feelings for/against it.  I
think if we could expose *something* (even if you can only get to it
in a C contrib module) that would be great.  But I realize it might be
impractical at this stage in the game.

*shrug*

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


[HACKERS] xpath improvement V2

2010-02-12 Thread Arie Bikker

Hi all,

I've combined the review suggestions of Jan Urbański, Scott Bailey, and
others.
This was a lot harder, then I had foreseen; and I took my time to do it
the right way (hope you agree!).

BTW. really appreciate your efforts, so far, to enlighten me on nuub 
errors/mistakes in the previous version.


Additional improvement (hence the V2):
two extra functions: xpath_value_text and xpath_value_strict
Both are quite general: xpath_value_text maps everything to text, except 
nodeset. xpath_value_strict has to be told exactly what to expect.

xpath_value_text(text xpath, xml doc [, namespace]) returns text
xpath_value_strict(anyelement typexample, text xpath, xml doc [, 
namespace]) returns anyelement

 See the doc for further explanation.
I chose this approach, as opposed to xpath_value_string/number/boolean 
for a couple of reasons:
- We want postgresql functions, with postgresql types. The functions 
should fit postgresql usage and hide libxml parlance.
- Functions in pg_catalog should be destined for broad use, not just 
satisfy the adhoc desire of the implementer.
- Function synopsis should be adequate to withstand libxml extension or 
improvements (libxml3?)
- Construction of XPath expressions for value retrieval is not trivial. 
A precise calling syntax, hopefully, focuses the user to select the 
right expressions.
- Loose implementation with autocasting opens the door for unwanted 
injection possibilities.


Lastly, when in need of a xpath_value_string (et al), it can easily be 
added by the user through:

CREATE FUNCTION xpath_value_string(text, xml) RETURNS text AS $$
SELECT xpath_value_strict('a'::text,$1,$2);
$$ LANGUAGE SQL;

Points fixed:
- source code indentation. (manually though, can't get pgindent to work
properly)
- Doc entries with some examples
- test/regress entries
- Detailed directions from Jan Urbanski on ereport and PG_TRY/CATCH.

Here's a shortlist of subjects you should definetely review (aka I'me 
not certain these are up to standards)

- calling style in xpath_value_strict with typexample. I really like it,
but I haven't seen such an approach elsewhere in the API.
- tone and style in doc. I'me not native English speaker and had to 
learn DocBook along the way.
- insertion in catalog/pg_proc.h. Just took some free oid numbers, and 
fiddled the other parameters.
- Is ERRCODE_DATA_EXCEPTION (in xml.c) acceptable as error code for a 
type mismatch? Could not find anything better.


kind regards, Arie Bikker

../makepatch

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


Re: [HACKERS] xpath improvement V2

2010-02-12 Thread Arie Bikker

And here is the patch.
*** doc/src/sgml/func.sgml.org	Tue Feb  2 12:53:59 2010
--- doc/src/sgml/func.sgml	Fri Feb 12 21:49:01 2010
***
*** 1,4 
! !-- $PostgreSQL: pgsql/doc/src/sgml/func.sgml,v 1.482.2.2 2009/11/24 19:21:04 petere Exp $ --
  
   chapter id=functions
titleFunctions and Operators/title
--- 1,4 
! !-- $PostgreSQL: pgsql/doc/src/sgml/func.sgml,v 1.482 2009/06/17 21:58:49 tgl Exp $ --
  
   chapter id=functions
titleFunctions and Operators/title
***
*** 821,827 
row
 entryliteralfunctionrandom/function()/literal/entry
 entrytypedp/type/entry
!entryrandom value in the range 0.0 lt;= x lt; 1.0/entry
 entryliteralrandom()/literal/entry
 entry/entry
/row
--- 821,827 
row
 entryliteralfunctionrandom/function()/literal/entry
 entrytypedp/type/entry
!entryrandom value between 0.0 and 1.0, inclusive/entry
 entryliteralrandom()/literal/entry
 entry/entry
/row
***
*** 5251,5259 
   listitem
para
  functionto_char(..., 'ID')/function's day of the week numbering
! matches the functionextract(isodow from ...)/function function, but
  functionto_char(..., 'D')/function's does not match
! functionextract(dow from ...)/function's day numbering.
/para
   /listitem
  
--- 5251,5259 
   listitem
para
  functionto_char(..., 'ID')/function's day of the week numbering
! matches the functionextract('isodow', ...)/function function, but
  functionto_char(..., 'D')/function's does not match
! functionextract('dow', ...)/function's day numbering.
/para
   /listitem
  
***
*** 8464,8474 
 /indexterm
  
 para
! To process values of data type typexml/type, PostgreSQL offers
! the function functionxpath/function, which evaluates XPath 1.0
! expressions.
 /para
  
  synopsis
  functionxpath/function(replaceablexpath/replaceable, replaceablexml/replaceableoptional, replaceablensarray/replaceable/optional)
  /synopsis
--- 8464,8486 
 /indexterm
  
 para
! To retrieve information from an typexml/type document an Xpath 1.0
! expression evaluation can be caried out. The result of this evaluation
! can have several data types depending on the expression and the content
! of the document. The functions implementing this all use an expression
! and a document as input and an optional namespace array.
! They differ in the type of expression they interpret and, consequently, the result they provide.
 /para
  
+sect3 id=functions-xml-processing-nodeset
+ titleNodeset processing/title
+ 
+para
+ To process expressions returning a nodeset, PostgreSQL offers
+ the function functionxpath/function, which returns an  array of
+ typexml/type values.
+/para
+ 
  synopsis
  functionxpath/function(replaceablexpath/replaceable, replaceablexml/replaceableoptional, replaceablensarray/replaceable/optional)
  /synopsis
***
*** 8506,8511 
--- 8518,8699 
  (1 row)
  ]]/screen
 /para
+/sect3
+ 
+sect3 id=functions-xml-processing-values
+ titleValue returning functions/title
+para
+ To retrieve single values from data type typexml/type PostgreSQL offers two
+ functions akin to functionxpath/function.
+ The function functionxpath_value_text/function returns a single
+ as  typetext/type result; the function functionxpath_value_strict/function returns a specific type governed by an input parameter used as type example.
+/para
+ 
+ synopsis
+ functionxpath_value_text/function(replaceablexpath/replaceable, replaceablexml/replaceableoptional, replaceablensarray/replaceable/optional)
+ /synopsis
+ synopsis
+ functionxpath_value_strict/function(replaceabletypexample/replaceable, replaceablexpath/replaceable, replaceablexml/replaceableoptional, replaceablensarray/replaceable/optional)
+ /synopsis
+ 
+para
+ The function functionxpath_value_text/function evaluates the XPath
+ expression replaceablexpath/replaceable against the XML value
+ replaceablexml/replaceable.  It returns a text value
+ corresponding to the evaluation produced by the XPath expression.
+ replaceablexpath/replaceable must be an expression that returns
+ a single value, not a nodeset.
+ replaceablexpath/replaceable expressions resulting in boolean, string or number are supported.
+/para
+ 
+   para
+ The second argument must be a well formed XML document. In particular,
+ it must have a single root node element.
+   /para
+ 
+para
+ The third argument of the function is an array of namespace
+ mappings with the same restrictions as for the replaceablexpath/replaceable function.
+/para
+ 
+para
+ This example will return the tagname of the root element:
+ screen![CDATA[
+ SELECT 

[HACKERS] WITH ... VALUES

2010-02-12 Thread Tom Lane
Came across something interesting while looking at Marko Tiikkaja's
cut-down WITH patch.  I see that our grammar allows a WITH clause in
front of VALUES, and analyze.c makes some effort to process it, but
AFAICT there isn't any actual use case for this because you can't
reference the WITH clause from the body of VALUES:

regression=# with q as (select * from int8_tbl) values (42);
 column1 
-
  42
(1 row)

regression=# with q as (select * from int8_tbl) values (q1);
ERROR:  column q1 does not exist
LINE 1: with q as (select * from int8_tbl) values (q1);
   ^
regression=# with q as (select * from int8_tbl) values (q.q1);
ERROR:  missing FROM-clause entry for table q
LINE 1: with q as (select * from int8_tbl) values (q.q1);
   ^

Even if you go back to 8.4 and turn on add_missing_from, you get:

regression=# with q as (select * from int8_tbl) values (q.q1);
NOTICE:  adding missing FROM-clause entry for table q
LINE 1: with q as (select * from int8_tbl) values (q.q1);
   ^
ERROR:  VALUES must not contain table references
LINE 1: with q as (select * from int8_tbl) values (q.q1);
   ^


So on the whole this seems like useless code.  Perhaps we should instead
throw an error along the line of WITH cannot be attached to VALUES?

regards, tom lane

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


Re: [HACKERS] xpath improvement V2 with xml.c

2010-02-12 Thread Arie Bikker

And a patch for the code implementing this

*** doc/src/sgml/func.sgml.org	Tue Feb  2 12:53:59 2010
--- doc/src/sgml/func.sgml	Fri Feb 12 21:49:01 2010
***
*** 1,4 
! !-- $PostgreSQL: pgsql/doc/src/sgml/func.sgml,v 1.482.2.2 2009/11/24 19:21:04 petere Exp $ --
  
   chapter id=functions
titleFunctions and Operators/title
--- 1,4 
! !-- $PostgreSQL: pgsql/doc/src/sgml/func.sgml,v 1.482 2009/06/17 21:58:49 tgl Exp $ --
  
   chapter id=functions
titleFunctions and Operators/title
***
*** 821,827 
row
 entryliteralfunctionrandom/function()/literal/entry
 entrytypedp/type/entry
!entryrandom value in the range 0.0 lt;= x lt; 1.0/entry
 entryliteralrandom()/literal/entry
 entry/entry
/row
--- 821,827 
row
 entryliteralfunctionrandom/function()/literal/entry
 entrytypedp/type/entry
!entryrandom value between 0.0 and 1.0, inclusive/entry
 entryliteralrandom()/literal/entry
 entry/entry
/row
***
*** 5251,5259 
   listitem
para
  functionto_char(..., 'ID')/function's day of the week numbering
! matches the functionextract(isodow from ...)/function function, but
  functionto_char(..., 'D')/function's does not match
! functionextract(dow from ...)/function's day numbering.
/para
   /listitem
  
--- 5251,5259 
   listitem
para
  functionto_char(..., 'ID')/function's day of the week numbering
! matches the functionextract('isodow', ...)/function function, but
  functionto_char(..., 'D')/function's does not match
! functionextract('dow', ...)/function's day numbering.
/para
   /listitem
  
***
*** 8464,8474 
 /indexterm
  
 para
! To process values of data type typexml/type, PostgreSQL offers
! the function functionxpath/function, which evaluates XPath 1.0
! expressions.
 /para
  
  synopsis
  functionxpath/function(replaceablexpath/replaceable, replaceablexml/replaceableoptional, replaceablensarray/replaceable/optional)
  /synopsis
--- 8464,8486 
 /indexterm
  
 para
! To retrieve information from an typexml/type document an Xpath 1.0
! expression evaluation can be caried out. The result of this evaluation
! can have several data types depending on the expression and the content
! of the document. The functions implementing this all use an expression
! and a document as input and an optional namespace array.
! They differ in the type of expression they interpret and, consequently, the result they provide.
 /para
  
+sect3 id=functions-xml-processing-nodeset
+ titleNodeset processing/title
+ 
+para
+ To process expressions returning a nodeset, PostgreSQL offers
+ the function functionxpath/function, which returns an  array of
+ typexml/type values.
+/para
+ 
  synopsis
  functionxpath/function(replaceablexpath/replaceable, replaceablexml/replaceableoptional, replaceablensarray/replaceable/optional)
  /synopsis
***
*** 8506,8511 
--- 8518,8699 
  (1 row)
  ]]/screen
 /para
+/sect3
+ 
+sect3 id=functions-xml-processing-values
+ titleValue returning functions/title
+para
+ To retrieve single values from data type typexml/type PostgreSQL offers two
+ functions akin to functionxpath/function.
+ The function functionxpath_value_text/function returns a single
+ as  typetext/type result; the function functionxpath_value_strict/function returns a specific type governed by an input parameter used as type example.
+/para
+ 
+ synopsis
+ functionxpath_value_text/function(replaceablexpath/replaceable, replaceablexml/replaceableoptional, replaceablensarray/replaceable/optional)
+ /synopsis
+ synopsis
+ functionxpath_value_strict/function(replaceabletypexample/replaceable, replaceablexpath/replaceable, replaceablexml/replaceableoptional, replaceablensarray/replaceable/optional)
+ /synopsis
+ 
+para
+ The function functionxpath_value_text/function evaluates the XPath
+ expression replaceablexpath/replaceable against the XML value
+ replaceablexml/replaceable.  It returns a text value
+ corresponding to the evaluation produced by the XPath expression.
+ replaceablexpath/replaceable must be an expression that returns
+ a single value, not a nodeset.
+ replaceablexpath/replaceable expressions resulting in boolean, string or number are supported.
+/para
+ 
+   para
+ The second argument must be a well formed XML document. In particular,
+ it must have a single root node element.
+   /para
+ 
+para
+ The third argument of the function is an array of namespace
+ mappings with the same restrictions as for the replaceablexpath/replaceable function.
+/para
+ 
+para
+ This example will return the tagname of the root element:
+ 

Re: [HACKERS] WITH ... VALUES

2010-02-12 Thread Jeff Davis
On Fri, 2010-02-12 at 16:59 -0500, Tom Lane wrote:
 Came across something interesting while looking at Marko Tiikkaja's
 cut-down WITH patch.  I see that our grammar allows a WITH clause in
 front of VALUES, and analyze.c makes some effort to process it, but
 AFAICT there isn't any actual use case for this because you can't
 reference the WITH clause from the body of VALUES:

  create table tmp(a int);
  insert into tmp values(2);
  with tmp2 as (select a + 1 as b from tmp)
values((select b from tmp2));


   column1 
  -
 3
  (1 row)


Regards,
Jeff Davis


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


Re: [HACKERS] Confusion over Python drivers {license}

2010-02-12 Thread Jeff Davis
On Fri, 2010-02-12 at 10:38 +0100, Federico Di Gregorio wrote:
 On 12/02/2010 01:00, Jeff Davis wrote:
  * I tried installing psycopg2-2.0.13 and the build system apparently
  doesn't support python3.1, so I assume that psycopg2 doesn't support
  python3 at all.
 
 python3 was almost completely supported some months ago but then I had
 to fix some bugs and almost 99% of psycopg users are still with python2
 so the python2 branch is in a better shape. But most of the work is done
 so, after the next release, I'll start porting changes to the python3
 branch (master on git) and finish the work.

Ok, sounds great.

Do you attempt to handle the encoding issues when working with python3?
I'd be interested to see your approach, because I didn't get much
feedback on the doc I wrote.

Regards,
Jeff Davis


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


Re: [HACKERS] WITH ... VALUES

2010-02-12 Thread Tom Lane
Jeff Davis pg...@j-davis.com writes:
 On Fri, 2010-02-12 at 16:59 -0500, Tom Lane wrote:
 AFAICT there isn't any actual use case for this because you can't
 reference the WITH clause from the body of VALUES:

   with tmp2 as (select a + 1 as b from tmp)
 values((select b from tmp2));

Ah, sneaky.  Never mind that then.

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] PostgreSQL::PLPerl::Call - Simple interface for calling SQL functions from PostgreSQL PL/Perl

2010-02-12 Thread Tim Bunce
There was some discussion a few weeks ago about inter-stored-procedure
calling from PL/Perl.

I thought I'd post the documentation (and tests) for a module I'm
working on to simplify calling SQL functions from PL/Perl.

Here are some real-world examples (not the best code, but genuine
use-cases):

Calling a function that returns a single value (single column):
Old:
$count_sql = spi_exec_query(SELECT * FROM tl_activity_stats_sql('
. $to_array(statistic= $stat, person_id = $lead-{person_id})
. '::text[], $debug))-{rows}-[0]-{tl_activity_stats_sql};
   
New:
$count_sql = call('tl_activity_stats_sql(text[],int)',
[ statistic= $stat, person_id = $lead-{person_id} ], $debug);

The call() function recognizes the [] in the signature and knows that it
needs to handle the corresponding argument being an array reference.

Calling a function that returns a single record (multiple columns):
Old:
$stat_sql = SELECT * FROM tl_priority_stats($lead-{id}, $debug);
$stat_sth = spi_query($stat_sql);
$stats = spi_fetchrow($stat_sth);
New:
$stats = call('tl_priority_stats(int,int)', $lead-{id}, $debug);

Calling a function that returns multiple rows of a single value:
Old:
my $sql = SELECT * FROM tl_domain_mlx_area_ids($mlx_board_id, $domain_id, 
$debug);
my $sth = spi_query($sql);
while( my $row = spi_fetchrow($sth) ) {
push(@mlx_area_ids, $row-{tl_domain_mlx_area_ids});
}
New:
@mlx_area_ids = call('tl_domain_mlx_area_ids(int,int,int)', $mlx_board_id, 
$domain_id, $debug);

I've appended the POD documentation and attached the (rough but working)
test script.

I plan to release the module to CPAN in the next week or so.

I'd greatly appreciate any feedback.

Tim.


=head1 NAME

PostgreSQL::PLPerl::Call - Simple interface for calling SQL functions from 
PostgreSQL PL/Perl

=head1 SYNOPSIS

use PostgreSQL::PLPerl::Call qw(call);

Returning single-row single-column values:

$pi = call('pi()'); # 3.14159265358979

$net = call('network(inet)', '192.168.1.5/24'); # '192.168.1.0/24';

$seqn = call('nextval(regclass)', $sequence_name);

$dims = call('array_dims(text[])', '{a,b,c}');   # '[1:3]'

# array arguments can be perl array references:
$ary = call('array_cat(int[], int[])', [1,2,3], [2,1]); # '{1,2,3,2,1}'

Returning multi-row single-column values:

@ary = call('generate_series(int,int)', 10, 15); # (10,11,12,13,14,15)

Returning single-row multi-column values:

# assuming create function func(int) returns table (r1 text, r2 int) ...
$row = call('func(int)', 42); # returns hash ref { r1=..., r2=... }

Returning multi-row multi-column values:

@rows = call('pg_get_keywords()'); # ({...}, {...}, ...)

=head1 DESCRIPTION

The Ccall function provides a simple effcicient way to call SQL functions
from PostgreSQL PL/Perl code.

The first parameter is a Isignature that specifies the name of the function
to call and then, in parenthesis, the types of any arguments as a comma
separated list. For example:

'pi()'
'generate_series(int,int)'
'array_cat(int[], int[])'

The types specify how the Iarguments to the call should be interpreted.
They don't have to exactly match the types used to declare the function you're
calling.

Any further parameters are used as arguments to the function being called.

=head2 Array Arguments

The argument value corresponding to a type that contains 'C[]' can be a
string formated as an array literal, or a reference to a perl array. In the
later case the array reference is automatically converted into an array literal
using the Cencode_array_literal() function.

=head2 Varadic Functions

Functions with Cvaradic arguments can be called with a fixed number of
arguments by repeating the type name in the signature the same number of times.
For example, given:

create function vary(VARADIC int[]) as ...

you can call that function with three arguments using:

call('vary(int,int,int)', $int1, $int2, $int3);

Alternatively, you can append the string 'C...' to the last type in the
signature to indicate that the argument is varadic. For example:

call('vary(int...)', @ints);

=head2 Results

The Ccall() function processes return values in one of four ways depending on
two criteria: single column vs. multi-column results, and list context vs 
scalar context.

If the results contain a single column with the same name as the function that
was called, then those values are extracted returned directly. This makes
simple calls very simple:

@ary = call('generate_series(int,int)', 10, 15); # (10,11,12,13,14,15)

Otherwise, the rows are returned as references to hashes:

@rows = call('pg_get_keywords()'); # ({...}, {...}, ...)

If the Ccall() function was executed in list context then all the values/rows
are returned, as shown above.

If the function was executed in scalar context then an exception will be thrown
if more than one row is returned. For example:

$foo = 

[HACKERS] Re: PostgreSQL::PLPerl::Call - Simple interface for calling SQL functions from PostgreSQL PL/Perl

2010-02-12 Thread Tim Bunce
On Fri, Feb 12, 2010 at 11:10:15PM +, Tim Bunce wrote:
 I've appended the POD documentation and attached the (rough but working)
 test script.

Oops. Here's the test script.

Tim.

create or replace function test_call() returns text language plperlu as $func$

use lib /Users/timbo/pg/PostgreSQL-PLPerl-Call/lib;
use PostgreSQL::PLPerl::Call;

use Test::More 'no_plan';

my $row;
my @ary;

# == single-value single-row function ==

# --- no arguments
like call('pi()'), qr/^3.14159/;

# bad calls
eval { call('pi()', 42) };
like $@, qr/expected 0 argument/;
eval { call('pi', 42) };
like $@, qr/Can't parse 'pi'/; # error from call() itself

# --- one argument, simple types
is call('abs(int)', -42), 42;
is call('abs(float)', -42.5), '42.5';
is call('bit_length(text)', 'jose'), 32;

# --- one argument, multi-word types
is call('abs(double precision)', -42.5), '42.5';
is call('bit_length(character varying(90))', 'jose'), 32;

# --- lock calls
call('pg_try_advisory_lock_shared(bigint)', 1234);
call('pg_advisory_unlock_all()');

# bad calls
eval { call('abs(int)', -42.5) };
like $@, qr/invalid input syntax for integer/;
eval { call('abs(text)', -42.5) };
like $@, qr/function abs\(text\) does not exist/;
eval { call('abs(nonesuchtype)', -42.5) };
like $@, qr/type nonesuchtype does not exist/;

# --- multi-argument, simple types
is call('trunc(numeric,int)', 42.4382, 2), '42.43';

# --- unusual types from strings
is call('host(inet)','192.168.1.5/24'), '192.168.1.5';
is call('network(inet)', '192.168.1.5/24'), '192.168.1.0/24';
is call('abbrev(cidr)',  '10.1.0.0/16'),'10.1/16';
is call('numnode(tsquery)', '(fat  rat) | cat'), 5;

spi_exec_query('create temp sequence seqn1 start with 42');
is call('nextval(regclass)', 'seqn1'), 42;
is call('nextval(text)', 'seqn1'), 43;

is call('string_to_array(text, text)', 'xx~^~yy~^~zz', '~^~'), '{xx,yy,zz}';

# --- array and array reference handling
is call('array_dims(text[])', '{a,b,c}'), '[1:3]';
is call('array_dims(text[])', [qw(a b c)]), '[1:3]';
is call('array_dims(text[])', [[1,2,3], [4,5,6]]), '[1:2][1:3]';
is call('array_cat(int[], int[])', [1,2,3], [2,1]), '{1,2,3,2,1}';


# == single-value multi-row function ==

@ary = call('unnest(int[])', '{11,12,13}');
is scalar @ary, 3;
is_deeply \...@ary, [ 11, 12, 13 ];

@ary = call('generate_series(int,int)', 10, 19);
is scalar @ary, 10;
is_deeply \...@ary, [ 10..19 ];

@ary = call('generate_series(int,int,int)', 10, 19, 4);
is_deeply \...@ary, [ 10, 14, 18 ];

@ary = call('generate_series(timestamp,timestamp,interval)', '2008-03-01', 
'2008-03-02', '12 hours');
is_deeply \...@ary, [ '2008-03-01 00:00:00', '2008-03-01 12:00:00', '2008-03-02 
00:00:00' ];

# bad calls
eval { scalar call('generate_series(int,int)', 10, 19) };
like $@, qr/returned more than one row/;

# == multi-value (record) returning functions ==

@ary = call('pg_get_keywords()');
cmp_ok scalar @ary, '', 200;
ok $row = $ary[0];
is ref $row, 'HASH';
ok exists $row-{word},'should contain a word column';
ok exists $row-{catcode}, 'should contain a catcode column';
ok exists $row-{catdesc}, 'should contain a catdesc column';

# single-record
spi_exec_query(q{
create or replace function f1(out r1 text, out r2 int) language plperl 
as $$
return { r1=10, r2=11 };
$$
});
@ary = call('f1()');
is scalar @ary, 1;
ok $row = $ary[0];
is $row-{r1}, 10;
is $row-{r2}, 11;
spi_exec_query('drop function f1()');

# multi-record
spi_exec_query(q{
create or replace function f2() returns table (r1 text, r2 int) 
language plperl as $$
return_next { r1 = $_, r2 = $_+1 } for 1..5;
return undef;
$$
});
@ary = call('f2()');
is scalar @ary, 5;
is $ary[-1]-{r1}, 5;
is $ary[-1]-{r2}, 6;
spi_exec_query('drop function f2()');

# == functions with defaults or varargs ==

spi_exec_query(q{
create or replace function f3(int default 42) returns int language 
plperl as $$
return shift() + 1;
$$
});
is call('f3()'), 43;
spi_exec_query('drop function f3(int)');

spi_exec_query(q{
create or replace function f4(VARIADIC numeric[]) returns float 
language plperlu as $$
use PostgreSQL::PLPerl::Call;
my $sum;
$sum += $_ for call('unnest(numeric[])', $_[0]);
return $sum;
$$
});
# call varadic with explicit number of args in the signature
is call('f4(numeric, numeric)',  10,11   ), 21;
is call('f4(numeric, numeric, numeric)', 10,11,12), 33;

# call varadic using '...' in the signature
is call('f4(numeric, numeric ...)', 10,11,12), 33;
is call('f4(numeric ...)',  10,11,12), 33;
is call('f4(numeric ...)',  10,11   ), 21;
is call('f4(numeric ...)',  10  ), 10;
# XXX doesn't work with no args - possibly natural consequence
#is call('f4(numeric ...)'   ), undef;
spi_exec_query('drop function f4(varadic 

Re: [HACKERS] log_error_verbosity function display

2010-02-12 Thread Bruce Momjian
Robert Haas wrote:
 On Thu, Feb 11, 2010 at 5:47 PM, Bruce Momjian br...@momjian.us wrote:
  Tom Lane wrote:
  Bruce Momjian br...@momjian.us writes:
   Jaime Casanova wrote:
   i like this with or without the (), but maybe we are breaking client
   apps if change that
 
   Ah, so you like FUNCTION.
 
  You can NOT change the line tag without almost certainly breaking
  log-reading tools like pgfouine. ?Even changing the content of the line
  risks that, and for no visible gain.
 
  This seems like the worst form of bike-shedding to me. ?This log entry
  has been formatted this way since 7.4, and nobody has ever complained
  about it, until you suddenly decided it was a problem. ?Leave it be.
 
  I propose to add '()' because it is confusing without it. ?I don't think
  many people are using the feature or we would have received suggestions
  for improvmement. ?As you can see, once I posted about it, there were a
  number of people who wanted improvements.
 
 I'm not sure if people affirmatively wanted improvements or if people
 were just discussing how to change it if a change was to be made.   I
 don't think you can infer that lack of suggestions for improvement
 implies that no one is using it; it could equally well imply that
 everyone likes it the way it is.  To be sure, I probably would have
 coded it a bit differently if I'd written the functionality
 originally, but I don't think it's horrible the way it is, and Tom is
 right that there is something to be said for consistency.

I have seen no other replies to this so I will not make any changes to
the output format.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]

2010-02-12 Thread Tim Bunce
On Fri, Feb 12, 2010 at 02:30:37PM -0700, Alex Hunsaker wrote:
 On Fri, Feb 12, 2010 at 13:42, Andrew Dunstan and...@dunslane.net wrote:
  Alex Hunsaker wrote:
 
  and leave the rest for the next release, unless you can
  convince me real fast that we're not opening a back door here. If we're
  going to allow DBAs to add things to the Safe container, it's going to be up
  front or not at all, as far as I'm concerned.
 
 I think backdoor is a  bit extreme.   Yes it could allow people who
 can set the plperl.*_init functions to muck with the safe.  As an
 admin I could also do that by setting plperl.on_init and overloading
 subs in the Safe namespace or switching out Safe.pm.

Exactly.

[As I mentioned before, the docs for Devel::NYTProf::PgPLPerl
http://code.google.com/p/perl-devel-nytprof/source/browse/trunk/lib/Devel/NYTProf/PgPLPerl.pm
talk about the need to _hack_ perl standard library modules
in order to be able to run NYTProf on plperl code.  The PERL5OPT
environment variable could be used as an alternative vector.]

Is there any kind of threat model (http://en.wikipedia.org/wiki/Threat_model)
that PostgreSQL developers use when making design decisions?

Clearly the PostgreSQL developers take security very seriously, and
rightly so. An explicit threat model document would provide a solid
framework to assess potential security issues when their raised.

The key issue here seems to be the trust, or lack of it, placed in DBAs.


 Anyway reasons I can come up for it are:
 -its general so we can add other modules/pragmas easily in the future
 -helps with the plperl/plperlu all or nothing situation
 -starts to flesh out how an actual exposed (read documented) interface
 should look for 9.1
 
 ... I know Tim mentioned David as having some use cases (cc'ed)

I've just posted the docs for a module that's a good example of the
kind of module a DBA might want to allow plperl code to use
http://archives.postgresql.org/pgsql-hackers/2010-02/msg01059.php

I'd be happy to add a large scary warning in the plc_safe_ok.pl
file saying that any manipulation of the (undocumented) variables
could cause a security risk and is not supported in any way.

Tim.

 So my $0.2 is I don't have any strong feelings for/against it.  I
 think if we could expose *something* (even if you can only get to it
 in a C contrib module) that would be great.  But I realize it might be
 impractical at this stage in the game.
 
 *shrug*
 
 -- 
 Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
 To make changes to your subscription:
 http://www.postgresql.org/mailpref/pgsql-hackers
 

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


Re: [HACKERS] knngist patch support

2010-02-12 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 Tom remarked in another email that he wasn't too happy with the
 opclass changes.  They seem kind of grotty to me, too, but I don't
 immediately have a better idea.  My fear is that there may be places
 in the code that rely on opclass operators only ever returning bool,
 and that changing that may break things.  It also feels like allowing
 non-bool-returning opclass members only for this one specific case is
 kind of a hack: is this an instance of some more general problem that
 we ought to be solving in some more general way?  Not sure.

Yes, that's exactly what I didn't like about it: the proposed changes
create confusion between opclass members that represent
index-optimizable WHERE conditions and those that represent
index-optimizable ORDER BY conditions.  You can get away with that to
some extent as long as you assume that the latter type of operator
never yields boolean and so can never appear at the top level of
WHERE.  But that assumption sucks.  There are plenty of cases where
people ORDER BY boolean values, so who's to argue that we will never
want an operator returning boolean in the second category?  And as
soon as you put it in, the planner is going to think that it's also
a potential index-qualification operator, which is something the AM
might or might not be prepared to support.

I think this is really unacceptable and there needs to be some cleaner
way of distinguishing the two types of operators.  Possibly a couple of
boolean columns added to pg_amop (you'd need two because there are three
possible states, in case an operator really can serve both purposes in a
particular opclass).  Or maybe we should do something else.  But
ignoring the issue won't do.

Maybe a more general idea would be to invent categories of opclass
members, where the only existing category is index search qualifier,
and these new knngist thingies are another, and maybe plus and minus for
window function ranges are a third.  But I'm not sure what you do if one
operator can be in more than one category.

regards, tom lane

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


Re: [HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]

2010-02-12 Thread Andrew Dunstan



Alex Hunsaker wrote:

  Yes it could allow people who
can set the plperl.*_init functions to muck with the safe.  As an
admin I could also do that by setting plperl.on_init and overloading
subs in the Safe namespace or switching out Safe.pm.
  


It's quite easy to subvert Safe.pm today, sadly. You don't need on*init, 
nor do you need to replace the System's Safe.pm. I'm not going to tell 
people how here, but it took me all of a few minutes to discover and 
verify when I went looking a few weeks ago, once I thought about it.


But that's quite different from us providing an undocumented way to 
expose arbitrary objects to the Safe container. In that case *we* become 
responsible for any insecure uses, and we don't even have the luxury of 
having put large warnings in the docs, because there aren't any docs.


Another objection is that discussion on this facility has been pretty 
scant. I think that's putting it mildly. Maybe it's been apparent to a 
few people what the implications are, but I doubt it's been apparent to 
many. That makes me quite nervous, which is why I'm raising it now.


Tim mentioned in his reply that he'd be happy to put warnings in 
plc_safe_ok.pl. But that file only exists in the source - it's turned 
into header file data as part of the build process, so warnings there 
will be seen by roughly nobody.


I still think if we do this at all it needs to be documented and 
surrounded with appropriate warnings.


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] Writeable CTEs and empty relations

2010-02-12 Thread Tom Lane
Marko Tiikkaja marko.tiikk...@cs.helsinki.fi writes:
 Here's the patch.  It's the same as the stuff in writeable CTE patches,
 but I added regression tests.

 Whoops.  The reference section in docs still had some traces of writeable
 CTEs.  Updated patch attached.

I spent some time playing with this but concluded that it's not
committable.  I ran into two significant problems:

1. In an INSERT statement, it's already possible to attach a WITH to
the contained statement, ie
INSERT INTO foo WITH ... SELECT ...
INSERT INTO foo WITH ... VALUES (...)
and the patch wasn't doing anything nice with the case where one tries
to put WITH at both places:
WITH ... INSERT INTO foo WITH ... VALUES (...)
(The SELECT case actually works, mostly, but the VALUES one doesn't.)
I thought about just concat'ing the two WITH lists but this introduces
various strange corner cases; in particular when one list is marked
RECURSIVE and the other isn't there's no way to avoid surprising
behavior.  However, since the option for an inner WITH already does
everything you would want, we could just forget about adding outer WITH
for INSERT.  The attached modified patch does that.

2. None of the cases play nicely with NEW or OLD references in a rule.
For example,

regression=#  create temp table x(f1 int);
CREATE TABLE
regression=#  create temp table y(f2 int);
CREATE TABLE
regression=# create rule r2 as on update to x do instead
with t as (select old.*) update y set f2 = t.f1 from t;
CREATE RULE
regression=# update x set f1 = f1+1;
ERROR:  bogus local parameter passed to WITH query
regression=#

I don't see any very nice way to fix this.  The problem is that the
NEW or OLD reference is treated as though it were a relation of the
main query (the UPDATE in this case), which is something that's not
valid to reference in a WITH query.  I'm afraid that it might not
be possible to fix it without significant changes in the way rules
work (and consequent compatibility issues).

We could possibly put in some hack to disallow OLD/NEW references in
the WITH queries, but that got past my threshold of ugliness, so
I'm not going to commit it without further discussion.

Attached is the patch as I had it before giving up (sans documentation
since I'd not gotten to that yet).  The main other change from what
you submitted was adding ruleutils.c support.

regards, tom lane

Index: src/backend/nodes/copyfuncs.c
===
RCS file: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v
retrieving revision 1.461
diff -c -r1.461 copyfuncs.c
*** src/backend/nodes/copyfuncs.c	12 Feb 2010 17:33:20 -	1.461
--- src/backend/nodes/copyfuncs.c	13 Feb 2010 00:49:41 -
***
*** 2279,2284 
--- 2279,2285 
  	COPY_NODE_FIELD(usingClause);
  	COPY_NODE_FIELD(whereClause);
  	COPY_NODE_FIELD(returningList);
+ 	COPY_NODE_FIELD(withClause);
  
  	return newnode;
  }
***
*** 2293,2298 
--- 2294,2300 
  	COPY_NODE_FIELD(whereClause);
  	COPY_NODE_FIELD(fromClause);
  	COPY_NODE_FIELD(returningList);
+ 	COPY_NODE_FIELD(withClause);
  
  	return newnode;
  }
Index: src/backend/nodes/equalfuncs.c
===
RCS file: /cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v
retrieving revision 1.382
diff -c -r1.382 equalfuncs.c
*** src/backend/nodes/equalfuncs.c	12 Feb 2010 17:33:20 -	1.382
--- src/backend/nodes/equalfuncs.c	13 Feb 2010 00:49:41 -
***
*** 899,904 
--- 899,905 
  	COMPARE_NODE_FIELD(usingClause);
  	COMPARE_NODE_FIELD(whereClause);
  	COMPARE_NODE_FIELD(returningList);
+ 	COMPARE_NODE_FIELD(withClause);
  
  	return true;
  }
***
*** 911,916 
--- 912,918 
  	COMPARE_NODE_FIELD(whereClause);
  	COMPARE_NODE_FIELD(fromClause);
  	COMPARE_NODE_FIELD(returningList);
+ 	COMPARE_NODE_FIELD(withClause);
  
  	return true;
  }
Index: src/backend/parser/analyze.c
===
RCS file: /cvsroot/pgsql/src/backend/parser/analyze.c,v
retrieving revision 1.401
diff -c -r1.401 analyze.c
*** src/backend/parser/analyze.c	12 Feb 2010 22:48:56 -	1.401
--- src/backend/parser/analyze.c	13 Feb 2010 00:49:41 -
***
*** 282,287 
--- 282,294 
  
  	qry-commandType = CMD_DELETE;
  
+ 	/* process the WITH clause independently of all else */
+ 	if (stmt-withClause)
+ 	{
+ 		qry-hasRecursive = stmt-withClause-recursive;
+ 		qry-cteList = transformWithClause(pstate, stmt-withClause);
+ 	}
+ 
  	/* set up range table with just the result rel */
  	qry-resultRelation = setTargetTable(pstate, stmt-relation,
    interpretInhOption(stmt-relation-inhOpt),
***
*** 380,386 
  	 * Must get write lock on INSERT target table before scanning SELECT, else
  	 * we will grab the wrong kind of initial lock if the target table is also
  	 * mentioned in the SELECT part. 

Re: [HACKERS] Confusion over Python drivers

2010-02-12 Thread Andrew McNamara
Andrew McNamara andr...@object-craft.com.au writes:
 The solution is to write the query in an unambiguous way:
 SELECT $1::date + 1;

 You are missing the point: this is not about what types the SQL execution
 sees. It is about making sure the correct recv function is applied to
 the binary parameter data.

Indeed, and the above locution does set that.

Sure, but it requires the driver to modify the query - that isn't
reasonable or practical.  Expecting the user to the driver to know
and correct set the type the driver will ultimately see is a recipe
for disaster.

-- 
Andrew McNamara, Senior Developer, Object Craft
http://www.object-craft.com.au/

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


Re: [HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]

2010-02-12 Thread Alex Hunsaker
On Fri, Feb 12, 2010 at 17:57, Andrew Dunstan and...@dunslane.net wrote:
 Another objection is that discussion on this facility has been pretty scant.
 I think that's putting it mildly.

Well I can certainly attest to that seeing as how I spotted it purely
by review...

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


Re: [HACKERS] knngist patch support

2010-02-12 Thread Robert Haas
On Fri, Feb 12, 2010 at 7:30 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Maybe a more general idea would be to invent categories of opclass
 members, where the only existing category is index search qualifier,
 and these new knngist thingies are another, and maybe plus and minus for
 window function ranges are a third.  But I'm not sure what you do if one
 operator can be in more than one category.

Well, if you were willing to change pg_amop so that the key was
(amopfamily, amoplefttype, amoprighttype, amopcategory) rather than
just (amopfamily, amoplefttype, amoprighttype), the issue of what to
do if an operator can be in more than one category becomes moot.  You
just specify the operator more than once if need be.

If you don't want the amopcategory to be part of the key, then you
just need to define it as a type that can handle multiple values yet
has a fast membership test.  A character string of some type would be
flexible - you could use any single character as a category identifier
- but given that we don't expect many categories and we do want good
performance, it seems like an int4 used as a bitmap field would be
more appropriate.

I think the first approach is better, partly because it seems to lend
itself to a cleaner syntax for CREATE OPERATOR CLASS.  Something like:

CREATE OPERATOR CLASS blah blah AS
   OPERATOR 3 ,
   OPERATOR ORDER 15 -;

...Robert

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


Re: [HACKERS] Writeable CTEs and empty relations

2010-02-12 Thread Robert Haas
On Fri, Feb 12, 2010 at 8:06 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Marko Tiikkaja marko.tiikk...@cs.helsinki.fi writes:
 Here's the patch.  It's the same as the stuff in writeable CTE patches,
 but I added regression tests.

 Whoops.  The reference section in docs still had some traces of writeable
 CTEs.  Updated patch attached.

 I spent some time playing with this but concluded that it's not
 committable.  I ran into two significant problems:

 1. In an INSERT statement, it's already possible to attach a WITH to
 the contained statement, ie
        INSERT INTO foo WITH ... SELECT ...
        INSERT INTO foo WITH ... VALUES (...)
 and the patch wasn't doing anything nice with the case where one tries
 to put WITH at both places:
        WITH ... INSERT INTO foo WITH ... VALUES (...)
 (The SELECT case actually works, mostly, but the VALUES one doesn't.)
 I thought about just concat'ing the two WITH lists but this introduces
 various strange corner cases; in particular when one list is marked
 RECURSIVE and the other isn't there's no way to avoid surprising
 behavior.  However, since the option for an inner WITH already does
 everything you would want, we could just forget about adding outer WITH
 for INSERT.  The attached modified patch does that.

That seems reasonable, though we might want to document that somehow
for posterity.

By the way, are we going to support WITH ... TABLE?

 2. None of the cases play nicely with NEW or OLD references in a rule.
 For example,

 regression=#  create temp table x(f1 int);
 CREATE TABLE
 regression=#  create temp table y(f2 int);
 CREATE TABLE
 regression=# create rule r2 as on update to x do instead
 with t as (select old.*) update y set f2 = t.f1 from t;
 CREATE RULE
 regression=# update x set f1 = f1+1;
 ERROR:  bogus local parameter passed to WITH query
 regression=#

 I don't see any very nice way to fix this.  The problem is that the
 NEW or OLD reference is treated as though it were a relation of the
 main query (the UPDATE in this case), which is something that's not
 valid to reference in a WITH query.  I'm afraid that it might not
 be possible to fix it without significant changes in the way rules
 work (and consequent compatibility issues).

 We could possibly put in some hack to disallow OLD/NEW references in
 the WITH queries, but that got past my threshold of ugliness, so
 I'm not going to commit it without further discussion.

On the face of it it's not obvious to me why you wouldn't just do
that.  If it's not valid to reference them there, then just don't let
it happen (now comes the part where you tell me why it's not that
simple).

...Robert

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


Re: [HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]

2010-02-12 Thread Robert Haas
On Fri, Feb 12, 2010 at 3:42 PM, Andrew Dunstan and...@dunslane.net wrote:
 I'm a bit inclined to commit the piece of this patch that concerns the
 warnings pragma, and leave the rest for the next release,

Based on the subsequent discussion on this thread, +1 for this
approach.  Allowing use warnings sounds good; opening a potential
security hole sounds bad.  Sounds like it would considerably simplify
the patch, too.

...Robert

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


Re: [HACKERS] knngist patch support

2010-02-12 Thread Robert Haas
On Fri, Feb 12, 2010 at 9:10 PM, Robert Haas robertmh...@gmail.com wrote:
 On Fri, Feb 12, 2010 at 7:30 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Maybe a more general idea would be to invent categories of opclass
 members, where the only existing category is index search qualifier,
 and these new knngist thingies are another, and maybe plus and minus for
 window function ranges are a third.  But I'm not sure what you do if one
 operator can be in more than one category.

 Well, if you were willing to change pg_amop so that the key was
 (amopfamily, amoplefttype, amoprighttype, amopcategory) rather than
 just (amopfamily, amoplefttype, amoprighttype), the issue of what to
 do if an operator can be in more than one category becomes moot.  You
 just specify the operator more than once if need be.

Except I'm full of it, because amopstrategy is in there too.  Hmm.
And that's unfortunate because the syscache machinery is limited to
four columns as lookup keys.

This is a bit ugly, but one idea that occurs to me is to change
amopstrategy from int16 to int32.  Internally, we'll treat the low 16
bits as the strategy number and the high 16 bits as the strategy
category, with strategy category 0 being index search qualifier.

...Robert

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


Re: [HACKERS] Writeable CTEs and empty relations

2010-02-12 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Fri, Feb 12, 2010 at 8:06 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 We could possibly put in some hack to disallow OLD/NEW references in
 the WITH queries, but that got past my threshold of ugliness, so
 I'm not going to commit it without further discussion.

 On the face of it it's not obvious to me why you wouldn't just do
 that.  If it's not valid to reference them there, then just don't let
 it happen (now comes the part where you tell me why it's not that
 simple).

Well, there's no obvious-to-the-user reason why it shouldn't work.
If we hack it, then an example like I gave will give a no such
table error, and I'll bet long odds we'll get bug reports about it.

(Now maybe we could suppress the bug reports if we could get it to
print something more like NEW can't be referenced in WITH, but
doing that seems significantly harder --- the way that I can see
to do it would be to not have NEW/OLD in the namespace while parsing
WITH, and that would lead to a pretty stupid error message.)

regards, tom lane

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


Re: [HACKERS] Patch: Remove gcc dependency in definition of inline functions

2010-02-12 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Wed, Feb 10, 2010 at 5:53 PM, Kurt Harriman harri...@acm.org wrote:
 Revised patch is attached (4th edition).

 This looks generally sane to me, though it seems entirely imaginable
 that it might break something, somewhere, for somebody... in which
 case I suppose we'll adjust as needed.

Looks sane to me too -- committed.  We'll soon see what the buildfarm
thinks (though the number of non-gcc buildfarm members is depressingly
small).

regards, tom lane

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


Re: [HACKERS] Patch: Remove gcc dependency in definition of inline functions

2010-02-12 Thread Robert Haas
On Fri, Feb 12, 2010 at 9:35 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Wed, Feb 10, 2010 at 5:53 PM, Kurt Harriman harri...@acm.org wrote:
 Revised patch is attached (4th edition).

 This looks generally sane to me, though it seems entirely imaginable
 that it might break something, somewhere, for somebody... in which
 case I suppose we'll adjust as needed.

 Looks sane to me too -- committed.  We'll soon see what the buildfarm
 thinks (though the number of non-gcc buildfarm members is depressingly
 small).

I can't feel bad about the near-ubiquity of gcc...  life would be a
lot harder if it were otherwise.

...Robert

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


Re: [HACKERS] knngist patch support

2010-02-12 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 Well, if you were willing to change pg_amop so that the key was
 (amopfamily, amoplefttype, amoprighttype, amopcategory) rather than
 just (amopfamily, amoplefttype, amoprighttype), the issue of what to
 do if an operator can be in more than one category becomes moot.  You
 just specify the operator more than once if need be.

Yeah, that occurred to me too after sending my earlier email.

 Except I'm full of it, because amopstrategy is in there too.  Hmm.
 And that's unfortunate because the syscache machinery is limited to
 four columns as lookup keys.

Ugh.  Still, we could certainly change the 4-key limit to 5, though it
might be a tad tedious to go round and edit all the SearchSysCache and
related calls.  Maybe while we were at it we should change them to
SearchSysCache1, SearchSysCache2, etc to not have the limit hardwired
textually in quite so many places...

 This is a bit ugly, but one idea that occurs to me is to change
 amopstrategy from int16 to int32.  Internally, we'll treat the low 16
 bits as the strategy number and the high 16 bits as the strategy
 category, with strategy category 0 being index search qualifier.

Hm, yeah that would work, but I agree it's ugly.

While thinking about different possible solutions here: one of the
things that was worrying me is that for cases where the same operator
can serve in more than one role, it might have to have either the same
opstrategy or different ones in different roles, depending on how the AM
has assigned strategy numbers.  The method with an extra index column
side-steps that nicely since there are two unrelated pg_amop entries.
If there's only one entry then you lose if you need different
strategies.  Robert's use-the-high-bits method works too, since there
would still be two separate entries, but some other possible
representations are eliminated by that worry.

regards, tom lane

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


Re: [HACKERS] knngist patch support

2010-02-12 Thread Robert Haas
On Fri, Feb 12, 2010 at 9:45 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 Well, if you were willing to change pg_amop so that the key was
 (amopfamily, amoplefttype, amoprighttype, amopcategory) rather than
 just (amopfamily, amoplefttype, amoprighttype), the issue of what to
 do if an operator can be in more than one category becomes moot.  You
 just specify the operator more than once if need be.

 Yeah, that occurred to me too after sending my earlier email.

 Except I'm full of it, because amopstrategy is in there too.  Hmm.
 And that's unfortunate because the syscache machinery is limited to
 four columns as lookup keys.

 Ugh.  Still, we could certainly change the 4-key limit to 5, though it
 might be a tad tedious to go round and edit all the SearchSysCache and
 related calls.  Maybe while we were at it we should change them to
 SearchSysCache1, SearchSysCache2, etc to not have the limit hardwired
 textually in quite so many places...

Maybe.  It sounds sort of awful though; and there's probably a
distributed performance penalty involved

 This is a bit ugly, but one idea that occurs to me is to change
 amopstrategy from int16 to int32.  Internally, we'll treat the low 16
 bits as the strategy number and the high 16 bits as the strategy
 category, with strategy category 0 being index search qualifier.

 Hm, yeah that would work, but I agree it's ugly.

On further review there's a serious problem with this idea:
pg_amop_opr_fam_index.

 While thinking about different possible solutions here: one of the
 things that was worrying me is that for cases where the same operator
 can serve in more than one role, it might have to have either the same
 opstrategy or different ones in different roles, depending on how the AM
 has assigned strategy numbers.  The method with an extra index column
 side-steps that nicely since there are two unrelated pg_amop entries.
 If there's only one entry then you lose if you need different
 strategies.  Robert's use-the-high-bits method works too, since there
 would still be two separate entries, but some other possible
 representations are eliminated by that worry.

OK, here's another idea.  Let's just add a new column to pg_amop
called amoporderstrategy.  If an operator can only be used for one
purpose or the other, we'll set the other value to -1.

...Robert

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


Re: [HACKERS] knngist patch support

2010-02-12 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 OK, here's another idea.  Let's just add a new column to pg_amop
 called amoporderstrategy.  If an operator can only be used for one
 purpose or the other, we'll set the other value to -1.

... problem for unique index, no?

regards, tom lane

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


Re: [HACKERS] knngist patch support

2010-02-12 Thread Robert Haas
On Fri, Feb 12, 2010 at 10:10 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 OK, here's another idea.  Let's just add a new column to pg_amop
 called amoporderstrategy.  If an operator can only be used for one
 purpose or the other, we'll set the other value to -1.

 ... problem for unique index, no?

Dang.  What a pain in the tail.  I guess we could make that column
nullable, but that's got it's own fair share of problems.

Is the only reasonable way to solve this problem a new catalog?
That's not tremendously scalable, but it's starting to feel like the
only way of solving this problem that doesn't involve massive surgery.

...Robert

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


Re: [HACKERS] knngist patch support

2010-02-12 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Fri, Feb 12, 2010 at 9:45 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 This is a bit ugly, but one idea that occurs to me is to change
 amopstrategy from int16 to int32.  Internally, we'll treat the low 16
 bits as the strategy number and the high 16 bits as the strategy
 category, with strategy category 0 being index search qualifier.
 
 Hm, yeah that would work, but I agree it's ugly.

 On further review there's a serious problem with this idea:
 pg_amop_opr_fam_index.

I think that's soluble though.  The reason that index exists is to
enforce the rule that an operator can stand in only one relationship
to an opfamily.  In this design the natural rule would be one
relationship per role, ie, the unique key would become
(operator, category, opfamily).

However, that does make it even uglier to have category shoehorned in as
part of a different field.  Back to wanting 5-key syscaches ...

regards, tom lane

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


Re: [HACKERS] knngist patch support

2010-02-12 Thread Robert Haas
On Fri, Feb 12, 2010 at 10:38 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Fri, Feb 12, 2010 at 9:45 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 This is a bit ugly, but one idea that occurs to me is to change
 amopstrategy from int16 to int32.  Internally, we'll treat the low 16
 bits as the strategy number and the high 16 bits as the strategy
 category, with strategy category 0 being index search qualifier.

 Hm, yeah that would work, but I agree it's ugly.

 On further review there's a serious problem with this idea:
 pg_amop_opr_fam_index.

 I think that's soluble though.  The reason that index exists is to
 enforce the rule that an operator can stand in only one relationship
 to an opfamily.  In this design the natural rule would be one
 relationship per role, ie, the unique key would become
 (operator, category, opfamily).

 However, that does make it even uglier to have category shoehorned in as
 part of a different field.  Back to wanting 5-key syscaches ...

Sigh.

...Robert

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


Re: [HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]

2010-02-12 Thread Alex Hunsaker
On Fri, Feb 12, 2010 at 17:57, Andrew Dunstan and...@dunslane.net wrote:
r

 Alex Hunsaker wrote:

  Yes it could allow people who
 can set the plperl.*_init functions to muck with the safe.

 It's quite easy to subvert Safe.pm today, sadly. ...

If anything that sounds like an argument for it =)

 But that's quite different from us providing an undocumented way to expose
 arbitrary objects to the Safe container. In that case *we* become
 responsible for any insecure uses, and we don't even have the luxury of
 having put large warnings in the docs, because there aren't any docs.

Hrm... Not sure I agree with this point.  If you are saying there is
some way to subvert safe by using these new vars thats not a bug (or
feature) of upstream safe.  Then I agree.  But if what you are saying
is as a (super)user I muck with these (internal) vars in my on_init
function and things become insecure.  Then I disagree, its just a less
ugly (uniform and perhaps more secure?) way of doing the below:

NYTProf/ PgPLPerl.pm
# hack to make DB::finish_profile available within PL/Perl
use Safe;
my $orig_share_from = \Safe::share_from;
*Safe::share_from = sub {
my $obj = shift;
$obj-$orig_share_from('DB', [ 'finish_profile' ]);
return $obj-$orig_share_from(@_);
};

 I still think if we do this at all it needs to be documented and surrounded
 with appropriate warnings.

This is really what I think the issue comes down to.  I think the
feeling is if we document it then we have to support it in the future.
 And we dont have a clear proposal, only a need.  The attitude seems
to be, well its an implementation artifact that might disappear in the
future.  Lets use it to help figure out what that future api should
like like.

I agree with Robert.  At this point in the commit feast its not the
time to be discussing things like this (sorry I could not get to it
sooner Tim!) :(  Though If a patch with good documentation does show
up Ill be happy to review it :)

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


Re: [HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]

2010-02-12 Thread Alex Hunsaker
On Fri, Feb 12, 2010 at 19:14, Robert Haas robertmh...@gmail.com wrote:
 Sounds like it would considerably simplify
 the patch, too.

I overstated that.  The way its done now we could just change the
decelerations to 'my' and drop an if block.  Id be in favor of more or
less keeping the internals as implemented in the latest patch the same
just not exposing them.

-- 
Sent 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: More frame options in window functions

2010-02-12 Thread Hitoshi Harada
2010/2/13 Tom Lane t...@sss.pgh.pa.us:
 Hitoshi Harada umi.tan...@gmail.com writes:
 [ more_frame_options patch ]

 Committed after rather extensive revisions.

Thanks a lot.

 I'm not terribly happy with the changes you made in WinGetFuncArgInPartition
 and WinGetFuncArgInFrame to force the window function mark to not go
 past frame start in some modes.  Not only is that pretty ugly, but I
 think it can mask bugs in window functions: it's an error for a window
 function to fetch a row before what it has set its mark to be, but in
 some cases that wouldn't be detected because of this change.  I think
 it would be better to revert those changes and find another method of
 protecting fetches needed to determine the frame head.  One idea is
 to create a separate read pointer that tracks the frame head whenever
 actual fetches of the frame head might be needed by update_frameheadpos.
 I committed it without changing that, but I think this should be
 revisited before trying to add the RANGE value PRECEDING/FOLLOWING
 options, because those will substantially expand the number of cases
 where that hack affects the behavior.

Well, you're right. In addition to this topic, I concern a little
about changing row fetching in aggregate from spool_tuples() to
window_gettupleslot(), for performance reason. It's needed to support
extended frame options, but for original basic frame options it may
get slow. Anyway, I agree to revisit and refactor to executor logic.


Regards,

-- 
Hitoshi Harada

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