Re: [HACKERS] Hostnames in pg_hba.conf
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
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
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
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
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
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
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}
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
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
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
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
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
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
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
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]
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
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
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
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]
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]
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
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
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
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.
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
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.
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.
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]
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]
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.
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.
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
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.
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
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
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)
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
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
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
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
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
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
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
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
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
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
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/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
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
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
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
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
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
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
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
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.
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.
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
[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
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
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]
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
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
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
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]
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
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
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
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
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
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}
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
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
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
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
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]
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
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]
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
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
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]
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
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
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]
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
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
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
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
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
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
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
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
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
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
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]
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]
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/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