Re: [HACKERS] NOTIFY with tuples
On Dec 13, 2011, at 8:21 PM, Tom Lane wrote: I'm not sure whether we'd want something like this in core, so for a first go-around, you might want to consider building it as an extension. ... I'm not sure you need NOTIFY for anything anywhere in here. Actually, what I'd suggest is just some code to serialize and deserialize tuples and transmit 'em via the existing NOTIFY payload facility. I agree that presenting it as some functions would be a lot less work The ability to cast RECORDs to JSON would be awesome for this. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: URI connection string support for libpq
On Tue, Dec 13, 2011 at 07:54:14PM -0500, Greg Smith wrote: After this bit of tinkering with the code, it feels to me like this really wants a split() function to break out the two sides of a string across a delimiter, eating it in the process. Adding the level of paranoia I'd like around every bit of code I see that does that type of operation right now would take a while. Refactoring in terms of split and perhaps a few similarly higher-level string parsing operations, targeted for this job, might make it easier to focus on fortifying those library routines instead. For example, instead of the gunk I just added that moves past either type of protocol prefix, I'd like to just say split(buf,://,left,right) and then move on with processing the right side. FWIW, python calls this operation partition, as in: left, delim, right = buf.partition(://) Have a nice day, -- Martijn van Oosterhout klep...@svana.org http://svana.org/kleptog/ He who writes carelessly confesses thereby at the very outset that he does not attach much importance to his own thoughts. -- Arthur Schopenhauer signature.asc Description: Digital signature
Re: [HACKERS] psql output locations
On Mon, Dec 12, 2011 at 21:04, Peter Eisentraut pete...@gmx.net wrote: On mån, 2011-12-12 at 14:47 +0100, Magnus Hagander wrote: We're either pretty inconsistent with our output in psql, or I'm not completely understanding it.. I was trying to implement a switch that would let me put all the output in the query output channel controlled by \o, and not just the output of the query itself. Because that would make it possible to control it from inside the script. Now, this made me notice: * There are 102 calls to psql_error(), 42 direct uses of fprintf(stderr), and 7 uses of fputs(stderr). And there is also write_stderr() used in the cancel handler. Is there actually a point behind all those direct uses, or should they really be psql_error() calls? Some of this could probably be more more uniform. But I don't see how this related to your question. All the output goes uniformly to stderr, which is what matters. I was overriding psql_error() to write it to the same file as the \o output was written too - and that only worked for some of the errors. It seemed like the logical place to put such a redirection... * There are a number of things that are always written to stdout, that there is no way to redirect. In some cases it's interactive prompts - makes sense - but also for example the output of \timing goes to stdout always. Is there some specific logic behind what/when this should be done? Everything that is not an error goes to stdout, no? Except the query output, if you change it. Maybe the way to do what you want is to invent a new setting that temporarily changes stdout. Yeah, that might be it. Or I need separate settings for put errors in the query output stream and put non-query-output-but-also-non-errors in the query output stream. The effect would be the same, I guess... -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] Command Triggers
Robert Haas robertmh...@gmail.com writes: it. Dimitri says that he wants it so that we can add support for CREATE TABLE, ALTER TABLE, and DROP TABLE to Slony, Bucardo, and Londiste. My fear is that it won't turn out to be adequate to that task, because there won't actually be enough information in the CREATE TABLE statement to do the same thing on all servers. In particular, you won't have the index or constraint names, and you might not have the schema or tablespace information either. In my experience of managing lots of trigger based replications (more than 100 nodes in half a dozen different projects), what I can tell from the field is that I don't care about index and constraint names. Being able to replicate the same CREATE TABLE statement that the provider just executed on the subscriber is perfectly fine for my use cases. Again, that's a caveat of the first implementation, you can't have sub commands support without forcing them through ProcessUtility and that's a much more invasive patch. Maybe we will need that later. Also it's quite easy to add support for the CREATE INDEX command, including index name support, and ALTER TABLE is already on the go. So we can document how to organize your DDL scripts for them to just work with the replication system. And you can even implement a command trigger that enforces respecting the limits (RAISE EXCEPTION when the CREATE TABLE command is embedding primary key creation rather than using a separate command for that). As for the schema, you can easily get the current search_path setting from the command trigger and force it to the same value on the subscriber before replaying the commands (hint: add current search_path to the event you're queuing for replay). select setting from pg_settings where name = 'search_path'; I appreciate that some use cases won't be possible to implement with the first version of this patch, really, but I believe we have enough use cases that are possible to implement with it that it's worth providing the feature. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch to allow users to kill their own queries
On Tue, Dec 13, 2011 at 11:59, Greg Smith g...@2ndquadrant.com wrote: The submission from Edward Muller I'm replying to is quite similar to what the other raging discussion here decided was the right level to target. There was one last year from Josh Kupershmidt with similar goals: http://archives.postgresql.org/pgsql-admin/2010-02/msg00052.php A good place to start is the concise summary of the new specification goal that Tom made in the other thread: If allowing same-user cancels is enough to solve 95% or 99% of the real-world use cases, let's just do that. Same-user cancels, but not termination. Only this, and nothing more. Good. Appropriate credits here would go Josh Kupershmidt, Edward Muller, and then myself; everyone did an equally useful chunk of this in that order. It's all packaged up for useful gitsumption at https://github.com/greg2ndQuadrant/postgres/tree/cancel_backend too. I attached it to the next CommitFest: https://commitfest.postgresql.org/action/patch_view?id=722 but would enjoy seeing a stake finally put through its evil heart before then, as I don't think there's much left to do now. test= select pg_terminate_backend(28154); ERROR: must be superuser to terminate other server processes HINT: you can use pg_cancel_backend() on your own processes That HINT sounds a bit weird to me. you can cancel your own queries using pg_cancel_backend() instead or something like that? Victory over the evil sleeping backend is complete, without a superuser in sight. \o/ There's one obvious and questionable design decision I made to highlight. Right now the only consumers of pg_signal_backend are the cancel and terminate calls. What I did was make pg_signal_backend more permissive, adding the idea that role equivalence = allowed, and therefore granting that to anything else that might call it. And then I put a stricter check on termination. This results in a redundant check of superuser on the termination check, and the potential for mis-using pg_signal_backend. I documented all that and liked the result; it feels better to me to have pg_signal_backend provide an API that is more flexible here. Pushback to structure this differently is certainly possible though, and I'm happy to iterate the patch to address that. It might drift back toward something closer to Josh's original design. How about passing a parameter to pg_signal_backend? Making pg_signal_backend(int pid, int sig, bool allow_samerole)? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters
On 2011-12-13 18:34, Tom Lane wrote: Heikki Linnakangasheikki.linnakan...@enterprisedb.com writes: Attached is a patch with those changes. I also I removed a few of the syntax error regression tests, that seemed excessive, plus some general naming and comment fiddling. I'll apply this tomorrow, if it still looks good to me after sleeping on it. However, I'm still concerned about whether this approach gives reasonable error messages in cases where the error would be detected during parse analysis of the rearranged statement. The regression test examples don't cover such cases, and I'm too busy right now to apply the patch and check for myself. What happens for example if a named parameter's value contains a misspelled variable reference, or a type conflict? I tested this and seems to be ok: regression=# select namedparmcursor_test1(2, 2) as Should be false, namedparmcursor_test1(20, 20) as Should be true; ERROR: column yy does not exist LINE 1: SELECT x AS param1, yy AS param12; regression=# select namedparmcursor_test1(2, 2) as Should be false, namedparmcursor_test1(20, 20) as Should be true; ERROR: invalid input syntax for integer: 2011-11-29 19:26:10.029084 CONTEXT: PL/pgSQL function namedparmcursor_test1 line 8 at OPEN regards, Yeb Havinga last error was created with create or replace function namedparmcursor_test1(int, int) returns boolean as $$ declare c1 cursor (param1 int, param12 int) for select * from rc_test where a param1 and b param12; y int := 10; x timestamp := now(); nonsense record; begin open c1(param12 := $1, param1 := x); end $$ language plpgsql; -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Command Triggers
Christopher Browne cbbro...@gmail.com writes: - What I'd much rather have is a form of the query that is replete with Full Qualification, so that create table foo (id serial primary key, data text not null unique default 'better replace this', dns_data dnsrr not null); may be transformed into a safer form like: create table public.foo (id serial primary key, data text not null unique default 'better replace this'::text, dns_data dns_rr.dnsrr not null); Andres did the same comment and I've begun working on that. The facility for fully qualifying object names are not always available in a form that we can use from the parsetree, but that's a SMOP. What's not clear, yet, is which transformations are troublesome. For instance, if there's already a sequence called foo_id_seq, then the sequence defined for that table winds up being foo_id_seq1, and it's not quite guaranteed that *that* would be identical across databases. But perhaps it's sufficient to implement what, of COMMAND TRIGGERS, can be done, and we'll see, as we proceed, whether or not it's enough. I'm not convinced that having the same sequence names on the subscribers is something we need here. Let me detail that, because maybe I just understood a major misunderstanding in the use case I'm interested into. I mostly use trigger based replication in cases where it's not possible to implement failover or switchover with that technique, because I'm doing cross-replication or more complex architectures. Failover is handled with WAL based techniques (wal shipping, streaming rep, etc). So I don't care that much about the sub object names (constraints, indexes, sequences): I need them to get created on the subscriber and that's about it. I think that's an important enough use case here. It's conceivable that a first implementation won't be enough to implement DDL triggers for Slony, and that we'd need to ask for additional functionality that doesn't make it in until 9.3. That seems better, to me, than putting it on the shelf, and having functionality in neither 9.2 nor 9.3... Or maybe Slony would end up relying partly on the command trigger facility and implementing the missing pieces in its own code base. Like it did with the txid_snapshot data type some years ago, for example. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Command Triggers
Jan Wieck janwi...@yahoo.com writes: I agree. While it is one of the most asked for features among the trigger based replication systems, I fear that an incomplete solution will cause more problems than it solves. It is far easier to tell people DDL doesn't propagate automatically, do this instead ... than to try to support a limited list of commands, that may or may not propagate as intended. And all Nothing stops you from checking that the command you want to replicate is indeed supported and refuse to run it on the provider when not, that's what command triggers are for :) sorts of side effects, like search_path, user names and even the existing schema in the replica can cause any given DDL string to actually do something completely different than what happened on the origin. Grab those on the provider from pg_settings and the like in the command trigger and restore them on the subscriber before applying the command? On top of that, the PostgreSQL main project has a built in replication solution that doesn't need any of this. There is no need for anyone, but us trigger replication folks, to keep command triggers in sync with all other features. You can't implement cross replication with built-in replication. I'm yet to work on a medium sized project where I don't need both streaming replication, wal archiving, and a trigger based replication system. I don't think it is going to be reliable enough any time soon to make this the default for any of the trigger based replication systems. You need to add yet and without some work in the client implementation. Last time I read such comments, it was about extensions. We still shipped something in 9.1, and by your measure, it's quite broken. When you implement an SQL only extension (no .so) you still have to use a Makefile and you need to deploy your scripts on the server filesystem before hand, it's not possible to install or update the extension from an SQL connection (even when doing only self-contained SQL commands). Also, the dependency system is not solving much real world problems, apart from the very simplest one that we can play with in contrib/. You can't even list shared objects (.so, .dll) that have been loaded so far in a session and reference the extension they belong to. Yet, I bet that some people will find that the extension system as we have it in 9.1 still is useful enough to have been released in there. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support PS: yes I intend to be working on fixing those extension limitations and caveats with a series of patches. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dump --exclude-table-data
Not sure that I have a lot to add here, but I am officially listed as a reviewer, which is a responsibility that I don't want to shirk. In my opinion, this patch is obviously useful. I don't find the asymmetry that it will create with pg_restore to be troubling, so I'd favour committing it as-is. Extremely minor problem noted: There are two spaces at the start of one sentence in your SGML doc updates. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] COUNT(*) and index-only scans
On Mon, Nov 21, 2011 at 6:43 PM, Robert Haas robertmh...@gmail.com wrote: In buffer fill mode, we scan the index and add matching tuples and their CTIDs to the buffer. When the buffer is full or the index AM reports that there are no more tuples in the scan, we switch to buffer drain mode. Instead you could do the two phases concurrently the way tuplesort implements the tapesort from Knuth. You keep a heap of ctids with an epoch. You fill the heap then you return the first one. Whenever you return one you read the next one and add it to the heap. If it falls before the last returned value you insert it with the next epoch but if it falls afterwards you can insert it into the heap in its correct position. -- greg -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Race condition in HEAD, possibly due to PGPROC splitup
On Wed, Dec 14, 2011 at 4:15 AM, Tom Lane t...@sss.pgh.pa.us wrote: Without the added assert, you'd only notice this if you were running a standby slave --- the zero xid results in an assert failure in WAL replay on the slave end, which is how I found out about this to start with. But since we've not heard reports of such before, I suspect that this is a recently introduced bug; and personally I'd bet money that it was the PGXACT patch that broke it. I can reproduce this and will look at it in detail. BTW, on an unrelated note, I was looking at the way ShmemInitStruct() is used. It internally uses ShmemAlloc to allocate from shared memory. ShmemAlloc always MAXALIGN the requested size. But while calculating the total shared memory requirement, we don't always MAXALIGN individual structure sizes. One example is KnownAssignedXidsValid, but I am sure there are other places where the originally computed and the requested sizes could be different because of alignment. I wonder if we are just lucky not to hit shared memory size mismatch, may be because we round up to the block size at the end. Thanks, Pavan -- Pavan Deolasee 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] Race condition in HEAD, possibly due to PGPROC splitup
On Wed, Dec 14, 2011 at 12:20 PM, Pavan Deolasee pavan.deola...@gmail.com wrote: On Wed, Dec 14, 2011 at 4:15 AM, Tom Lane t...@sss.pgh.pa.us wrote: Without the added assert, you'd only notice this if you were running a standby slave --- the zero xid results in an assert failure in WAL replay on the slave end, which is how I found out about this to start with. But since we've not heard reports of such before, I suspect that this is a recently introduced bug; and personally I'd bet money that it was the PGXACT patch that broke it. I can reproduce this and will look at it in detail. Looking at CommitTransaction(), it seems quite clear to me that we call ProcArrayEndTransaction() before releasing the locks held by the transaction. So its quite possible that when GetRunningTransactionLocks goes through the list of currently held locks, the pgxact-xid is already cleared. This seems to a old bug to me and not related to PGXACT work. In fact, I can force the assertion failure by braking into gdb and pausing the process running the following statements, right after it clears the xid by calling ProcArrayEndTransaction(). At that point, if I hit CHECKPOINT from another session, the assertion fails. Session 1: BEGIN; LOCK TABLE test IN ACCESS EXCLUSIVE MODE; COMMIT; == break after ProcArrayEndTransaction finishes Session 2: CHECKPOINT; Thanks, Pavan -- Pavan Deolasee 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] Patch for cursor calling with named parameters
On 14.12.2011 12:31, Yeb Havinga wrote: On 2011-12-13 18:34, Tom Lane wrote: Heikki Linnakangasheikki.linnakan...@enterprisedb.com writes: Attached is a patch with those changes. I also I removed a few of the syntax error regression tests, that seemed excessive, plus some general naming and comment fiddling. I'll apply this tomorrow, if it still looks good to me after sleeping on it. However, I'm still concerned about whether this approach gives reasonable error messages in cases where the error would be detected during parse analysis of the rearranged statement. The regression test examples don't cover such cases, and I'm too busy right now to apply the patch and check for myself. What happens for example if a named parameter's value contains a misspelled variable reference, or a type conflict? I tested this and seems to be ok: regression=# select namedparmcursor_test1(2, 2) as Should be false, namedparmcursor_test1(20, 20) as Should be true; ERROR: column yy does not exist LINE 1: SELECT x AS param1, yy AS param12; For the record, the caret pointing to the position is there, too. As in: regression=# do $$ declare c1 cursor (param1 int, param2 int) for select 123; begin open c1(param2 := xxx, param1 := 123); end; $$; ERROR: column xxx does not exist LINE 1: SELECT 123 AS param1, xxx AS param2; ^ QUERY: SELECT 123 AS param1, xxx AS param2; CONTEXT: PL/pgSQL function inline_code_block line 5 at OPEN I think that's good enough. It would be even better if we could print the original OPEN statement as the context, as in: ERROR: column xxx does not exist LINE 4: OPEN c1(param2 := xxx, param1 := 123); ^ but it didn't look quite like that before the patch either, and isn't specific to this patch but more of a general usability issue in PL/pgSQL. Committed. -- 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] [PATCH] PostgreSQL fails to build with 32bit MinGW-w64
On 12/14/2011 04:43 AM, Mark Cave-Ayland wrote: On 12/12/11 15:00, Andrew Dunstan wrote: Yeah, fair enough. I'll work on that. If we're talking about adding support for a previously-unsupported Definitely do this (and then file a bug report with the project). I've worked with both Kai and NightStrike from the MingW-W64 project to fix previous bugs, and as long as they can build the offending source themselves then they are very helpful and quick to respond. Done and done (see https://sourceforge.net/tracker/?func=detailaid=3458244group_id=202880atid=983354). Hi Andrew, Did you see Kai's update on the ticket? If this is the case, I know that we have seen similar bugs with PostGIS and the work-around has been to add -ffloat-store to the compiler flags for the affected files if that helps? Hmm. Yeah, if I remove -O0 and instead set CFLAGS to -ffloat-store the error goes away. So, would we want to use that just for this file, or for the whole build? FTR, the comment in the bug reads: AFAICS from report, the issue happens with value 1e200 (as invalid range). The issue I might see here (as it doesn't occure with x64 version, which uses sse instructions instead of x87) is that x87 registers internally have higher precision then 32-bit. So failures in range occure on conversion from FPU register down to memory store. For x64 SSE this is different, as here math operations are really just done in specified precission. Have you checked, if you get different result by using on 32-bit explicit SSE instructions? As things seems to work at -O0, but not at -On (with n 0), it is pretty unlikely that runtime-functions itself are causing this issue. So therefore my guess goes here for internal/external precision of used FPU. 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] Patch to allow users to kill their own queries
On 12/14/2011 05:24 AM, Magnus Hagander wrote: How about passing a parameter to pg_signal_backend? Making pg_signal_backend(int pid, int sig, bool allow_samerole)? That sounds like it will result in less code, and make the API I was documenting be obvious from the parameters instead. I'll refactor to do that, tweak the hint message, and resubmit. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] COUNT(*) and index-only scans
On Wed, Dec 14, 2011 at 6:58 AM, Greg Stark st...@mit.edu wrote: On Mon, Nov 21, 2011 at 6:43 PM, Robert Haas robertmh...@gmail.com wrote: In buffer fill mode, we scan the index and add matching tuples and their CTIDs to the buffer. When the buffer is full or the index AM reports that there are no more tuples in the scan, we switch to buffer drain mode. Instead you could do the two phases concurrently the way tuplesort implements the tapesort from Knuth. You keep a heap of ctids with an epoch. You fill the heap then you return the first one. Whenever you return one you read the next one and add it to the heap. If it falls before the last returned value you insert it with the next epoch but if it falls afterwards you can insert it into the heap in its correct position. Yeah, that's pretty much what I was imagining, although my explanation of it was slightly muddled. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Command Triggers
Excerpts from Dimitri Fontaine's message of mié dic 14 07:22:21 -0300 2011: Again, that's a caveat of the first implementation, you can't have sub commands support without forcing them through ProcessUtility and that's a much more invasive patch. Maybe we will need that later. I can get behind this argument: force all stuff through ProcessUtility for regularity, and not necessarily in the first patch for this feature. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. 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] pg_dump --exclude-table-data
On 12/14/2011 06:28 AM, Peter Geoghegan wrote: Not sure that I have a lot to add here, but I am officially listed as a reviewer, which is a responsibility that I don't want to shirk. In my opinion, this patch is obviously useful. I don't find the asymmetry that it will create with pg_restore to be troubling, so I'd favour committing it as-is. Extremely minor problem noted: There are two spaces at the start of one sentence in your SGML doc updates. Thanks. Committed with that changed, although we seem to be getting altogether too obsessive about white space, IMNSHO. 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] [PATCH] PostgreSQL fails to build with 32bit MinGW-w64
On 14/12/11 13:59, Andrew Dunstan wrote: Hmm. Yeah, if I remove -O0 and instead set CFLAGS to -ffloat-store the error goes away. So, would we want to use that just for this file, or for the whole build? Well the latest documentation for gcc gives 2 options: -ffloat-store and -fexcess-precision=style which are documented at http://gcc.gnu.org/onlinedocs/gcc-4.6.2/gcc/Optimize-Options.html#Optimize-Options. For PostGIS we only applied the flags for specific files, because of severe performance warnings in older versions of the gcc documentation such as this: http://www.delorie.com/gnu/docs/gcc/gcc_10.html. I have no idea whether these warnings still hold true or not for more modern compiler versions. ISTM that the best solution would be to determine whether or not -fexcess-precision=standard does the right thing (and also determine the performance hit) or take a look at the excess precision notes in the older documentation to see if parts of the code can be rearranged to eliminate the effect. ATB, Mark. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dump --exclude-table-data
On 14 December 2011 14:31, Andrew Dunstan and...@dunslane.net wrote: Thanks. Committed with that changed, although we seem to be getting altogether too obsessive about white space, IMNSHO. I agree, but I think it's important that we judge patches by a consistent standard. Right now, for better or worse, that standard includes being obsessed with white space. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: URI connection string support for libpq
Excerpts from Greg Smith's message of Wed Dec 14 02:54:14 +0200 2011: Initial quick review of your patch: you suggested this as the general form: psql -d postgresql://user@pw:host:port/dbname?param1=value1param2=value2... That's presumably supposed to be: psql -d postgresql://user:pw@host:port/dbname?param1=value1param2=value2... Yes, that was clearly a typo, so user:pw@host:port. If we had to pick one URI prefix, it should be postgres. But given the general name dysfunction around this project, I can't see how anyone would complain if we squat on postgresql too. That'd be true if we've started afresh in the absence of any existing URI implementations. IMO, what makes a connection URI useful is: a) it keeps all the connection parameters in a single string, so you can easily send it to other people to use, and b) it works everywhere, so the people who've got the URI can use it and expect to get the same results as you do. (Well, not without some quirks, like effects of locally-set environment variables or presence of .pgpass or service files, or different nameserver opinions about which hostname resolves to which IP address, but that is pretty much the case with any sort of URIs.) This is not in objection to what you say, but rather an important thing to keep in mind for the purpose of this discussion. Whatever decision we make here, the libpq-binding connectors are going to be compatible with each other automatically if they just pass the URI to libpq. However, should we stick to using postgresql:// URI prefix exclusively, these might need to massage the URI a bit before passing further (like replacing postgres:// with postgresql://, also accepting the latter should be reasonable.) With proper recommendations from our side, the new client code will use the longer prefix, thus achieving compatibility with the only(?) driver not based on libpq (that is, JDBC) in the long run. Attached patch modifies yours to prove we can trivially support both, in hopes of detonating this argument before it rages on further. Tested like this: $ psql -d postgres://gsmith@localhost:5432/gsmith And that works too now. I doubt either of us like what I did to the handoff between conninfo_uri_parse and conninfo_uri_parse_options to achieve that, but this feature is still young. Yes, the caller could just do the pointer arithmetics itself, since the exact URI prefix is known at the time, then pass it to conninfo_uri_parse. After this bit of tinkering with the code, it feels to me like this really wants a split() function to break out the two sides of a string across a delimiter, eating it in the process. Adding the level of paranoia I'd like around every bit of code I see that does that type of operation right now would take a while. Refactoring in terms of split and perhaps a few similarly higher-level string parsing operations, targeted for this job, might make it easier to focus on fortifying those library routines instead. For example, instead of the gunk I just added that moves past either type of protocol prefix, I'd like to just say split(buf,://,left,right) and then move on with processing the right side. A search with cscope over my repo clone doesn't give any results for split, so I assume you're talking about a new function with a signature similar to the following: split(char *buf, const char *delim, char **left, char **right) Note, there should be no need for parameter left, since that will be pointing to the start of buf. Also, we might just return right as a function's value instead of using out-parameter, with NULL meaning delimiter was not found in the buffer. Now, if you look carefully at the patch's code, there are numerous places where it accepts either of two delimiting characters and needs to examine one before zeroing it out, so it'll need something more like this: char *need_a_good_name_for_this(char *buf, const char *possible_delims, char *actual_delim) where it will store a copy of encountered delimiting char in *actual_delim before modifying the buffer. I agree with your comment that we need to add some sort of regression tests for this. Given how the parsing is done right now, we'd want to come up with some interesting invalid strings too. Making sure this fails gracefully (and not in a buffer overflow way) might even use something like fuzz testing too. Around here we've just been building some Python scripting to do that sort of thing, tests that aren't practical to do with pg_regress. I'd appreciate if you could point me to any specific example of such existing tests to take some inspiration from. -- Regards, Alex -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] LibreOffice driver 1: Building libpq with Mozilla LDAP instead of OpenLDAP
Pavel Golub pa...@microolap.com writes: You wrote: TL about OSX though. (You're aware that Apple ships a perfectly fine TL libpq.so in Lion, no?) Is it true? Really? Where can we read about it? /Library/WebServer/Documents/postgresql/html ... I don't know where else Apple documents this, but there's a complete-looking set of client-side libraries and command line tools from Postgres 9.0.4 in base Lion. I understand that if you buy Lion Server you get the postmaster too, but I haven't done that so I can't verify it from personal experience. libpq.dylib is definitely right there in /usr/lib though, and it apparently is well-configured, because /usr/bin/pg_config says CONFIGURE = '--mandir=/usr/share/man' '--infodir=/usr/share/info' '--disable-dependency-tracking' '--prefix=/usr' '--sbindir=/usr/libexec' '--sysconfdir=/private/etc' '--localstatedir=/var/pgsql' '--htmldir=/Library/WebServer/Documents/postgresql' '--enable-thread-safety' '--enable-dtrace' '--with-tcl' '--with-perl' '--with-python' '--with-gssapi' '--with-krb5' '--with-pam' '--with-ldap' '--with-bonjour' '--with-openssl' '--with-libxml' '--with-libxslt' '--with-system-tzdata=/usr/share/zoneinfo' 'CFLAGS=-arch x86_64 -arch i386 -pipe -Os -g -Wall -Wno-deprecated-declarations' 'LDFLAGS=-arch x86_64 -arch i386 -pipe -Os -g -Wall -Wno-deprecated-declarations' 'LDFLAGS_EX=-mdynamic-no-pic' I've not made an attempt to use it directly myself, but it sure looks like it should do what the OP wants. 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] NOTIFY with tuples
On Tue, Dec 13, 2011 at 11:27 PM, Thomas Munro mu...@ip9.org wrote: Actually, what I'd suggest is just some code to serialize and deserialize tuples and transmit 'em via the existing NOTIFY payload facility. I agree that presenting it as some functions would be a lot less work than inventing bespoke syntax, but what you sketched still involves writing a lot of communications infrastructure from scratch, and I'm not sure it's worth doing that. Thank you both for your feedback! Looking at commands/async.c, it seems as thought it would be difficult for function code running in the backend to get its hands on the payload containing the serialized tuple, since the notification is immediately passed to the client in NotifyMyFrontEnd and there is only one queue for all notifications, you can't just put things back or not consume some of them yet IIUC. Maybe the code could changed to handle payloads holding serialized tuples differently, and stash them somewhere backend-local rather than sending to the client, so that a function returning SETOF (or a new executor node type) could deserialize them when the user asks for them. Or did you mean that libpq could support deserializing tuples on the client side? One way of grabbing notifications in a backend function would be via dblink -- you LISTEN on a sideband connection and grab notifications via http://www.postgresql.org/docs/9.1/interactive/contrib-dblink-get-notify.html. As to the wider point I'm wondering why you can't layer your API on top of existing facilities (tables, notifications, etc). PGQ (have you seen that?) does this and it's an absolute marvel. Meaning, I bet you could do this with an 'all sql (or plpgsql)' implementation. That's a good thing -- C code significantly raises the bar in terms of putting your code in the hands of people who might be interested in using it. merlin -- Sent 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 to allow users to kill their own queries
Magnus Hagander mag...@hagander.net writes: On Tue, Dec 13, 2011 at 11:59, Greg Smith g...@2ndquadrant.com wrote: HINT: you can use pg_cancel_backend() on your own processes That HINT sounds a bit weird to me. you can cancel your own queries using pg_cancel_backend() instead or something like that? Doesn't follow message style guidelines either ;-). Hints should be complete sentences, with initial cap and ending period. http://www.postgresql.org/docs/devel/static/error-style-guide.html 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] Race condition in HEAD, possibly due to PGPROC splitup
Pavan Deolasee pavan.deola...@gmail.com writes: BTW, on an unrelated note, I was looking at the way ShmemInitStruct() is used. It internally uses ShmemAlloc to allocate from shared memory. ShmemAlloc always MAXALIGN the requested size. But while calculating the total shared memory requirement, we don't always MAXALIGN individual structure sizes. One example is KnownAssignedXidsValid, but I am sure there are other places where the originally computed and the requested sizes could be different because of alignment. I wonder if we are just lucky not to hit shared memory size mismatch, may be because we round up to the block size at the end. That sort of thing is down in the noise. One reason we throw in the 100KB slop is so we don't have to sweat details like 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] Race condition in HEAD, possibly due to PGPROC splitup
Pavan Deolasee pavan.deola...@gmail.com writes: Looking at CommitTransaction(), it seems quite clear to me that we call ProcArrayEndTransaction() before releasing the locks held by the transaction. So its quite possible that when GetRunningTransactionLocks goes through the list of currently held locks, the pgxact-xid is already cleared. This seems to a old bug to me and not related to PGXACT work. Hm. So maybe the correct fix is to deem the lock already released if we get zero when we read the xid? It's not clear to me what the requirements for GetRunningTransactionLocks actually are, but if it's okay for it to think a lock is released slightly ahead of when the rest of the system thinks so, that would work. 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] foreign key locks, 2nd attempt
On Tue, Dec 13, 2011 at 06:36:21PM -0300, Alvaro Herrera wrote:
Excerpts from Noah Misch's message of dom dic 04 09:20:27 -0300 2011:
@@ -2725,11 +2884,20 @@ l2:
oldtup.t_data-t_infomask = ~(HEAP_XMAX_COMMITTED |
HEAP_XMAX_INVALID |
HEAP_XMAX_IS_MULTI |
- HEAP_IS_LOCKED |
+ HEAP_LOCK_BITS |
HEAP_MOVED);
+oldtup.t_data-t_infomask2 = ~HEAP_UPDATE_KEY_INTACT;
HeapTupleClearHotUpdated(oldtup);
/* ... and store info about transaction updating this tuple */
-HeapTupleHeaderSetXmax(oldtup.t_data, xid);
+if (TransactionIdIsValid(keep_xmax_old))
+{
+HeapTupleHeaderSetXmax(oldtup.t_data, keep_xmax_old);
+oldtup.t_data-t_infomask |= keep_xmax_old_infomask;
+}
+else
+HeapTupleHeaderSetXmax(oldtup.t_data, xid);
+if (key_intact)
+oldtup.t_data-t_infomask2 |= HEAP_UPDATE_KEY_INTACT;
HeapTupleHeaderSetCmax(oldtup.t_data, cid, iscombo);
/* temporarily make it look not-updated */
oldtup.t_data-t_ctid = oldtup.t_self;
Shortly after this, we release the content lock and go off toasting the
tuple
and finding free space. When we come back, could the old tuple have
accumulated additional KEY SHARE locks that we need to re-copy?
Yeah, I've been wondering about this: do we have a problem already with
exclusion constraints? I mean, if a concurrent inserter doesn't see the
tuple that we've marked here as deleted while we toast it, it could
result in a violated constraint, right? I haven't built a test case to
prove it.
Does the enforcement code for exclusion constraints differ significantly from
the ordinary unique constraint code? If not, I'd expect the concurrent inserter
to treat the tuple precisely like an uncommitted delete, in which case it will
wait for the deleter.
I settled on this:
/*
* If any subtransaction of the current top transaction already
holds a
* lock as strong or stronger than what we're requesting, we
* effectively hold the desired lock already. We *must* succeed
* without trying to take the tuple lock, else we will deadlock
against
* anyone wanting to acquire a stronger lock.
*/
Now, I can't see the reason that we didn't previously consider locks as
strong as what we're requesting ... but surely it's the same case?
I think it does degenerate to the same case. When we hold an exclusive lock
in master, HeapTupleSatisfiesUpdate() will return HeapTupleMayBeUpdated. So,
we can only get here while holding a mere share lock.
In both HEAP_XMAX_MULTI conditional blocks, you do not set HEAP_XMAX_INVALID
for an aborted updater. What is the new meaning of HEAP_XMAX_INVALID for
multixacts? What implications would arise if we instead had it mean that
the
updating xid is aborted? That would allow us to get the mid-term
performance
benefit of the hint bit when the updating xid spills into a multixact, and
it
would reduce code duplication in this function.
Well, HEAP_XMAX_INVALID means the Xmax is not valid, period. If there's
a multi whose updater is aborted, there's still a multi that needs to be
checked in various places, so we cannot set that bit.
Ah, yes. Perhaps a better question: would changing HEAP_XMAX_INVALID to
HEAP_UPDATER_INVALID pay off? That would help HeapTupleSatisfiesMVCC() at the
expense of HeapTupleSatisfiesUpdate(), probably along with other consequences I
haven't contemplated adequately.
--
Sent 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: CHECK FUNCTION statement
Pavel Stehule wrote:
changes:
* fixed warnings
* support for options - actually only two options are supported -
quite and fatal_errors
these options are +/- useful - main reason for their existence is
testing of support of options - processing on CHECK ... stmt side and
processing on checker function side.
options are send as 2d text array - some like
'{{quite,on},{fatal_errors,on}} - so direct call of checker function
is possible
* regress test for multi check
First of all: It should be quiet and not quite.
The patch applies and builds fine.
It fails one of ist own regression tests, here is the diff:
*** /postgres/cvs/postgresql/src/test/regress/expected/plpgsql.out
2011-12-14 11:50:44.0 +0100
--- /postgres/cvs/postgresql/src/test/regress/results/plpgsql.out
2011-12-14 16:19:45.0 +0100
***
*** 4975,4991
end;
$$ language plpgsql;
check function all in schema plpgsql_check;
- NOTICE: checked function plpgsql_check.fce1()
WARNING: error in function plpgsql_check.fce2()
DETAIL: too few parameters specified for RAISE
CONTEXT: line 3 at RAISE
NOTICE: checked function plpgsql_check.fce3()
check function all in schema plpgsql_check with (quite);
WARNING: error in function plpgsql_check.fce2()
DETAIL: too few parameters specified for RAISE
CONTEXT: line 3 at RAISE
check function all in schema plpgsql_check with (fatal_errors);
- NOTICE: checked function plpgsql_check.fce1()
ERROR: too few parameters specified for RAISE
CONTEXT: PL/pgSQL function fce2 line 3 at RAISE
check function all in schema plpgsql_check with (quite, fatal_errors on);
--- 4975,4990
end;
$$ language plpgsql;
check function all in schema plpgsql_check;
WARNING: error in function plpgsql_check.fce2()
DETAIL: too few parameters specified for RAISE
CONTEXT: line 3 at RAISE
NOTICE: checked function plpgsql_check.fce3()
+ NOTICE: checked function plpgsql_check.fce1()
check function all in schema plpgsql_check with (quite);
WARNING: error in function plpgsql_check.fce2()
DETAIL: too few parameters specified for RAISE
CONTEXT: line 3 at RAISE
check function all in schema plpgsql_check with (fatal_errors);
ERROR: too few parameters specified for RAISE
CONTEXT: PL/pgSQL function fce2 line 3 at RAISE
check function all in schema plpgsql_check with (quite, fatal_errors on);
==
The quiet option is not very intuitive:
test= CHECK FUNCTION ALL WITH (quite 'off');
NOTICE: skip check function atrig(), it is trigger function
NOTICE: skip check function perl_max(integer,integer), language plperl
hasn't checker function
NOTICE: checked function ok()
NOTICE: checked function newstyle(integer)
CHECK FUNCTION
test= CHECK FUNCTION ALL WITH (quite 'on');
NOTICE: skip check function atrig(), it is trigger function
CHECK FUNCTION
I understand that quiet cannot silence this message, nor
skip ..., uses C language and skip ..., it uses internal language,
but that means that it is not very useful as it is.
If all we need is a sample option, I think that fatal_errors is
enough, and I think that is an option that can be useful.
Yours,
Laurenz Albe
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] foreign key locks, 2nd attempt
Noah Misch n...@leadboat.com writes: On Tue, Dec 13, 2011 at 06:36:21PM -0300, Alvaro Herrera wrote: Yeah, I've been wondering about this: do we have a problem already with exclusion constraints? I mean, if a concurrent inserter doesn't see the tuple that we've marked here as deleted while we toast it, it could result in a violated constraint, right? I haven't built a test case to prove it. Does the enforcement code for exclusion constraints differ significantly from the ordinary unique constraint code? It's an entirely separate code path (involving an AFTER trigger). I don't know if there's a problem, but Alvaro's right to worry that it might behave differently. 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] psql output locations
On Wed, Dec 14, 2011 at 4:45 AM, Magnus Hagander mag...@hagander.net wrote: * There are a number of things that are always written to stdout, that there is no way to redirect. In some cases it's interactive prompts - makes sense - but also for example the output of \timing goes to stdout always. Is there some specific logic behind what/when this should be done? Everything that is not an error goes to stdout, no? Except the query output, if you change it. Maybe the way to do what you want is to invent a new setting that temporarily changes stdout. Yeah, that might be it. Or I need separate settings for put errors in the query output stream and put non-query-output-but-also-non-errors in the query output stream. The effect would be the same, I guess... That seems an awful lot harder (and messier) than just changing the all the call sites to use the same error-reporting function. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [REVIEW] pg_last_xact_insert_timestamp
On Mon, Dec 12, 2011 at 12:17 PM, Simon Riggs si...@2ndquadrant.com wrote:
On Sat, Dec 10, 2011 at 12:29 PM, Greg Smith g...@2ndquadrant.com wrote:
We can send regular special messages from WALSender to WALReceiver that do
not form part of the WAL stream, so we don't bulk
up WAL archives. (i.e. don't use w messages).
Here's my understanding of how this would work.
Let me explain a little more and provide a very partial patch.
We define a new replication protocol message 'k' which sends a
keepalive from primary to standby when there is no WAL to send. The
message does not form part of the WAL stream so does not bloat WAL
files, nor cause them to fill when unattended.
Keepalives contain current end of WAL and a current timestamp.
Keepalive processing is all done on the standby and there is no
overhead on a primary which does not use replication. There is a
slight overhead on primary for keepalives but this happens only when
there are no writes. On the standby we already update shared state
when we receive some data, so not much else to do there.
When the standby has applied up to the end of WAL the replication
delay is receipt time - send time of keepalive.
Patch introduces regular keepalives from WALsender to WALreceiver,
using a new protocol message 'k'.
These are sent at intervals of a fraction of replication_delay or 10s
if not set,
whenever no WAL records have been sent recently.
Patch exposes info used for standby_delay calculation as used in 9.0+
In addition introduces direct calculations of replication apply delay
and replication transfer latency, both in ms.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index d6332e5..71c40cc 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1467,6 +1467,54 @@ The commands accepted in walsender mode are:
variablelist
varlistentry
term
+ Primary keepalive message (B)
+ /term
+ listitem
+ para
+ variablelist
+ varlistentry
+ term
+ Byte1('k')
+ /term
+ listitem
+ para
+ Identifies the message as a sender keepalive.
+ /para
+ /listitem
+ /varlistentry
+ varlistentry
+ term
+ Byte8
+ /term
+ listitem
+ para
+ The current end of WAL on the server, given in
+ XLogRecPtr format.
+ /para
+ /listitem
+ /varlistentry
+ varlistentry
+ term
+ Byte8
+ /term
+ listitem
+ para
+ The server's system clock at the time of transmission,
+ given in TimestampTz format.
+ /para
+ /listitem
+ /varlistentry
+ /variablelist
+ /para
+ /listitem
+ /varlistentry
+ /variablelist
+ /para
+
+ para
+ variablelist
+ varlistentry
+ term
Standby status update (F)
/term
listitem
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 9d96044..77e2760 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -452,6 +452,9 @@ typedef struct XLogCtlData
XLogRecPtr recoveryLastRecPtr;
/* timestamp of last COMMIT/ABORT record replayed (or being replayed) */
TimestampTz recoveryLastXTime;
+ /* timestamp of when we started replaying the current chunk of WAL data,
+ * only relevant for replication or archive recovery */
+ TimestampTz currentChunkStartTime;
/* end of the last record restored from the archive */
XLogRecPtr restoreLastRecPtr;
/* Are we requested to pause recovery? */
@@ -606,6 +609,7 @@ static void exitArchiveRecovery(TimeLineID endTLI,
static bool recoveryStopsHere(XLogRecord *record, bool *includeThis);
static void recoveryPausesHere(void);
static void SetLatestXTime(TimestampTz xtime);
+static void SetCurrentChunkStartTime(TimestampTz xtime);
static void CheckRequiredParameterValues(void);
static void XLogReportParameters(void);
static void LocalSetXLogInsertAllowed(void);
@@ -5846,6 +5850,41 @@ GetLatestXTime(void)
}
/*
+ * Save timestamp of the next chunk of WAL records to apply.
+ *
+ * We keep this in XLogCtl, not a simple static variable, so that it can be
+ * seen by all backends.
+ */
+static void
+SetCurrentChunkStartTime(TimestampTz xtime)
+{
+ /* use volatile pointer to prevent code rearrangement */
+ volatile XLogCtlData *xlogctl = XLogCtl;
+
+ SpinLockAcquire(xlogctl-info_lck);
+ xlogctl-currentChunkStartTime = xtime;
+ SpinLockRelease(xlogctl-info_lck);
+}
+
+/*
+ * Fetch timestamp of latest processed commit/abort record.
+ * Startup process maintains an accurate local copy in XLogReceiptTime
+ */
+TimestampTz
+GetCurrentChunkReplayStartTime(void)
+{
+ /* use volatile pointer to prevent code rearrangement */
+ volatile XLogCtlData *xlogctl = XLogCtl;
+ TimestampTz xtime;
+
+
Re: [HACKERS] NOTIFY with tuples
On 14 December 2011 15:10, Merlin Moncure mmonc...@gmail.com wrote: As to the wider point I'm wondering why you can't layer your API on top of existing facilities (tables, notifications, etc). PGQ (have you seen that?) does this and it's an absolute marvel. Meaning, I bet you could do this with an 'all sql (or plpgsql)' implementation. That's a good thing -- C code significantly raises the bar in terms of putting your code in the hands of people who might be interested in using it. Well I was interested in the idea of using the NOTIFY payload somehow for high performance (it's not backed by a table that gets fsynced and needs to be vacuumed etc, and it delivers data to clients without an extra round trip), and I guess also really like the idea of streams being first class objects in a kind of StreamSQL-lite language extension. But I've been playing around with Robert's suggestion, and I realised that I can dress up the foo_read and foo_write functions (probably written in pure plpgsql) with a VIEW so that I can INSERT and SELECT tuples, and to be able to join it against other tables. Here's what I have working so far: https://github.com/macdice/pg_stream/blob/master/hack.sql I guess at this point this becomes off topic for pgsql-hackers. Thanks all for the pointers and ideas. PGQ looks interesting, I'll check it out. -- Sent 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] PostgreSQL fails to build with 32bit MinGW-w64
Andrew Dunstan and...@dunslane.net writes: Done and done (see https://sourceforge.net/tracker/?func=detailaid=3458244group_id=202880atid=983354). Did you see Kai's update on the ticket? If this is the case, I know that we have seen similar bugs with PostGIS and the work-around has been to add -ffloat-store to the compiler flags for the affected files if that helps? Hmm. Yeah, if I remove -O0 and instead set CFLAGS to -ffloat-store the error goes away. Hmm, we have been bit by that recently elsewhere: http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=ff68b256a533b398e3420750f34d161aeee4e099 I suspect what you are looking at is not at all mingw-specific but will soon start showing up on other x86 platforms. I see from the bug report that that's gcc 4.7.0, which hasn't made it into most distros yet but surely will soon. So, would we want to use that just for this file, or for the whole build? -ffloat-store is a brute force solution, I think, and would affect old versions of gcc that don't exhibit any problems. I would suggest altering configure to see whether the compiler recognizes -fexcess-precision=standard and adding that to CFLAGS if so. 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] pg_dump --exclude-table-data
On Dec 14, 2011, at 6:31 AM, Andrew Dunstan wrote: Thanks. Committed with that changed, although we seem to be getting altogether too obsessive about white space, IMNSHO. If that’s all there is to complain about, I think it’s a pretty good sign. ;-P David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] PostgreSQL fails to build with 32bit MinGW-w64
On Wed, Dec 14, 2011 at 11:14 AM, Tom Lane t...@sss.pgh.pa.us wrote: -ffloat-store is a brute force solution, I think, and would affect old versions of gcc that don't exhibit any problems. I would suggest altering configure to see whether the compiler recognizes -fexcess-precision=standard and adding that to CFLAGS if so. Would it be better to change either the code or the test case to be less sensitive to this issue? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] VACUUM in SP-GiST
I started reading the spgist vacuum code last night, and didn't like it at all. I found a number of smaller issues, but it seems to me that the design is just fundamentally wrong. Unless I'm misunderstanding it, the approach is to recursively traverse the tree in sort of the same way that a full-index search would do, except that it continues to hold lock on the immediate parent page of whichever page is currently being visited, so that it can update the downlink if it has to delete the first leaf tuple of a chain. I've got several complaints about that: 1. Holding exclusive locks on upper pages while we visit possibly-hundreds of leaf pages seems pretty bad from a concurrency standpoint. It doesn't hold locks all the way up to the root, so it's not a complete loss, but even a first-level inner page can have a lot of children. 2. I do not trust this code to visit every leaf page (which, if it failed to do so and thereby missed deleting a heap reference, would result in a silently corrupt index). Unlike a regular index search, which makes a list of lower-level targets while examining an inner tuple just once, this code depends on the assumption that it can reacquire lock on an upper page and then resychronize itself with whatever's been done meanwhile to the inner tuple it was looking at. I don't trust that. The mechanism that's being used relies on page LSN having been changed if anything was changed on the page, which does not work in unlogged tables. Furthermore, if the inner tuple did change, it has to rescan *everything* that the inner tuple leads to. That's horrendously inefficient, and it's not even clear that it would ever terminate in the face of a constant stream of concurrent updates. 3. Even if it all worked, it would be slow as can be, because it requires a whole lot of nonsequential accesses to the index. For instance, the same leaf page might be visited many times (once for each leaf chain on the page), and not necessarily close together either. Also, the code doesn't seem to perform any cleanup at all on inner pages. I am not expecting it to try to get rid of childless inner tuples, but surely it ought to at least convert old redirects to dead and get rid of dead tuples at the end of the page, much as for leaf pages? What I'm envisioning doing instead is making a single sequential scan over the entire index. On both leaf and inner pages we could clean redirects and remove end dead tuples. On leaf pages we'd also check for tuples to be deleted. There's no real difficulty in removing deleted tuples as long as they are not at the heads of their chains. (The code would have to reverse-engineer which tuples are at the heads of their chains, but that's easily done while scanning the page; we just make a reverse-lookup map for the nextOffset pointers.) If we have to delete a tuple at the head of its chain, but there's still at least one live tuple in its chain, we could reshuffle the tuples to bring another live tuple to that same offset number, thereby not invalidating the parent downlink. The only hard part is what to do when we delete all members of a chain: we can't reset the parent downlink because we do not know where it is. What I propose to do in that case is install a variant form of dead tuple that just indicates this chain is empty. One way to represent that would be a redirect tuple pointing nowhere, but I think it would be cleaner to repurpose the isDead and isRedirect bits as a two-bit field with four states. We'd have LIVE, DEAD, REDIRECT, and this fourth state for a dead tuple that is not recyclable because we know there's a link to it somewhere. A scan would ignore such a tuple. An insertion that arrives at such a tuple could either replace it (if there's enough room on the page) or convert it to a redirect (if not). Last night I was thinking the fourth state could be named TOMBSTONE, but maybe it'd be better to use DEAD for this case and rename the existing removable dead tuple state to PLACEHOLDER, since that case only exists to preserve the offset numbers of other tuples on the page. Comments? 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] PostgreSQL fails to build with 32bit MinGW-w64
Robert Haas robertmh...@gmail.com writes: On Wed, Dec 14, 2011 at 11:14 AM, Tom Lane t...@sss.pgh.pa.us wrote: -ffloat-store is a brute force solution, I think, and would affect old versions of gcc that don't exhibit any problems. I would suggest altering configure to see whether the compiler recognizes -fexcess-precision=standard and adding that to CFLAGS if so. Would it be better to change either the code or the test case to be less sensitive to this issue? AFAICS it's really impractical to do that. The code Andrew is having problems with is essentially double a,b,c; ... a = b * c; if (isinf(a)) throw error; and the problem is that the multiplication result overflows in double precision, but not in the wider-than-double register precision. Therefore, if a is in a register and the isinf() primitive inspects the register, it will return false, even though when the value gets stored to memory it will become an infinity. I don't see anything we can do to the code that avoids this issue. You might think that explicitly casting b * c to double would help, but our experiments in connection with the planner Assert case proved it didn't. The only other thing we could possibly do is move the multiplication into a separate subroutine, but what's to stop the compiler from inlining that and generating the same code anyway? Basically, what's going on here is that the gcc boys have decided that speed trumps both sanity and conformance to the letter of the C standard, unless you turn on compiler switches that say please act sane. So we'd better do that, unless you'd like to be dealing with this type of issue for the rest of the project's lifespan. It's much the same type of problem as with -fno-strict-aliasing, except that someday we might consider biting the bullet and dealing with that piece of insanity-in-the-name-of-speed. Floating-point performance is not interesting enough for Postgres' purposes that I can imagine that we'd ever want to deal with this kind of gotcha to improve FP speed. 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] PostgreSQL fails to build with 32bit MinGW-w64
On Wed, Dec 14, 2011 at 12:36 PM, Tom Lane t...@sss.pgh.pa.us wrote: AFAICS it's really impractical to do that. The code Andrew is having problems with is essentially double a,b,c; ... a = b * c; if (isinf(a)) throw error; and the problem is that the multiplication result overflows in double precision, but not in the wider-than-double register precision. Therefore, if a is in a register and the isinf() primitive inspects the register, it will return false, even though when the value gets stored to memory it will become an infinity. Uh, wow. That really is pretty insane. How is anyone supposed to write sensible code around that non-API? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] SP-GiST versus index-only scans
It would not take very much at all to make the SP-GiST stuff support index-only scans, except for one thing: it's conceivable that an opclass might choose to store only a lossy version of the original indexed datum, in which case it'd not be able to reconstruct the datum on demand. I'm not sure how likely this is, because the design of SP-GiST forces root-level leaf tuples to be stored with the original datum, but in principle leaf tuples at lower levels might have lossy representations. None of the current opclasses do that, but Oleg and Teodor evidently foresee such cases, else they'd not have bothered with the recheck support that's in there. (If the original datum is reconstructable then the opclass can surely deliver a correct answer for every leaf tuple.) So the problem is that we have to either disallow such opclass designs, or support per-opclass rather than per-index-AM decisions about whether index-only scans are possible. We discussed that idea when the index-only-scans patch went in a few months ago, but blew it off on the grounds that there didn't seem to be any immediate need for it. Well, the need is here. I think we should get rid of the pg_am.amcanreturn boolean column and replace it with an AM support function. btree's would just return constant true, but I envision SP-GiST's version interrogating the opclass config function. (If we make the config function return this info, we don't need to add anything to CREATE OPERATOR CLASS, which seems like a good thing to me.) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] PostgreSQL fails to build with 32bit MinGW-w64
Robert Haas robertmh...@gmail.com writes: Uh, wow. That really is pretty insane. How is anyone supposed to write sensible code around that non-API? Usability seems to be very low on the gcc project's list of goals these days. Personally I think this sort of thing might be fine if it were triggered by -ffast-math or something like that. But as a default behavior it's entirely ridiculous. 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] SP-GiST versus index-only scans
On 2011-12-14 19:00, Tom Lane wrote:
So the problem is that we have to either disallow such opclass designs,
or support per-opclass rather than per-index-AM decisions about whether
index-only scans are possible.
Just a quick comment, for some queries like the famous
select count(*) from table where column @@ to_tsquery('something');
I do think index-only-scans does make sense even on indices
where the original tuple cannot be re-constructed. This also goes
for gin indices as well.
.. and yes, I do have a real-world application that would utillize this.
(and love it)
Jesper
--
Jesper
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SP-GiST versus index-only scans
Jesper Krogh jes...@krogh.cc writes:
On 2011-12-14 19:00, Tom Lane wrote:
So the problem is that we have to either disallow such opclass designs,
or support per-opclass rather than per-index-AM decisions about whether
index-only scans are possible.
Just a quick comment, for some queries like the famous
select count(*) from table where column @@ to_tsquery('something');
I do think index-only-scans does make sense even on indices
where the original tuple cannot be re-constructed. This also goes
for gin indices as well.
I think this is somewhat wishful thinking unfortunately. The difficulty
is that if the index isn't capable of reconstructing the original value,
then it's probably giving only an approximate (lossy) answer, which
means we'll have to visit the heap to recheck each result, which
pretty much defeats the purpose of an index-only scan. So I can't get
excited about contorting things to support this.
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] Race condition in HEAD, possibly due to PGPROC splitup
On Wed, Dec 14, 2011 at 3:20 PM, Tom Lane t...@sss.pgh.pa.us wrote: Pavan Deolasee pavan.deola...@gmail.com writes: Looking at CommitTransaction(), it seems quite clear to me that we call ProcArrayEndTransaction() before releasing the locks held by the transaction. So its quite possible that when GetRunningTransactionLocks goes through the list of currently held locks, the pgxact-xid is already cleared. This seems to a old bug to me and not related to PGXACT work. Hm. So maybe the correct fix is to deem the lock already released if we get zero when we read the xid? It's not clear to me what the requirements for GetRunningTransactionLocks actually are, but if it's okay for it to think a lock is released slightly ahead of when the rest of the system thinks so, that would work. OK, I'll look at this. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] PostgreSQL fails to build with 32bit MinGW-w64
On 12/14/2011 11:14 AM, Tom Lane wrote: -ffloat-store is a brute force solution, I think, and would affect old versions of gcc that don't exhibit any problems. I would suggest altering configure to see whether the compiler recognizes -fexcess-precision=standard and adding that to CFLAGS if so. OK, this and the associated configure change seems to do the trick: diff --git a/configure.in b/configure.in index 9cf084d..b29bb61 100644 --- a/configure.in +++ b/configure.in @@ -437,6 +437,8 @@ if test $GCC = yes -a $ICC = no; then PGAC_PROG_CC_CFLAGS_OPT([-fno-strict-aliasing]) # Disable optimizations that assume no overflow; needed for gcc 4.3+ PGAC_PROG_CC_CFLAGS_OPT([-fwrapv]) + # Disable FP optimizations that cause isinf errors on gcc 4.5+ + PGAC_PROG_CC_CFLAGS_OPT([-fexcess-precision=standard]) elif test $ICC = yes; then # Intel's compiler has a bug/misoptimization in checking for # division by NAN (NaN == 0), -mp1 fixes it, so add it to the CFLAGS. I guess we should backpatch it? 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] SP-GiST versus index-only scans
On 2011-12-14 19:48, Tom Lane wrote:
Jesper Kroghjes...@krogh.cc writes:
On 2011-12-14 19:00, Tom Lane wrote:
So the problem is that we have to either disallow such opclass designs,
or support per-opclass rather than per-index-AM decisions about whether
index-only scans are possible.
Just a quick comment, for some queries like the famous
select count(*) from table where column @@ to_tsquery('something');
I do think index-only-scans does make sense even on indices
where the original tuple cannot be re-constructed. This also goes
for gin indices as well.
I think this is somewhat wishful thinking unfortunately. The difficulty
is that if the index isn't capable of reconstructing the original value,
then it's probably giving only an approximate (lossy) answer, which
means we'll have to visit the heap to recheck each result, which
pretty much defeats the purpose of an index-only scan. So I can't get
excited about contorting things to support this.
I can see that it is hard to generalize, but in the tsvector case the
we are indeed not capable of reconstructing the row since the
positions are not stored in the index, the actual lookup is not a
lossy and I'm fairly sure (based on experience) that pg dont
revisit heap-tuples for checking (only for visibillity).
--
Jesper
--
Jesper
--
Sent 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] PostgreSQL fails to build with 32bit MinGW-w64
Andrew Dunstan and...@dunslane.net writes: + # Disable FP optimizations that cause isinf errors on gcc 4.5+ + PGAC_PROG_CC_CFLAGS_OPT([-fexcess-precision=standard]) Looks sane to me, except isinf errors is an awfully narrow reading of the problem. Maybe just say assorted errors? Also, do we know that gcc 4.5 poses the issue? I'm only aware of reports for 4.6 and 4.7. I guess we should backpatch it? +1. Back branches will see these same problems as soon as anybody tries to compile them with latest-n-greatest gcc. 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] SP-GiST versus index-only scans
Jesper Krogh jes...@krogh.cc writes: On 2011-12-14 19:48, Tom Lane wrote: I think this is somewhat wishful thinking unfortunately. The difficulty is that if the index isn't capable of reconstructing the original value, then it's probably giving only an approximate (lossy) answer, which means we'll have to visit the heap to recheck each result, which pretty much defeats the purpose of an index-only scan. I can see that it is hard to generalize, but in the tsvector case the we are indeed not capable of reconstructing the row since the positions are not stored in the index, the actual lookup is not a lossy and I'm fairly sure (based on experience) that pg dont revisit heap-tuples for checking (only for visibillity). Well, the way the tsvector code handles this stuff is that it reports the result as lossy only if the query actually poses a constraint on position (some do, some don't). That case was actually what made us move the determination of lossiness from plan time to execution time, since in the case of a non-constant tsquery, there's no way for the planner to know about it (and even with the constant case, you'd need a helper function that doesn't exist today). But this behavior is problematic for index-only scans because the planner can't tell whether a query will be lossy or not, and it makes a heck of a lot bigger difference than it used to. 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] PostgreSQL fails to build with 32bit MinGW-w64
On 12/14/2011 03:09 PM, Tom Lane wrote: Andrew Dunstanand...@dunslane.net writes: + # Disable FP optimizations that cause isinf errors on gcc 4.5+ + PGAC_PROG_CC_CFLAGS_OPT([-fexcess-precision=standard]) Looks sane to me, except isinf errors is an awfully narrow reading of the problem. Maybe just say assorted errors? Also, do we know that gcc 4.5 poses the issue? I'm only aware of reports for 4.6 and 4.7. It looked to me like this switch landed in gcc 4.5 because they were getting problems like this. See http://gcc.gnu.org/ml/gcc-patches/2008-11/msg00105.html I guess we should backpatch it? +1. Back branches will see these same problems as soon as anybody tries to compile them with latest-n-greatest gcc. Yeah. Will do. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] review: CHECK FUNCTION statement
Hello
so removed quite option
and removed multiple check regression tests also - there is missing
explicit order of function checking, so regress tests can fail :(
Regards
Pavel
2011/12/14 Albe Laurenz laurenz.a...@wien.gv.at:
Pavel Stehule wrote:
changes:
* fixed warnings
* support for options - actually only two options are supported -
quite and fatal_errors
these options are +/- useful - main reason for their existence is
testing of support of options - processing on CHECK ... stmt side and
processing on checker function side.
options are send as 2d text array - some like
'{{quite,on},{fatal_errors,on}} - so direct call of checker function
is possible
* regress test for multi check
First of all: It should be quiet and not quite.
The patch applies and builds fine.
It fails one of ist own regression tests, here is the diff:
*** /postgres/cvs/postgresql/src/test/regress/expected/plpgsql.out
2011-12-14 11:50:44.0 +0100
--- /postgres/cvs/postgresql/src/test/regress/results/plpgsql.out
2011-12-14 16:19:45.0 +0100
***
*** 4975,4991
end;
$$ language plpgsql;
check function all in schema plpgsql_check;
- NOTICE: checked function plpgsql_check.fce1()
WARNING: error in function plpgsql_check.fce2()
DETAIL: too few parameters specified for RAISE
CONTEXT: line 3 at RAISE
NOTICE: checked function plpgsql_check.fce3()
check function all in schema plpgsql_check with (quite);
WARNING: error in function plpgsql_check.fce2()
DETAIL: too few parameters specified for RAISE
CONTEXT: line 3 at RAISE
check function all in schema plpgsql_check with (fatal_errors);
- NOTICE: checked function plpgsql_check.fce1()
ERROR: too few parameters specified for RAISE
CONTEXT: PL/pgSQL function fce2 line 3 at RAISE
check function all in schema plpgsql_check with (quite, fatal_errors on);
--- 4975,4990
end;
$$ language plpgsql;
check function all in schema plpgsql_check;
WARNING: error in function plpgsql_check.fce2()
DETAIL: too few parameters specified for RAISE
CONTEXT: line 3 at RAISE
NOTICE: checked function plpgsql_check.fce3()
+ NOTICE: checked function plpgsql_check.fce1()
check function all in schema plpgsql_check with (quite);
WARNING: error in function plpgsql_check.fce2()
DETAIL: too few parameters specified for RAISE
CONTEXT: line 3 at RAISE
check function all in schema plpgsql_check with (fatal_errors);
ERROR: too few parameters specified for RAISE
CONTEXT: PL/pgSQL function fce2 line 3 at RAISE
check function all in schema plpgsql_check with (quite, fatal_errors on);
==
The quiet option is not very intuitive:
test= CHECK FUNCTION ALL WITH (quite 'off');
NOTICE: skip check function atrig(), it is trigger function
NOTICE: skip check function perl_max(integer,integer), language plperl
hasn't checker function
NOTICE: checked function ok()
NOTICE: checked function newstyle(integer)
CHECK FUNCTION
test= CHECK FUNCTION ALL WITH (quite 'on');
NOTICE: skip check function atrig(), it is trigger function
CHECK FUNCTION
I understand that quiet cannot silence this message, nor
skip ..., uses C language and skip ..., it uses internal language,
but that means that it is not very useful as it is.
If all we need is a sample option, I think that fatal_errors is
enough, and I think that is an option that can be useful.
Yours,
Laurenz Albe
check_function-2011-12-14-3.diff.gz
Description: GNU Zip compressed data
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unite recovery.conf and postgresql.conf
I've submitted two patches for adding new include features to the postgresql.conf file. While not quite commit quality yet, I hope everyone will agree their reviews this week suggest both are close enough that any number of people could finish them off. Before re-opening this can of worms, I wanted people to be comfortable that we can expect them to be available as building blocks before 9.2 development is finished. Both of those came out of requests from this unification thread, and they're a helpful part of what I'd like to propose. I don't see any path forward here that still expects the recovery.conf file to function as it used to. The make replication easy crowd will eventually be larger than the pre-9.0 user base, if it isn't already. And they clearly want no parts of that thing. There's been over a year of arguing around how to cope with it that will satisfy everyone, so many messages I couldn't even read them all usefully in our archives and had to go here: http://postgresql.1045698.n5.nabble.com/recovery-conf-location-td2854644.html http://postgresql.1045698.n5.nabble.com/unite-recovery-conf-and-postgresql-conf-td4785717.html I don't think it's possible. What I would propose is a specification based on forced failure if there's any sign of recovery.conf, combined with the simplest migration path we can design to ease upgrades from older versions. I think we can make the transition easy enough. Users and tool vendors can make relatively simple changes to support 9.2 without changing everything they're used to just yet--while still being very clear deprecation has arrived and they should reconsider their approach. Only bug-compatible levels of backwards compatibility would make this transparent to them, and there's way too many issues to allow moving forward that way--a forward path that as far as I can see is desired by the majority of users, and just as importantly for all of the potential new users we're impacting with the current mess. There's another, perhaps under considered, concern I want to highlight as well. Tom has repeatedly noted that one of the worst problems here would go away if the existence means do recovery nature of recovery.conf went elsewhere. And we know some packagers want to separate the necessary to manipulate configuration files from the database directory, for permissions and management reasons. As Heikki nicely put it (over a year ago), You don't want to give monitoring tools that decide on failover write access to the data directory. Any information that the user is supplying for the purpose of specifying things needs to be easy to relocate to a separate config file area, instead of treating it more like a control file in $PGDATA. Some chatting this morning with Simon pointed out a second related concern there, which makes ideas like specify the path to the recovery.conf file infeasible. The data_directory is itself a parameter, so anything tied to that or a new GUC means that config files specified there we would need two passes. First identify the data directory, then go back again to read recovery.conf from somewhere else. And nobody wants to wander that way. If it doesn't fit cleanly into the existing postgresql.conf parsing, it's gotta go. Here's the rough outline of what I think would work here: -All settings move into the postgresql.conf. -As of 9.2, relocating the postgresql.conf means there are no user writable files needed in the data directory. -Creating a standby.enabled file in the directory that houses the postgresql.conf (same logic as include uses to locate things) puts the system into recovery mode. That feature needs to save some state, and making those decisions based on existence of a file is already a thing we do. Rather than emulating the rename to recovery.done that happens now, the server can just delete it, to keep from incorrectly returning to a state it's exited. A UI along the lines of the promote one, allowing pg_ctl standby, should fall out of here. I think this is enough that people who just want to use replication features need never hear about this file at all, at least during the important to simplify first run through. -If startup finds a recovery.conf file where it used to live at, abort--someone is expecting the old behavior. Hint to RTFM or include a short migration guide right on the spot. That can have a nice section about how you might use the various postgresql.conf include* features if they want to continue managing those files separately. Example: rename it as replication.conf and use include_if_exists if you want to be able to rename it to recovery.done like before. Or drop it into a conf.d directory where the rename will make it then skipped. -Tools such as pgpool that want to write a simple configuration file, only touching the things that used to go into recovery.conf, can tell people to do the same trick. End their
Re: [HACKERS] Command Triggers
On Wed, Dec 14, 2011 at 9:05 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Dimitri Fontaine's message of mié dic 14 07:22:21 -0300 2011: Again, that's a caveat of the first implementation, you can't have sub commands support without forcing them through ProcessUtility and that's a much more invasive patch. Maybe we will need that later. I can get behind this argument: force all stuff through ProcessUtility for regularity, and not necessarily in the first patch for this feature. That seems like a pretty heavy dependency on an implementation detail that we might want to change at some point. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unite recovery.conf and postgresql.conf
On Wed, Dec 14, 2011 at 4:16 PM, Greg Smith g...@2ndquadrant.com wrote: [ plan for deprecating recovery.conf ] +1. I'd be very happy with this plan. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unite recovery.conf and postgresql.conf
On Wed, Dec 14, 2011 at 22:16, Greg Smith g...@2ndquadrant.com wrote: I've submitted two patches for adding new include features to the postgresql.conf file. While not quite commit quality yet, I hope everyone will agree their reviews this week suggest both are close enough that any number of people could finish them off. Before re-opening this can of worms, I wanted people to be comfortable that we can expect them to be available as building blocks before 9.2 development is finished. Both of those came out of requests from this unification thread, and they're a helpful part of what I'd like to propose. I don't see any path forward here that still expects the recovery.conf file to function as it used to. The make replication easy crowd will eventually be larger than the pre-9.0 user base, if it isn't already. And they clearly want no parts of that thing. There's been over a year of arguing around how to cope with it that will satisfy everyone, so many messages I couldn't even read them all usefully in our archives and had to go here: http://postgresql.1045698.n5.nabble.com/recovery-conf-location-td2854644.html http://postgresql.1045698.n5.nabble.com/unite-recovery-conf-and-postgresql-conf-td4785717.html I don't think it's possible. What I would propose is a specification based on forced failure if there's any sign of recovery.conf, combined with the simplest migration path we can design to ease upgrades from older versions. I think we can make the transition easy enough. Users and tool vendors can make relatively simple changes to support 9.2 without changing everything they're used to just yet--while still being very clear deprecation has arrived and they should reconsider their approach. Only bug-compatible levels of backwards compatibility would make this transparent to them, and there's way too many issues to allow moving forward that way--a forward path that as far as I can see is desired by the majority of users, and just as importantly for all of the potential new users we're impacting with the current mess. There's another, perhaps under considered, concern I want to highlight as well. Tom has repeatedly noted that one of the worst problems here would go away if the existence means do recovery nature of recovery.conf went elsewhere. And we know some packagers want to separate the necessary to manipulate configuration files from the database directory, for permissions and management reasons. As Heikki nicely put it (over a year ago), You don't want to give monitoring tools that decide on failover write access to the data directory. Any information that the user is supplying for the purpose of specifying things needs to be easy to relocate to a separate config file area, instead of treating it more like a control file in $PGDATA. Some chatting this morning with Simon pointed out a second related concern there, which makes ideas like specify the path to the recovery.conf file infeasible. The data_directory is itself a parameter, so anything tied to that or a new GUC means that config files specified there we would need two passes. First identify the data directory, then go back again to read recovery.conf from somewhere else. And nobody wants to wander that way. If it doesn't fit cleanly into the existing postgresql.conf parsing, it's gotta go. Here's the rough outline of what I think would work here: -All settings move into the postgresql.conf. -As of 9.2, relocating the postgresql.conf means there are no user writable files needed in the data directory. -Creating a standby.enabled file in the directory that houses the postgresql.conf (same logic as include uses to locate things) puts the system into recovery mode. That feature needs to save some state, and making those decisions based on existence of a file is already a thing we do. Rather than emulating the rename to recovery.done that happens now, the server can just delete it, to keep from incorrectly returning to a state it's exited. A UI along the lines of the promote one, allowing pg_ctl standby, should fall out of here. I think this is enough that people who just want to use replication features need never hear about this file at all, at least during the important to simplify first run through. You say the server can just delete it. But how will this work if the file is *not* in the data directory? If you are on a Debian style system for example, where all these files go in /etc/postgresql - wouldn't that suddenly require the postgres user to have write access in this directory? If it actually has to be the server that modifies the file, I think it *does* make sense for this file to live in the data directory... [cutting lots of good explanations] Other than that consideration, +1 for this proposal. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list
Re: [HACKERS] unite recovery.conf and postgresql.conf
On 12/14/11 1:16 PM, Greg Smith wrote: -Creating a standby.enabled file in the directory that houses the postgresql.conf (same logic as include uses to locate things) puts the system into recovery mode. That feature needs to save some state, and making those decisions based on existence of a file is already a thing we do. Rather than emulating the rename to recovery.done that happens now, the server can just delete it, to keep from incorrectly returning to a state it's exited. A UI along the lines of the promote one, allowing pg_ctl standby, should fall out of here. I think this is enough that people who just want to use replication features need never hear about this file at all, at least during the important to simplify first run through. How will things work for PITR? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Command Triggers
Robert Haas robertmh...@gmail.com writes: I can get behind this argument: force all stuff through ProcessUtility for regularity, and not necessarily in the first patch for this feature. That seems like a pretty heavy dependency on an implementation detail that we might want to change at some point. Given ProcessUtility_hook, how much of an implementation detail rather than a public API are we talking about? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unite recovery.conf and postgresql.conf
On 12/14/2011 04:47 PM, Magnus Hagander wrote: You say the server can just delete it. But how will this work if the file is *not* in the data directory? If you are on a Debian style system for example, where all these files go in /etc/postgresql - wouldn't that suddenly require the postgres user to have write access in this directory? If it actually has to be the server that modifies the file, I think it *does* make sense for this file to live in the data directory... Perhaps I should have softened the suggestion to relocating the postgresql.conf makes it *possible* to have no user writable files in the data directory. That was one of the later additions I made, it didn't bake overnight before sending like the bulk did. A Debian system might want it to stay in the data directory. If we consider this not normally touched by the user state information--they can poke it by hand, but the preferred way is to use pg_ctl--perhaps it could live in /var/run/postgresql instead. [Surely I'll find out momentarily, now that I've trolled everyone here who is more familiar than me with the rules around what goes into /var] I think the bigger idea I was trying to support in this part is just how many benefits there are from breaking this role into one decoupled from the main server configuration. It's not a new suggestion, but I think it was cut down by backward compatibility concerns before being fully explored. It seems all of the good ways to provide cleaner UIs need that, and it surely gives better flexibility to packagers for it to float free from the config. Who can predict what people will end up doing in their packages. (And the Gentoo changes have proven it's not just Debian) If we drag this conversation back toward the best way to provide that cleaner UI, but can pick up enough agreement that backward compatibility limited to the sort of migration ideas I outlined is acceptable, I'd be happy with that progress. Hopes of reaching that point is the reason I dumped time into those alternative include options. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unite recovery.conf and postgresql.conf
On 12/14/2011 04:57 PM, Josh Berkus wrote: How will things work for PITR? Left that out mainly because I was already running too long there, but I do think there's a reasonable path. There is one additional wrinkle in there to consider that I've thought of so far, falling into the what is the best UI for this? category I think. Put the stuff you used to insert into recovery.conf into postgresql.conf instead. If you don't like that, use another file and include it with one of the multiple options for that--same migration option I already suggested. Run pg_ctl recovery; under the hood that's actually creating standby.enabled instead of recovery.conf, but you don't have to know that. You'd suggested renaming it to reflect its most common usage now, and I thought that was quite sensible. It helps with the things have changed, please drive carefully feel too. It seems possible to have two files for state kickoff/tracking here instead, maybe have recovery.enabled and standby.enabled. Is that extra complexity a useful thing? I haven't dug into that new topic much yet. (Look at that! I think I just found a *new* topic here!) There are some questions around what to do when it's done. The new proposed behavior is to delete the standby.enabled file. But that doesn't remove the changes made for recovery like the old recovery.done rename did. This is why I commented that some more thinking is likely needed about how to handle seeing those only-makes-sense-in-recovery options when not being started for recovery/standby; it's not obvious that any approach will make everyone happy. If you want to do something special yourself to clean that up, there's already recovery_end_command available for that. Let's say you wanted to force the old name and did include_if_exists conf.d/recovery.conf, to trick it even if the patrolling for the name idea goes in. Now you could do: recovery_end_command = 'rm -f /tmp/pgsql.trigger.5432 mv conf.d/recovery.conf conf.d/recovery.done' Like some people are used to and might still prefer for some reason. There'd be time left over to head out to the lawn and yell at the kids there. Actually, this might be the right approach for tools that are trying to change as little as possible but add quick 9.2 compatibility. I think there's enough pluggable bits in every direction here that people can assemble the setup they'd like out of the available parts, Maybe these slightly different semantics between archive recovery and standby mode are exactly why they should be kicked off by differently named files? -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unite recovery.conf and postgresql.conf
Greg, Put the stuff you used to insert into recovery.conf into postgresql.conf instead. If you don't like that, use another file and include it with one of the multiple options for that--same migration option I already suggested. Run pg_ctl recovery; under the hood that's actually creating standby.enabled instead of recovery.conf, but you don't have to know that. You'd suggested renaming it to reflect its most common usage now, and I thought that was quite sensible. It helps with the things have changed, please drive carefully feel too. So for streaming replication, will I need to have a standby.enabled file, or will there be a parameter in postgresql.conf (or included files) which controls whether or not that server is a standby, available? In the best of all possible worlds, I'd really like to have a GUC which 100% controls whether or not the current server is a standby. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Collecting statistics on CSV file data
(2011/12/14 15:34), Shigeru Hanada wrote: (2011/12/13 22:00), Etsuro Fujita wrote: Thank you for your effectiveness experiments and proposals for improvements. I updated the patch according to your proposals. Attached is the updated version of the patch. I think this patch could be marked as Ready for committer with some minor fixes. Please find attached a revised patch (v6.1). Many thanks. Best regards, Etsuro Fujita -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unite recovery.conf and postgresql.conf
On 12/14/2011 08:56 PM, Josh Berkus wrote: So for streaming replication, will I need to have a standby.enabled file, or will there be a parameter in postgresql.conf (or included files) which controls whether or not that server is a standby, available? In the best of all possible worlds, I'd really like to have a GUC which 100% controls whether or not the current server is a standby You keep asking the hard questions. Right now, I kind of like that it's possible to copy a postgresql.conf file from master to standby and just use it. That should still be possible with the realignment into GUCs: -standby_mode = on: Ignored unless you've started the server with standby.enabled, won't bother the master if you include it. -primary_conninfo: This will look funny on the master showing it connecting to itself, but it will get ignored there too. I was hoping to just copy over a base backup, pg_ctl standby creates the needed file and starts the server, and I'm done. Isn't that the easiest replication hello, world possible here? If you think there's an easier way here, please describe it more; I'm missing it so far. Some settings will look a bit weird in the identical postgresql.conf in this case, but it think it can be made to work. Now, eventually you will have to sort this out, but my normal principle here is that any issue deferred until after people have a working system is automatically easier for them to stomach. Yes there's complexity, but people are facing it after the happy dance when the standby works for the first time. The unavoidable bad situation happens if you promote a standby made this way. Replicating more standbys from it won't work; you have to fix primary_conninfo at some point. But once you're the master, you should be able to change primary_conninfo anytime--even if you SIGHUP to reload, it will now be ignored--so sorting that out doesn't even require a server restart. [Problem of how exactly to define a GUC with those properties while also doing the right thing when you are a standby was noted then snuck by quietly] If that is replaced with an edit to the postgresql.conf, that makes the bar for setting up a standby higher in my mind. Now we have every clusterware product forced into the position pgpool already finds itself, where it needs to cope with making at least one change to that file. I can see a middle ground position where you can have the standby.enabled file, but you can also set something in the postgresql.conf, but now we're back to conflict and order resolution madness. [See: which of postgresql.conf and recovery.conf should be read first?] [Crazy talk begins here, but without further abuse of parenthetical brackets] There is a route this way I wouldn't mind wandering down, but it circles back to one of the even bigger debates. I would be perfectly happy fully embracing multiple configuration files for the server by default on every install. Then the things that vary depending on current role can all be put into one place, with some expected splits along this line. Put all the stuff related to standby configuration in one file; then tools can know I can overwrite this whole file and that will be true. There's an obvious objection here that having this crap in two files is the problem we're trying to eliminate! I would still see this as forward, because at the very minimum that split should refactor the replication and recovery target pieces into different files. Different tools will want to feel they own them and can rewrite them, and making that easy should be a major goal. Also, it will be possible to rearrange them if you'd like in whatever order makes sense, which you can't do now for the recovery.conf part. You'd just be breaking tools that might expect the default split doing that; if you don't care, have at it. Wandering any distance down that whole road surely stretches the premise of simple migration procedure using include too far to be true anymore. I was thinking that for 9.2, it seems feasible to get much of this legacy stuff sorted better (from the perspective of the person focused on simple replication), and add some enabling features. No recovery.conf, everything is a GUC, migration path isn't so bad, people get exposed to new concepts for include file organization. I'd like to do some bigger reorganization too, but that seems too much change to shove all into one release. There's a simple path from there that leads to both easier tools all around and SET PERSISTENT, and it comes with a pile of disruption so big I could throw in standby controls are now 100% GUC for you plus a unicorn and it would slip right by unnoticed. That's a tough roadmap to sell unless those promised benefits are proven first though. And I'm thinking a release doing all that is going to want to be named 10.0--and what I could really use is a nice, solid 9.2 that doesn't scare
