Re: [HACKERS] [RFC][PATCH] wal decoding, attempt #2 - Design Documents (really attached)
On 22.09.2012 20:00, Andres Freund wrote:
[[basic-schema]]
.Architecture Schema
[ditaa]
--
Traditional Stuff
+-+-+-+-++
| Backend | Backend | Backend | Autovac | ...|
+++---+-+++++-+--+
|| | | |
+--+ | ++ | |
+-+ | | | ++ |
|| | | | |
|v v v v |
| ++ |
| | WAL writer |--+
| ++
| | | | | |
v v v v v v +---+
++ +-+ +-| Startup/Recovery |
|{s} | |{s} | | +---+
|Catalog | | WAL |---+-| SR/Hot Standby|
|| | | | +---+
++ +-+ +-| Point in Time |
^ |+---+
---|--|
| New Stuff
+---+ |
| vRunning separately
| ++ +=-+
| | Walsender | | | |
| |v | |+---+ |
| +-+ | | +-| Logical Rep. | |
| | WAL | | | | +---+ |
+-| decoding | | | +-| Multimaster | |
| +--+--/ | | | +---+ |
| || | | +-| Slony | |
| |v | | | +---+ |
| +-+ | | +-| Auditing | |
| | TX | | | | +---+ |
+-| reassembly | | | +-| Mysql/... | |
| +-/ | | | +---+ |
| || | | +-| Custom Solutions | |
| |v | | | +---+ |
| +-+ | | +-| Debugging | |
| | Output| | | | +---+ |
+-| Plugin|--|--|-+-| Data Recovery | |
+-/ | |+---+ |
|| | |
++ +--|
--
This diagram triggers a pet-peeve of mine: What do all the boxes and
lines mean? An architecture diagram should always include a key. I find
that when I am drawing a diagram myself, adding the key clarifies my own
thinking too.
This looks like a data-flow diagram, where the arrows indicate the data
flows between components, and the boxes seem to represent processes. But
in that case, I think the arrows pointing from the plugins in walsender
to Catalog are backwards. The catalog information flows from the Catalog
to walsender, walsender does not write to the catalogs.
Zooming out to look at the big picture, I think the elephant in the room
with this whole effort is how it fares against trigger-based
replication. You list a number of disadvantages that trigger-based
solutions have, compared to the proposed logical replication. Let's take
a closer look at them:
* essentially duplicates the amount of writes (or even more!)
True.
* synchronous replication hard or impossible to implement
I don't see any explanation it could be implemented in the proposed
logical replication either.
* noticeable CPU overhead
* trigger functions
* text conversion of data
Well, I'm pretty sure we could find some micro-optimizations for these
if we put in the effort. And the proposed code isn't exactly free, either.
* complex parts implemented in several solutions
Not sure what this means, but the proposed code is quite complex too.
* not in core
IMHO that's a good thing, and I'd hope this new logical replication to
live outside core as well, as much as possible. But whether or not
something is in core is just a political decision, not a reason to
implement something new.
If the only meaningful advantage is reducing the amount of WAL written,
I can't help thinking that we should just try to address that in the
existing solutions, even if it seems easy to solve at a first glance,
but a solution not using a normal transactional table for its log/queue
has to solve a lot of problems, as the document says. Sorry to be a
naysayer, but I'm pretty scared of all the new code and complexity these
patches bring into core.
PS. I'd love to see a basic Slony plugin for this, for example, to see
how much extra code on top of the posted patches you need to write in a
plugin like that to make it functional. I'm worried that it's a lot..
- Heikki
--
Sent 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 8/8] Introduce wal decoding via catalog timetravel
On 10/11/2012 04:31 AM, Tom Lane wrote: Greg Stark st...@mit.edu writes: On Thu, Oct 11, 2012 at 2:40 AM, Tom Lane t...@sss.pgh.pa.us wrote: Isn't there an even more serious problem, namely that this assumes *all* transactions are serializable? What happens when they aren't? Or even just that the effective commit order is not XID order? I don't think it assumes the transactions are serializable because it's only concerned with writes, not reads. So the transaction it's replaying may or may not have been able to view the data written by other transactions that commited earlier but it doesn't matter when trying to reproduce the effects using constants. I would believe that argument if the apply operations were at a similar logical level to our current WAL records, namely drop these bits into that spot. Unfortunately, they're not. I think this argument falls to the ground entirely as soon as you think about DDL being applied by transactions A,B,C and then needing to express what concurrent transactions X,Y,Z did in source terms. Even something as simple as a few column renames could break it, let alone anything as esoteric as changing the meaning of datatype literals. This is the whole reason of moving the reassembly to the source side and having the possibility to use old snapshots to get the catalog information. Also, the locks that protect you from effects of field name changes by DDL concurrent transactions protect also the logical reassembly if done in the commit order. 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 8/8] Introduce wal decoding via catalog timetravel
On 10/11/2012 03:10 AM, Robert Haas wrote: On Wed, Oct 10, 2012 at 7:02 PM, Peter Geoghegan pe...@2ndquadrant.com wrote: The purpose of ApplyCache/transaction reassembly is to reassemble interlaced records, and organise them by XID, so that the consumer client code sees only streams (well, lists) of records split by XID. I think I've mentioned it before, but in the interest of not being seen to critique the bikeshed only after it's been painted: this design gives up something very important that exists in our current built-in replication solution, namely pipelining. The lack of pipelining (and the following complexity of applycache and spilling to disk) is something we have discussed with Andres and to my understanding it is not a final design decision but just stepping stones in how this quite large development is structured. The pipelining (or parallel apply as I described it) requires either a large number of apply backends and code to manage them or autonomous transactions. It could (arguably !) be easier to implement autonomous transactions instead of apply cache, but Andres had valid reasons to start with apply cache and move to parallel apply later . As I understand it the parallel apply is definitely one of the things that will be coming and after that the performance characteristics (fast AND smooth) will be very similar to current physical WAL streaming. With streaming replication as it exists today, a transaction that modifies a huge amount of data (such as a bulk load) can be applied on the standby as it happens. The rows thus inserted will become visible only if and when the transaction commits on the master and the commit record is replayed on the standby. This has a number of important advantages, perhaps most importantly that the lag between commit and data visibility remains short. With the proposed system, we can't start applying the changes until the transaction has committed and the commit record has been replayed, so a big transaction is going to have a lot of apply latency. Now, I am not 100% opposed to a design that surrenders this property in exchange for other important benefits, but I think it would be worth thinking about whether there is any way that we can design this that either avoids giving that property up at all, or gives it up for the time being but allows us to potentially get back to it in a later version. Reassembling complete transactions is surely cool and some clients will want that, but being able to apply replicated transactions *without* reassembling them in their entirety is even cooler, and some clients will want that, too. If we're going to stick with a design that reassembles transactions, I think there are a number of issues that deserve careful thought. First, memory usage. I don't think it's acceptable for the decoding process to assume that it can allocate enough backend-private memory to store all of the in-flight changes (either as WAL or in some more decoded form). We have assiduously avoided such assumptions thus far; you can write a terabyte of data in one transaction with just a gigabyte of shared buffers if you so desire (and if you're patient). Here's you making the same point in different words: Applycache is presumably where you're going to want to spill transaction streams to disk, eventually. That seems like a prerequisite to commit. Second, crash recovery. I think whatever we put in place here has to be able to survive a crash on any node. Decoding must be able to restart successfully after a system crash, and it has to be able to apply exactly the set of transactions that were committed but not applied prior to the crash. Maybe an appropriate mechanism for this already exists or has been discussed, but I haven't seen it go by; sorry if I have missed the boat. You consider this to be a throw-away function that won't ever be committed. However, I strongly feel that you should move it into /contrib, so that it can serve as a sort of reference implementation for authors of decoder client code, in the same spirit as numerous existing contrib modules (think contrib/spi). Without prejudice to the rest of this review which looks quite well-considered, I'd like to add a particular +1 to this point. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] change in LOCK behavior
On 11 October 2012 01:43, Tom Lane t...@sss.pgh.pa.us wrote: I think we have to revert and go back to the drawing board on this. Given that change was also sold on the basis of higher performance, I suggest we retest performance to check there is a gain. If there is still a gain, I suggest we add this as a SIGHUP option, default to off, rather than completely remove it. I might also observe since the problem only happens with lock waits, perhaps we can set a flag can_reuse_snapshot that gets cleared if we need to perform a lock wait before executing the main statement. -- 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] FDW for PostgreSQL
Hi Hanada-san, Please examine attached v2 patch (note that is should be applied onto latest dblink_fdw_validator patch). I've reviewed your patch quickly. I noticed that the patch has been created in a slightly different way from the guidelines: http://www.postgresql.org/docs/devel/static/fdw-planning.html The guidelines say the following, but the patch identifies the clauses in baserel-baserestrictinfo in GetForeignRelSize, not GetForeignPaths. Also, it has been implemented so that most sub_expressions are evaluated at the remote end, not the local end, though I'm missing something. For postgresql_fdw to be a good reference for FDW developers, ISTM it would be better that it be consistent with the guidelines. I think it would be needed to update the following document or redesign the function to be consistent with the following document. As an example, the FDW might identify some restriction clauses of the form foreign_variable= sub_expression, which it determines can be executed on the remote server given the locally-evaluated value of the sub_expression. The actual identification of such a clause should happen during GetForeignPaths, since it would affect the cost estimate for the path. 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] replace plugins directory with GUC
Heikki Linnakangas hlinnakan...@vmware.com writes: Now that we support include-directories in postgresql.conf, you could put a mylib.conf file in the include directory that contains the above line, if you want to enable/disable a module just by moving things around in the filesystem (after configuring an include directory in postgresql.conf). But actually, you can't, because there's no way to append to a setting, you can only override. That's an obvious missing feature in the include mechanism. Even ignoring the plugins directory, it would be nice to be able to append libraries to shared_preload_libraries. I think we need a += operator in the configuration file, complementing the = we have now, and defined so that when the setting is not yet defined it's a bare =. And I think this plugins thing needs to be revisited, yes. 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] Truncate if exists
On 10/09/2012 10:04 PM, Robert Haas wrote: - if a table is not yet or no more visible, because of search_path modification I don't think I understand the case you are describing here. Here's a sample : begin; set search_path = foo, public; create table c ( … ) ; commit; begin; set search_path = bar, public; create table d ( … ); truncate if exists c; commit; - if a table was dropped, for any reason But in this case surely you could use DROP IF EXISTS. Well, TRUNCATE and DROP TABLE are not the same, I don't see your point ? I've been a big proponent of adding IF EXISTS support to CREATE TABLE and ALTER TABLE but I'm having a hard time getting excited about this one. I can't imagine that many people would use it, and those who do can implement it in about 10 lines of PL/pgsql. The existence of DO blocks and the fact that PL/pgsql is now installed by default have made it much more convenient to solve these kinds of problems using those tools rather than needing dedicated syntax. That does not mean that the most frequently used cases shouldn't have dedicated syntax anyway, for convenience, but I'm doubtful that this falls into that category. I don't think we can ask people to do DO blocks for TRUNCATE, when they simply use IF EXISTS with DROP TABLE. Even if TRUNCATE is not a DDL, it's often use as is -- Sébastien Lardière PostgreSQL DBA Team Manager Hi-Media -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Truncate if exists
On 10/09/2012 04:06 PM, Tom Lane wrote: Second, to my mind the point of a multi-table TRUNCATE is to ensure that all the referenced tables get reset to empty *together*. With something like this, you'd have no such guarantee. Consider a timeline like this: Session 1 Session 2 TRUNCATE IF EXISTS a, b, c; ... finds c doesn't exist ... ... working on a and b ... CREATE TABLE c ( ... ); INSERT INTO c ...; ... commits ... Now we have a, b, and c, but c isn't empty, violating the expectations of session 1. So even if there's a use-case for IF EXISTS on a single table, I think it's very very dubious to allow it in multi-table commands. Hi, I've to say that I don't understand your timeline : - If c exist in Session 1, CREATE TABLE in Session 2 can't be done, neither INSERT - If c doesn't exists in Session 1, it will be ignored, then, Session 2 work fine. In any case, TRUNCATE is sent before INSERT, but it can't lock an object which still not exists. I understand that people don't want TRUNCATE IF EXISTS, but, in my point of view, even if TRUNCATE is not a DDL, it's the same use-case as DROP TABLE IF EXISTS. Regards, -- Sébastien Lardière PostgreSQL DBA Team Manager Hi-Media -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] BUG #7534: walreceiver takes long time to detect n/w breakdown
On Wednesday, October 10, 2012 9:15 PM Heikki Linnakangas wrote: On 04.10.2012 13:12, Amit kapila wrote: Following changes are done to support replication timeout in sender as well as receiver: 1. One new configuration parameter wal_receiver_timeout is added to detect timeout at receiver task. 2. Existing parameter replication_timeout is renamed to wal_sender_timeout. Ok. The other option would be to have just one GUC, I'm open to bikeshedding on this one. On one hand, there's no reason the timeouts have to the same, so it would be nice to have separate settings, but on the other hand, I can't imagine a case where a single setting wouldn't work just as well. I think for below case, they are required to be separate: 1. M1 (Master), S1 (Standby 1), S2 (Standby 2) 2. S1 is standby for M1, and S2 is standby for S1. Basically a simple case of cascaded replication 3. M1 and S1 are on local network but S2 is placed at geographically different location. (what I want to say is n/w between M1-S1 is of good speed and S1-S2 is very slow) 4. In above case, user might want to configure different timeouts for sender and receiver on S1. Attached is an updated patch. I reverted the merging of message types and fixed a bunch of cosmetic issues. There was one bug: in the main loop of walreceiver, you send the ping message on every wakeup after enough time has passed since last reception. That means that if the server doesn't reply promptly, you send a new ping message every 100 ms (NAPTIME_PER_CYCLE), until it gets a reply. Walsender had the same issue, but it was not quite as sever there because the naptime was longer. Fixed that. Thanks. How does this look now? The Patch is fine and test results are also fine. With Regards, Amit Kapila. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tuplesort memory usage: grow_memtuples
Do you intend to follow through with this, Jeff? -- 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] Incorrect behaviour when using a GiST index on points
On Tue, Oct 02, 2012 at 01:58:40PM -0400, Noah Misch wrote: On Mon, Aug 27, 2012 at 7:43 PM, Tom Lane t...@sss.pgh.pa.us wrote: There's also the big-picture question of whether we should just get rid of fuzzy comparisons in the geometric types instead of trying to hack indexes to work around them. In any event, I think we should entertain a patch to make the GiST operator class methods bug-compatible with corresponding operators. Even if we decide to change operator behavior in HEAD, the back branches could use it. We have broad agreement that the specific implementation of fuzz in geometric comparison operators is shoddy, but nobody has voiced interest in designing a concrete improvement. I propose adding a TODO item Remove or improve rounding in geometric comparison operators, endorsing Alexander's design, and reviewing his patch. Objections? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Is there a good reason why PL languages do not support cstring type arguments and return values ?
2012/10/11 Dimitri Fontaine dimi...@2ndquadrant.fr: Heikki Linnakangas hlinnakan...@vmware.com writes: I've wanted to allow writing i/o functions in non-C languages for a long time as well, but never got around to do anything about it. Custom datatypes are really powerful, but as soon as you have to write C code, that raises the bar significantly. I/O functions written in, say, PL/pgSQL would be an order of magnitude slower than ones written in C, but for many applications it would be OK. Do you want a crazy idea now? Yes, I do mean Yet Another One. I'm thinking about what it would take to have a new PL/C language where the backend would actually compile and link/load the C code at CREATE FUNCTION time, using dynamic code generation techniques. That would allow writing functions in C and not have to ship a binary executable file on the system, which would solve a bunch of problems. With that tool and this use case, you could simply ship inline your C coded IO functions in the middle of the PL/pythonu extension, using the exact same mechanisms. In the more general view of our offerings, that would fix C coded extensions for Hot Standby, for one thing. long time I am thinking about it. I would to use our embedded C - but replace libpq by SPI. Other idea - compile PL/pgSQL to C - and ... Regards Pavel 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] September 2012 commitfest
On Wed, Oct 10, 2012 at 12:19:17PM -0300, Alvaro Herrera wrote: Many of those patches waiting on authors have been in such state for a rather long time. I feel inclined to mark them returned with feedback, and have them posted again for the next commitfest. +1 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC][PATCH] wal decoding, attempt #2 - Design Documents (really attached)
On Thursday, October 11, 2012 09:15:47 AM Heikki Linnakangas wrote:
On 22.09.2012 20:00, Andres Freund wrote:
[[basic-schema]]
.Architecture Schema
[ditaa]
-
-
Traditional Stuff
+-+-+-+-++
| Backend | Backend | Backend | Autovac | ...|
+++---+-+++++-+--+
+--+ | ++ | |
+-+ | | | ++ |
|v v v v |
|
| ++ |
|
| | WAL writer |--+
|
| ++
v v v v v v +---+
++ +-+ +-| Startup/Recovery |
|{s} | |{s} | | +---+
|Catalog | | WAL |---+-| SR/Hot Standby|
|
|| | | | +---+
++ +-+ +-| Point in Time |
^ |+---+
---|--|
| New Stuff
+---+ |
| vRunning separately
|
| ++ +=-+
|
| | Walsender | | | |
| |
| |v | |+---+ |
|
| +-+ | | +-| Logical Rep. | |
|
| | WAL | | | | +---+ |
+-| decoding | | | +-| Multimaster | |
| +--+--/ | | | +---+ |
|
| || | | +-| Slony | |
| |
| |v | | | +---+ |
|
| +-+ | | +-| Auditing | |
|
| | TX | | | | +---+ |
+-| reassembly | | | +-| Mysql/... | |
| +-/ | | | +---+ |
|
| || | | +-| Custom Solutions | |
| |
| |v | | | +---+ |
|
| +-+ | | +-| Debugging | |
|
| | Output| | | | +---+ |
+-| Plugin|--|--|-+-| Data Recovery | |
+-/ | |+---+ |
++ +--|
-
-
This diagram triggers a pet-peeve of mine: What do all the boxes and
lines mean? An architecture diagram should always include a key. I find
that when I am drawing a diagram myself, adding the key clarifies my own
thinking too.
Hm. Ok.
This looks like a data-flow diagram, where the arrows indicate the data
flows between components, and the boxes seem to represent processes. But
in that case, I think the arrows pointing from the plugins in walsender
to Catalog are backwards. The catalog information flows from the Catalog
to walsender, walsender does not write to the catalogs.
The reason I drew it that way is that it actively needs to go back to the
catalog and query it which is somewhat different of the rest which basically
could be seen as a unidirectional pipeline.
Zooming out to look at the big picture, I think the elephant in the room
with this whole effort is how it fares against trigger-based
replication. You list a number of disadvantages that trigger-based
solutions have, compared to the proposed logical replication. Let's take
a closer look at them:
* essentially duplicates the amount of writes (or even more!)
True.
By now I think its essentially unfixable.
* synchronous replication hard or impossible to implement
I don't see any explanation it could be implemented in the proposed
logical replication either.
Its basically the same as its for synchronous streaming repl. At the place
where SyncRepWaitForLSN() is done you instead/also wait for the decoding to
reach that lsn (its in the wal, so everything is decodable) and for the other
side to have confirmed reception of those changes. I think this should be
doable with only minor code modifications.
The existing support for all that is basically the reason we want to reuse the
walsender framework. (will start a thread about that soon)
* noticeable CPU overhead
* trigger functions
* text conversion of data
Well, I'm pretty sure we could find some micro-optimizations for these
if we put in the effort.
Any improvements there are a good idea independent from this proposal but I
don't see how we can fundamentally improve from the status quo.
And the proposed code isn't exactly free, either.
If you don't have frequent DDL its really not all that expensive. In the
version without DDL support I didn't manage to saturate the ApplyCache with
either parallel COPY in
[HACKERS] Windows help needed for flex and bison
The flex and bison make rules refactoring I just did broke the Windows build. I think the fixes should look like the patch below. Could someone please verify and/or commit that? diff --git a/src/tools/msvc/pgbison.pl b/src/tools/msvc/pgbison.pl index d6f2444..15db921 100644 --- a/src/tools/msvc/pgbison.pl +++ b/src/tools/msvc/pgbison.pl @@ -42,7 +42,7 @@ local $/ = undef; $make = $mf; close($mf); -my $headerflag = ($make =~ /\$\(BISON\)\s+-d/ ? '-d' : ''); +my $headerflag = ($make =~ /^$output: BISONFLAGS\b.*-d/ ? '-d' : ''); system(bison $headerflag $input -o $output); exit $? 8; diff --git a/src/tools/msvc/pgflex.pl b/src/tools/msvc/pgflex.pl index 259f218..29dbc8e 100644 --- a/src/tools/msvc/pgflex.pl +++ b/src/tools/msvc/pgflex.pl @@ -44,7 +44,7 @@ local $/ = undef; $make = $mf; close($mf); -my $flexflags = ($make =~ /^\s*FLEXFLAGS\s*=\s*(\S.*)/m ? $1 : ''); +my $flexflags = ($make =~ /^$output:\s*FLEXFLAGS\s*=\s*(\S.*)/m ? $1 : ''); system(flex $flexflags -o$output $input); if ($? == 0) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] embedded list
Alvaro Herrera escribió: I also included two new functions in that patch, dlisti_push_head and dlisti_push_tail. These functions are identical to dlist_push_head and dlist_push_tail, except that they do not accept non-circular lists. What this means is that callers that find the non-circularity acceptable can use the regular version, and will run dlist_init() on demand; callers that want the super-tight code can use the other version. I didn't bother with a new dlist_is_empty. Is there any more input on this? At this point I would recommend committing this patch _without_ these dlisti functions, i.e. we will only have the functions that check for NULL-initialized dlists. We can later discuss whether to include them or not (it would be a much smaller patch and would not affect the existing functionality in any way.) -- Álvaro Herrerahttp://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] embedded list
On Thursday, October 11, 2012 03:23:12 PM Alvaro Herrera wrote: Alvaro Herrera escribió: I also included two new functions in that patch, dlisti_push_head and dlisti_push_tail. These functions are identical to dlist_push_head and dlist_push_tail, except that they do not accept non-circular lists. What this means is that callers that find the non-circularity acceptable can use the regular version, and will run dlist_init() on demand; callers that want the super-tight code can use the other version. I didn't bother with a new dlist_is_empty. Is there any more input on this? At this point I would recommend committing this patch _without_ these dlisti functions, i.e. we will only have the functions that check for NULL-initialized dlists. We can later discuss whether to include them or not (it would be a much smaller patch and would not affect the existing functionality in any way.) I can live with that. I didn't have a chance to look at the newest revision yet, will do that after I finish my first pass through foreign key locks. Andres -- Andres Freund 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] [BUGS] BUG #7534: walreceiver takes long time to detect n/w breakdown
On 11.10.2012 13:17, Amit Kapila wrote: How does this look now? The Patch is fine and test results are also fine. Ok, thanks. Committed. - Heikki -- Sent 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
On Friday, August 31, 2012 06:59:51 AM Alvaro Herrera wrote: v21 is merged to latest master. Ok, I am starting to look at this. (working with a git merge of alvherre/fklocks into todays master) In a very first pass as somebody who hasn't followed the discussions in the past I took notice of the following things: * compiles and survives make check * README.tuplock jumps in quite fast without any introduction * the reasons for the redesign aren't really included in the patch but just in the - very nice - pgcon paper. * This is a bit of a performance regression, but since we expect FOR SHARE locks to be seldom used, we don’t feel this is a serious problem. makes me a bit uneasy, will look at performance separately * Should RelationGetIndexattrBitmap check whether a unique index is referenced from a pg_constraint row? Currently we pay part of the overhead independent from the existance of foreign keys. * mode == LockTupleKeyShare, == LockTupleShare, == LockTupleUpdate in heap_lock_tuple's BeingUpdated branch look like they should be an if/else if * I don't really like HEAP_UPDATE_KEY_REVOKED as a name, what about HEAP_KEYS_UPDATED (akin to HEAP_HOT_UPDATED) * how can we get infomask2 HEAP_UPDATE_KEY_REVOKED infomask HEAP_XMAX_LOCK_ONLY? * heap_lock_tuple with mode == LockTupleKeyShare nowait looks like it would wait anyway in heap_lock_updated_tuple_rec * rename HeapSatisfiesHOTUpdate, adjust comments? * I wonder whether HeapSatisfiesHOTUpdate could be made to compute the result for key_attrs and hot_attrs at the same time, often enough they will do the same thing on the same column * you didn't start it but I find that all those l$i jump labels make the code harder to follow * shouldn't XmaxGetUpdateXid be rather called MultiXactIdGetUpdateXid or such? * /* * If the existing lock mode is identical to or weaker than the new * one, we can act as though there is no existing lock and have the * upper block handle this. */ block? * I am not yet sure whether the (xmax == add_to_xmax) optimization in compute_new_xmax_infomask is actually safe * ConditionalMultiXactIdWait and MultiXactIdWait should probably rely on a common implementation * I am not that convinced that moving the waiting functions away from multixact.c is a good idea, but I guess the required knowledge about lockmodes makes it hard not to do so * Haven't looked yet, but I have a slightly uneasy feeling about Hot Standby interactions. Did you look at that? * Hm? HeapTupleSatisfiesVacuum #if 0 ResetMultiHintBit(tuple, buffer, xmax, true); #endif This obviously is not a real review, but just learning what the problem patch actually try to do. This is quite a bit to take in ;). I will let it sink in ant try to have a look at the architecture and performance next. Greetings, Andres .oO(and people call catalog timetravel complicated) -- Andres Freund 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
[HACKERS] Re: [BUGS] BUG #7534: walreceiver takes long time to detect n/w breakdown
On Thursday, October 11, 2012 8:22 PM Heikki Linnakangas wrote: On 11.10.2012 13:17, Amit Kapila wrote: How does this look now? The Patch is fine and test results are also fine. Ok, thanks. Committed. Thank you very much. With Regards, Amit Kapila. -- Sent 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
Andres Freund wrote:
On Friday, August 31, 2012 06:59:51 AM Alvaro Herrera wrote:
v21 is merged to latest master.
Ok, I am starting to look at this.
(working with a git merge of alvherre/fklocks into todays master)
In a very first pass as somebody who hasn't followed the discussions in the
past I took notice of the following things:
* compiles and survives make check
Please verify src/test/isolation's make installcheck as well.
* README.tuplock jumps in quite fast without any introduction
Hmm .. I'll give that a second look. Maybe some material from the pgcon
paper should be added as introduction.
* This is a bit of a performance regression, but since we expect FOR SHARE
locks to be seldom used, we don’t feel this is a serious problem. makes me a
bit uneasy, will look at performance separately
Thanks. Keep in mind though that the bits we really want good
performance for is FOR KEY SHARE, which is going to be used in foreign
keys. FOR SHARE is staying just for hypothetical backwards
compatibility. I just found that people is using it, see for example
http://stackoverflow.com/a/3243083
* Should RelationGetIndexattrBitmap check whether a unique index is
referenced
from a pg_constraint row? Currently we pay part of the overhead independent
from the existance of foreign keys.
Noah also suggested something more or less in the along the same lines.
My answer is that I don't want to do it currently; maybe we can improve
on what indexes we use later, but since this can be seen as a modularity
violation, I prefer to keep it as simple as possible.
* mode == LockTupleKeyShare, == LockTupleShare, == LockTupleUpdate in
heap_lock_tuple's BeingUpdated branch look like they should be an if/else if
Hm, will look at that.
* I don't really like HEAP_UPDATE_KEY_REVOKED as a name, what about
HEAP_KEYS_UPDATED (akin to HEAP_HOT_UPDATED)
Thanks.
* how can we get infomask2 HEAP_UPDATE_KEY_REVOKED infomask
HEAP_XMAX_LOCK_ONLY?
SELECT FOR KEY UPDATE? This didn't exist initially, but previous
reviewers (Noah) felt that it made sense to have it for consistency. It
makes the lock conflict table make more sense, anyway.
* heap_lock_tuple with mode == LockTupleKeyShare nowait looks like it
would
wait anyway in heap_lock_updated_tuple_rec
Oops.
* rename HeapSatisfiesHOTUpdate, adjust comments?
Yeah.
* I wonder whether HeapSatisfiesHOTUpdate could be made to compute the result
for key_attrs and hot_attrs at the same time, often enough they will do the
same thing on the same column
Hmm, I will look at that.
* you didn't start it but I find that all those l$i jump labels make the code
harder to follow
You mean you suggest to have them renamed? That can be arranged.
* shouldn't XmaxGetUpdateXid be rather called MultiXactIdGetUpdateXid or
such?
Yes.
/*
* If the existing lock mode is identical to or weaker than the
new
* one, we can act as though there is no existing lock and have
the
* upper block handle this.
*/
block?
Yeah, as in the if {} block above. Since this is not clear maybe it
needs rewording.
* I am not yet sure whether the (xmax == add_to_xmax) optimization in
compute_new_xmax_infomask is actually safe
Will think about it and add a/some comment(s).
* ConditionalMultiXactIdWait and MultiXactIdWait should probably rely on a
common implementation
Will look at that.
* I am not that convinced that moving the waiting functions away from
multixact.c is a good idea, but I guess the required knowledge about
lockmodes
makes it hard not to do so
Yeah. The line between multixact.c and heapam.c is a zigzag currently,
I think. Maybe it needs to be more clear; I think the multixact modes
really belong into heapam.c, and they are just uint32 to multixact.c. I
wasn't brave enough to attempt this; maybe I should.
* Haven't looked yet, but I have a slightly uneasy feeling about Hot Standby
interactions. Did you look at that?
* Hm?
HeapTupleSatisfiesVacuum
#if 0
ResetMultiHintBit(tuple, buffer, xmax, true);
#endif
Yeah. I had a couple of ResetMultiHintBit calls sprinkled in some of
the tqual routines, with the idea that if they saw multis that were no
longer live they could be removed, and replaced by just the remaining
updater. However, this doesn't really work because it's not safe to
change the Xmax when not holding exclusive lock, and the tqual routines
don't know that. So we're stuck with keeping the multi in there, even
when we know they are no longer interesting. This is a bit sad, because
it would be a performance benefit to remove them; but it's not strictly
necessary for correctness, so we can leave the optimization for a later
phase. I let those #ifdefed out calls in place so that it's easy to see
where the reset needs to happen.
This obviously is not a real review, but just learning what the
Re: [HACKERS] embedded list
On Thursday, October 11, 2012 03:27:17 PM Andres Freund wrote: On Thursday, October 11, 2012 03:23:12 PM Alvaro Herrera wrote: Alvaro Herrera escribió: I also included two new functions in that patch, dlisti_push_head and dlisti_push_tail. These functions are identical to dlist_push_head and dlist_push_tail, except that they do not accept non-circular lists. What this means is that callers that find the non-circularity acceptable can use the regular version, and will run dlist_init() on demand; callers that want the super-tight code can use the other version. I didn't bother with a new dlist_is_empty. Is there any more input on this? At this point I would recommend committing this patch _without_ these dlisti functions, i.e. we will only have the functions that check for NULL-initialized dlists. We can later discuss whether to include them or not (it would be a much smaller patch and would not affect the existing functionality in any way.) I can live with that. I didn't have a chance to look at the newest revision yet, will do that after I finish my first pass through foreign key locks. I looked at and I am happy enough ;) One thing: I think you forgot to adjust dlist_reverse_foreach to the NULL list header. Thanks! Andres -- Andres Freund 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] Windows help needed for flex and bison
On 10/11/2012 09:05 AM, Peter Eisentraut wrote: The flex and bison make rules refactoring I just did broke the Windows build. I think the fixes should look like the patch below. Could someone please verify and/or commit that? Close, but not quite. I have made it work and committed 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] enhanced error fields
On 10 October 2012 14:56, Pavel Stehule pavel.steh...@gmail.com wrote:
(eelog3.diff)
This looks better.
You need a better comment here:
+ * relerror.c
+ * relation error loging functions
+ *
I'm still not satisfied with the lack of firm guarantees about what
errcodes one can assume these fields will be available for. I suggest
that this be explicitly documented within errcodes.h. For example,
right now if some client code wants to discriminate against a certain
check constraint in its error-handling code (typically to provide a
user-level message), it might do something like this:
try
{
...
}
catch(CheckViolation e)
{
// This works:
if (e.constraint_name == postive_balance)
MessageBox(Your account must have a positive balance.);
// This is based on a check constraint in a domain, and is
therefore broken right now:
else if (e.constraint_name == valid_barcode)
MessageBox(You inputted an invalid barcode - check digit was wrong);
}
This is broken right now, simply because the application cannot rely
on the constraint name being available, since for no particular reason
some of the ERRCODE_CHECK_VIOLATION ereport sites (like in execQual.c)
don't provide an errconstraint(). What is needed is a coding standard
that says ERRCODE_CHECK_VIOLATION ereport call sites need to have an
errconstraint(). Without this, the patch is quite pointless.
My mind is not 100% closed to the idea that we provide these extra
fields on a best-effort basis per errcode, but it's pretty close to
there. Why should we allow this unreasonable inconsistency? The usage
pattern that I describe above is a real one, and I thought that the
whole point was to support it.
I have previously outlined places where this type of inconsistency
exists, in an earlier revision. [1]
If you want to phase in the introduction of requiring that all
relevant callsites use this infrastructure, I guess I'm okay with
that. However, I'm going to have to insist that for each of these new
fields, for any errcode you identify as requiring the field, either
all callsites get all relevant fields, or no call sites get all
relevant fields, and that each errcode be documented as such. So you
should probably just bite the bullet and figure out a reasonable and
comprehensive set of rules on adding these fields based on errcode.
Loosey goosey isn't going to cut it.
I'm having a difficult time imagining why we'd only have the
constraint/tablename for these errcodes (with one exception, noted
below):
/* Class 23 - Integrity Constraint Violation */
#define ERRCODE_INTEGRITY_CONSTRAINT_VIOLATION
MAKE_SQLSTATE('2','3','0','0','0')
#define ERRCODE_RESTRICT_VIOLATION MAKE_SQLSTATE('2','3','0','0','1')
#define ERRCODE_NOT_NULL_VIOLATION MAKE_SQLSTATE('2','3','5','0','2')
#define ERRCODE_FOREIGN_KEY_VIOLATION MAKE_SQLSTATE('2','3','5','0','3')
#define ERRCODE_UNIQUE_VIOLATION MAKE_SQLSTATE('2','3','5','0','5')
#define ERRCODE_CHECK_VIOLATION MAKE_SQLSTATE('2','3','5','1','4')
#define ERRCODE_EXCLUSION_VIOLATION MAKE_SQLSTATE('2','3','P','0','1')
You previously defending some omissions [2] on the basis that they
involved domains, so some fields were unavailable. This doesn't appear
to be quite valid, though. For example, consider this untouched
callsite within execQual, that relates to a domain:
if (!conIsNull
!DatumGetBool(conResult))
ereport(ERROR,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg(value
for domain %s violates check constraint \%s\,
format_type_be(ctest-resulttype),
con-name)));
There is no reason why you couldn't have at least given the constraint
name. It might represent an unreasonable burden for you to determine
the table that these constraints relate to by going through the rabbit
hole of executor state, since we haven't had any complaints about this
information being available within error messages before, AFAIK. If
that is the case, the general non-availability of this information for
domains needs to be documented. I guess that's logical enough, since
there doesn't necessarily have to be a table involved in the event of
a domain constraint violation. However, there does have to be a
constraint, for example, involved.
FWIW, I happen to think that not-null constraints at the domain level
are kind of stupid (though check constraints are great), but what do I
know...
Anyway, the bottom line is that authors of Postgres client libraries
(and their users) ought to have a reasonable set of guarantees about
when this information is available. If that means you have to make one
or two explicit,
Re: [HACKERS] change in LOCK behavior
On 11 October 2012 17:53, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: On 11 October 2012 01:43, Tom Lane t...@sss.pgh.pa.us wrote: I think we have to revert and go back to the drawing board on this. Given that change was also sold on the basis of higher performance, I suggest we retest performance to check there is a gain. If there is still a gain, I suggest we add this as a SIGHUP option, default to off, rather than completely remove it. I'm not in favor of adding a GUC for this. The right fix is to redesign the locking/snapshotting process, not expose its warts in bizarre little knobs that make users deal with the tradeoffs. While I agree with that thought, I'd like to try a little harder than simply revert. Maybe what we really need is to find a way to make taking a snapshot a lot cheaper, such that the whole need for this patch goes away. We're not going to get far with the idea of making SnapshotNow MVCC-safe unless it becomes a lot cheaper to get an MVCC snapshot. I recall some discussion of trying to reduce a snapshot to a WAL offset --- did that idea crash and burn, or is it still viable? I think that is still at the fond wish stage and definitely not backpatchable in this universe. Anyway, I believe that for now we ought to revert and rethink, not look for band-aid ways of preserving this patch. I suggested a way to automatically trigger a second snapshot. I think that would be acceptable to backpatch. But since I'm leaving this to you, I'll leave that decision to you as well. -- 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] change in LOCK behavior
Simon Riggs si...@2ndquadrant.com writes: On 11 October 2012 17:53, Tom Lane t...@sss.pgh.pa.us wrote: Maybe what we really need is to find a way to make taking a snapshot a lot cheaper, such that the whole need for this patch goes away. We're not going to get far with the idea of making SnapshotNow MVCC-safe unless it becomes a lot cheaper to get an MVCC snapshot. I recall some discussion of trying to reduce a snapshot to a WAL offset --- did that idea crash and burn, or is it still viable? I think that is still at the fond wish stage and definitely not backpatchable in this universe. Of course not. This performance improvement is simply lost for 9.2. We can try to do better in future releases, but it's a failed experiment for now. Life is hard :-( I suggested a way to automatically trigger a second snapshot. I think that would be acceptable to backpatch. If it worked, I might be amenable to that, but it doesn't. You can't trigger taking a new snapshot off whether we waited for a lock; that still has race conditions, just ones that are not so trivial to demonstrate manually. (The other transaction might have committed microseconds before you reach the point of waiting for the lock.) It would have to be a rule like take a new snapshot if we acquired any new lock since the previous snapshot. While that would work, we'd end up with no performance gain worth mentioning, since there would almost always be some lock acquisitions during parsing. 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] Truncate if exists
For starters, the use-case hasn't been explained to my satisfaction. In what situation is it actually helpful to TRUNCATE a table that's not there yet? Aren't you going to have to do a CREATE IF NOT EXISTS to keep from failing later in the script? If so, why not just do that first? There is a use case for the truncate 'mutliple' tables, maybe less clear for a single table. Sébastien will speak here I suppose. Second, to my mind the point of a multi-table TRUNCATE is to ensure that all the referenced tables get reset to empty *together*. With something like this, you'd have no such guarantee. Consider a timeline like this: Session 1 Session 2 TRUNCATE IF EXISTS a, b, c; ... finds c doesn't exist ... ... working on a and b ... CREATE TABLE c ( ... ); INSERT INTO c ...; ... commits ... Now we have a, b, and c, but c isn't empty, violating the expectations of session 1. So even if there's a use-case for IF EXISTS on a single table, I think it's very very dubious to allow it in multi-table commands. well, in such case you probably don't want to use IF EXISTS. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation
Re: [HACKERS] September 2012 commitfest
On Wed, Oct 10, 2012 at 6:17 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Tom Lane wrote: Alvaro Herrera alvhe...@2ndquadrant.com writes: IIRC, the parallel pg_dump one is said to need review by a Windows expert, which is not me, so I've not looked at it. Andrew? Magnus? There's, unfortunately, not a chance I'll have a time to look at any of that until after pgconf.eu. Sorry. -- 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] change in LOCK behavior
On 11 October 2012 18:22, Tom Lane t...@sss.pgh.pa.us wrote: I suggested a way to automatically trigger a second snapshot. I think that would be acceptable to backpatch. If it worked, I might be amenable to that, but it doesn't. You can't trigger taking a new snapshot off whether we waited for a lock; that still has race conditions, just ones that are not so trivial to demonstrate manually. (The other transaction might have committed microseconds before you reach the point of waiting for the lock.) It would have to be a rule like take a new snapshot if we acquired any new lock since the previous snapshot. While that would work, we'd end up with no performance gain worth mentioning, since there would almost always be some lock acquisitions during parsing. So where's the race? AFAICS it either waits or it doesn't - the code isn't vague on that point. If we wait we set the flag. The point is that lock waits are pretty rare since most locks are compatible, so triggering a second snap if we waited is not any kind of problem, even if we waited for a very short time. -- 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] change in LOCK behavior
On Thu, Oct 11, 2012 at 2:23 PM, Simon Riggs si...@2ndquadrant.com wrote: So where's the race? AFAICS it either waits or it doesn't - the code isn't vague on that point. If we wait we set the flag. The point is that lock waits are pretty rare since most locks are compatible, so triggering a second snap if we waited is not any kind of problem, even if we waited for a very short time. That actually wouldn't fix the problem, because we might have this scenario: 1. We take a snapshot. 2. Some other process commits, clearing its XID from its PGPROC and releasing locks. 3. We take a lock. :-( -- 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] change in LOCK behavior
Simon Riggs si...@2ndquadrant.com writes: On 11 October 2012 18:22, Tom Lane t...@sss.pgh.pa.us wrote: If it worked, I might be amenable to that, but it doesn't. You can't trigger taking a new snapshot off whether we waited for a lock; that still has race conditions, just ones that are not so trivial to demonstrate manually. (The other transaction might have committed microseconds before you reach the point of waiting for the lock.) So where's the race? Same example as before, except that the exclusive-lock-holding transaction commits (and releases its lock) between the time that the other transaction takes its parse/plan snapshot and the time that it takes AccessShare lock on the table. 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] September 2012 commitfest
On 10/11/2012 02:22 PM, Magnus Hagander wrote: On Wed, Oct 10, 2012 at 6:17 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Tom Lane wrote: Alvaro Herrera alvhe...@2ndquadrant.com writes: IIRC, the parallel pg_dump one is said to need review by a Windows expert, which is not me, so I've not looked at it. Andrew? Magnus? There's, unfortunately, not a chance I'll have a time to look at any of that until after pgconf.eu. Sorry. I have a quietish few days starting on Saturday, will be looking at this then. Is it only the Windows aspect that needs reviewing? Are we more or less happy with the rest? 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] change in LOCK behavior
On 11 October 2012 19:36, Robert Haas robertmh...@gmail.com wrote: On Thu, Oct 11, 2012 at 2:23 PM, Simon Riggs si...@2ndquadrant.com wrote: So where's the race? AFAICS it either waits or it doesn't - the code isn't vague on that point. If we wait we set the flag. The point is that lock waits are pretty rare since most locks are compatible, so triggering a second snap if we waited is not any kind of problem, even if we waited for a very short time. That actually wouldn't fix the problem, because we might have this scenario: 1. We take a snapshot. 2. Some other process commits, clearing its XID from its PGPROC and releasing locks. 3. We take a lock. Hmm, so now the patch author thinks his patch is not just broken with respect to lock waits, but in all cases? Surely the above race condition is obvious, now and before. Why is it suddenly unacceptable? (If you believe that, why on earth did you commit?) Whenever you take a snapshot things can change before you start using it. And as a result all previous releases of Postgres suffer this problem. Ergo, we should defer taking the snapshot until the very last point when we need to use it. Why is that *not* being suggested here? -- 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] change in LOCK behavior
On 11 October 2012 19:41, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: On 11 October 2012 18:22, Tom Lane t...@sss.pgh.pa.us wrote: If it worked, I might be amenable to that, but it doesn't. You can't trigger taking a new snapshot off whether we waited for a lock; that still has race conditions, just ones that are not so trivial to demonstrate manually. (The other transaction might have committed microseconds before you reach the point of waiting for the lock.) So where's the race? Same example as before, except that the exclusive-lock-holding transaction commits (and releases its lock) between the time that the other transaction takes its parse/plan snapshot and the time that it takes AccessShare lock on the table. A cache invalidation could also set the need-second-snapshot flag. -- 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] Truncate if exists
On Wed, Oct 10, 2012 at 3:32 AM, Simon Riggs si...@2ndquadrant.com wrote: 2) Clearly, rollout scripts benefit from not throwing errors. Personally I would prefer setting SET ddl_abort_on_missing_object = false; at the top of a script than having to go through every SQL statement and add extra syntax. That might even help people more than littering SQL with extra clauses. I've been thinking about this a bit more. It seems to me that the awkwardness here has a lot to do with the fact that the IF EXISTS is attached to the command rather than sitting outside it. We're basically trying to put the control logic inside the command itself, whereas probably what we really want is for the control logic to be able to exist around the command, like this: IF TABLE foo EXISTS THEN TRUNCATE TABLE foo; END IF But of course that doesn't work. I think you have to write something like this: do $$ begin if (select 1 from pg_class where relname = 'foo' and pg_table_is_visible(oid)) then truncate table foo; end if; end $$; That is a lot more typing and it's not exactly intuitive. One obvious thing that would help is a function pg_table_exists(text) that would return true or false. But even with that there's a lot of syntactic sugar in there that is less than ideal: begin/end, dollar-quoting, do. Whatever becomes of this particular patch, I think we'd make a lot of people really happy if we could find a way to dispense with some of that stuff in simple cases. -- 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] Making the planner more tolerant of implicit/explicit casts
I looked into the complaint in bug #7598, http://archives.postgresql.org/pgsql-bugs/2012-10/msg00090.php The core of the problem is in an inner sub-select that's written like outercol IN (SELECT varcharcol FROM ... WHERE varcharcol = anothercol ... The = operator is actually texteq, since varchar has no equality operator of its own, which means there's a RelabelType in there. Likewise, the comparison expression generated for the IN construct involves a RelabelType on varcharcol. Now, ruleutils.c prefers to make the cast explicit, so this prints as outercol IN (SELECT varcharcol FROM ... WHERE varcharcol::text = anothercol ... which is semantically the same thing ... but after dump and reload, the RelabelType in the inner WHERE now has CoercionForm COERCE_EXPLICIT_CAST instead of COERCE_IMPLICIT_CAST. And it turns out that that causes process_equivalence to not match it up with the varcharcol instance in the IN expression, so the planner fails to make as many equivalence deductions as it should, resulting in an inferior plan. Basically the thing to do about this is to ensure that we consistently use CoercionForm COERCE_DONTCARE in expressions that are getting fed to the EquivalenceClass machinery. That doesn't change the semantics of the expression tree, but it ensures that equivalent expressions will be seen as equal(). The minimum-risk way to do that would be for canonicalize_ec_expression to copy the presented expression and then apply set_coercionform_dontcare to it. However, the requirement to copy is a bit annoying. Also, we've seen bugs of this general ilk multiple times before, so I'm not entirely satisfied with just hacking EquivalenceClasses for it. What I'm thinking about is modifying eval_const_expressions so that one of its responsibilities is to force CoercionForm fields to COERCE_DONTCARE in the output; which would take basically no additional code, for instance the fix for RelabelType looks like *** a/src/backend/optimizer/util/clauses.c --- b/src/backend/optimizer/util/clauses.c *** eval_const_expressions_mutator(Node *nod *** 2669,2675 newrelabel-resulttype = relabel-resulttype; newrelabel-resulttypmod = relabel-resulttypmod; newrelabel-resultcollid = relabel-resultcollid; ! newrelabel-relabelformat = relabel-relabelformat; newrelabel-location = relabel-location; return (Node *) newrelabel; } --- 2669,2675 newrelabel-resulttype = relabel-resulttype; newrelabel-resulttypmod = relabel-resulttypmod; newrelabel-resultcollid = relabel-resultcollid; ! newrelabel-relabelformat = COERCE_DONTCARE; newrelabel-location = relabel-location; return (Node *) newrelabel; } The net effect of this is that *all* CoercionForms seen in the planner would be COERCE_DONTCARE. We could get rid of set_coercionform_dontcare altogether, and also get rid of the ugly special-case comparison logic in equalfuncs.c. This is a more invasive fix, for sure, but it would permanently prevent the whole class of bugs instead of just stomping the manifestation in process_equivalence. Thoughts? 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] Truncate if exists
On 11 October 2012 19:59, Robert Haas robertmh...@gmail.com wrote:
On Wed, Oct 10, 2012 at 3:32 AM, Simon Riggs si...@2ndquadrant.com wrote:
2) Clearly, rollout scripts benefit from not throwing errors.
Personally I would prefer setting SET ddl_abort_on_missing_object =
false; at the top of a script than having to go through every SQL
statement and add extra syntax. That might even help people more than
littering SQL with extra clauses.
I've been thinking about this a bit more. It seems to me that the
awkwardness here has a lot to do with the fact that the IF EXISTS is
attached to the command rather than sitting outside it. We're
basically trying to put the control logic inside the command itself,
whereas probably what we really want is for the control logic to be
able to exist around the command, like this:
IF TABLE foo EXISTS THEN
TRUNCATE TABLE foo;
END IF
But of course that doesn't work. I think you have to write something like
this:
do $$
begin
if (select 1 from pg_class where relname = 'foo' and
pg_table_is_visible(oid)) then
truncate table foo;
end if;
end
$$;
That is a lot more typing and it's not exactly intuitive. One obvious
thing that would help is a function pg_table_exists(text) that would
return true or false. But even with that there's a lot of syntactic
sugar in there that is less than ideal: begin/end, dollar-quoting, do.
Whatever becomes of this particular patch, I think we'd make a lot of
people really happy if we could find a way to dispense with some of
that stuff in simple cases.
Yeh, definitely.
So we just need a function called pg_if_table_exists(table, SQL) which
wraps a test in a subtransaction.
And you write
SELECT pg_if_table_exists('foo', 'TRUNCATE TABLE foo');
and we can even get rid of all that other DDL crud that's been added
and we can have pg_if_table_not_exists() also.
--
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] change in LOCK behavior
On Thu, Oct 11, 2012 at 2:48 PM, Simon Riggs si...@2ndquadrant.com wrote: Hmm, so now the patch author thinks his patch is not just broken with respect to lock waits, but in all cases? Surely the above race condition is obvious, now and before. Why is it suddenly unacceptable? (If you believe that, why on earth did you commit?) Whenever you take a snapshot things can change before you start using it. And as a result all previous releases of Postgres suffer this problem. I agree with this. I think that the reversion that Tom is pushing for is fixing a very special case of a more general problem. Now, it might be a sufficiently important special-case that we ought to go and do the revert, but if your argument is that this is an unsatisfying band-aid, I couldn't agree more. It appears to me that this discussion is exposing the fact that we really haven't given much thought to when snapshots ought to be taken or to arranging the order of operations so that they are taken as late as is safely possible. Ergo, we should defer taking the snapshot until the very last point when we need to use it. Why is that *not* being suggested here? Well, that's actually not safe either, at least if taken literally. For example, you can't do part of a sequential scan without a snapshot just because all the tuples are frozen, and then take a snapshot when you hit a unfrozen xmin/xmax. Both you and I previously came up with that idea independently, and it would save a lot of cycles, but it's unsafe. Some other backend can update a tuple after you look at it, and then commit. When you get to the new tuple, you'll already have returned the old tuple, and your new snapshot - since it's taken after the commit - will also see the new tuple. So we have to take the snapshot before you begin execution, but it seems that to avoid surprising behavior we also have to take it after acquiring locks. And it looks like locking is intertwined with a bunch of other parse analysis tasks that might require a snapshot to have been taken first. Whee. -- 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] Deprecating RULES
Not many RULE-lovers out there, once you've tried to use them. Allowing RULEs complicates various things and can make security more difficult. For 9.3, I suggest we create a DDL trigger by default which prevents RULEs and throws an ERROR that explains they are now deprecated. Anybody that really cares can delete this and use them. Sometime in future, we hard code it, barring complaints. Comments? -- 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] September 2012 commitfest
On Thu, Oct 11, 2012 at 2:42 PM, Andrew Dunstan and...@dunslane.net wrote: I have a quietish few days starting on Saturday, will be looking at this then. Is it only the Windows aspect that needs reviewing? Are we more or less happy with the rest? I think the Windows issues were the biggest thing, but I suspect there may be a few other warts as well. It's a lot of code, and it's modifying pg_dump, which is an absolute guarantee that it's built on a foundation made out of pure horse manure. -- 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] change in LOCK behavior
On 11 October 2012 20:25, Robert Haas robertmh...@gmail.com wrote: On Thu, Oct 11, 2012 at 2:48 PM, Simon Riggs si...@2ndquadrant.com wrote: Hmm, so now the patch author thinks his patch is not just broken with respect to lock waits, but in all cases? Surely the above race condition is obvious, now and before. Why is it suddenly unacceptable? (If you believe that, why on earth did you commit?) Whenever you take a snapshot things can change before you start using it. And as a result all previous releases of Postgres suffer this problem. I agree with this. I think that the reversion that Tom is pushing for is fixing a very special case of a more general problem. Now, it might be a sufficiently important special-case that we ought to go and do the revert, but if your argument is that this is an unsatisfying band-aid, I couldn't agree more. It appears to me that this discussion is exposing the fact that we really haven't given much thought to when snapshots ought to be taken or to arranging the order of operations so that they are taken as late as is safely possible. Ergo, we should defer taking the snapshot until the very last point when we need to use it. Why is that *not* being suggested here? Well, that's actually not safe either, at least if taken literally. Right - I didn't mean that far - since it was me suggested deferring snapshots previously I'm aware that doesn't work. But immediately before we read the first heap row... since we might need to wait on index access, heap I/O etc. -- 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] September 2012 commitfest
On 11 October 2012 20:30, Robert Haas robertmh...@gmail.com wrote: On Thu, Oct 11, 2012 at 2:42 PM, Andrew Dunstan and...@dunslane.net wrote: I have a quietish few days starting on Saturday, will be looking at this then. Is it only the Windows aspect that needs reviewing? Are we more or less happy with the rest? I think the Windows issues were the biggest thing, but I suspect there may be a few other warts as well. It's a lot of code, and it's modifying pg_dump, which is an absolute guarantee that it's built on a foundation made out of pure horse manure. That may be so, but enough people dependent upon it that now I'm wondering whether we should be looking to create a new utility altogether, or at least have pg_dump_parallel and pg_dump to avoid any screw ups with people's backups/restores. -- 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] change in LOCK behavior
Robert Haas robertmh...@gmail.com writes: So we have to take the snapshot before you begin execution, but it seems that to avoid surprising behavior we also have to take it after acquiring locks. And it looks like locking is intertwined with a bunch of other parse analysis tasks that might require a snapshot to have been taken first. Whee. Yeah. I think that a good solution to this would involve guaranteeing that the execution snapshot is not taken until we have all locks that are going to be taken on the tables. Which is likely to involve a fair amount of refactoring, though I admit I've not looked at details. In any case, it's a mistake to think about this in isolation. If we're going to do something about redefining SnapshotNow to avoid its race conditions, that's going to move the goalposts quite a lot. Anyway, my feeling about it is that I don't want 9.2 to have an intermediate behavior between the historical one and whatever we end up designing to satisfy these concerns. That's why I'm pressing for reversion and not a band-aid fix in 9.2. I certainly hope we can do better going forward, but this is not looking like whatever we come up with would be sane to back-patch. 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] change in LOCK behavior
On 11 October 2012 20:43, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: So we have to take the snapshot before you begin execution, but it seems that to avoid surprising behavior we also have to take it after acquiring locks. And it looks like locking is intertwined with a bunch of other parse analysis tasks that might require a snapshot to have been taken first. Whee. Yeah. I think that a good solution to this would involve guaranteeing that the execution snapshot is not taken until we have all locks that are going to be taken on the tables. Which is likely to involve a fair amount of refactoring, though I admit I've not looked at details. In any case, it's a mistake to think about this in isolation. If we're going to do something about redefining SnapshotNow to avoid its race conditions, that's going to move the goalposts quite a lot. Anyway, my feeling about it is that I don't want 9.2 to have an intermediate behavior between the historical one and whatever we end up designing to satisfy these concerns. That's why I'm pressing for reversion and not a band-aid fix in 9.2. I certainly hope we can do better going forward, but this is not looking like whatever we come up with would be sane to back-patch. Agreed, please revert. -- 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] Deprecating RULES
Simon Riggs si...@2ndquadrant.com writes: Not many RULE-lovers out there, once you've tried to use them. Allowing RULEs complicates various things and can make security more difficult. For 9.3, I suggest we create a DDL trigger by default which prevents RULEs and throws an ERROR that explains they are now deprecated. This is utter nonsense. We can't deprecate them until we have a substitute that is better. If you want to get rid of rules, build the replacement; don't just try to be a pain in the ass to users. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] WAL_DEBUG logs spurious data
Hi,
I stumbled across a minor issue in xlog.c:1030: the WAL_DEBUG code block
there passes rdata-data to the rm_desc() methode. However, that's only
the first XLogRecData struct, not the entire XLog record. So the
rm_desc() method effectively reports spurious data for any subsequent part.
Take a commit record with subxacts as an example: during XLogInsert,
Postgres reports the following:
LOG: INSERT @ 0/16F3270: prev 0/16F3234; xid 688; len 16: Transaction -
commit: 2012-10-11 09:31:17.790368-07; subxacts: 3214563816
Note that the xid in subxacts is way off. During recovery from WAL, the
record is logged correctly:
LOG: REDO @ 0/16F3270; LSN 0/16F329C: prev 0/16F3234; xid 688; len 16:
Transaction - commit: 2012-10-11 09:31:17.790368-07; subxacts: 689
Attached is a possible fix.
Regards
Markus Wanner
*** a/src/backend/access/transam/xlog.c
--- b/src/backend/access/transam/xlog.c
***
*** 1033,1039 begin:;
#ifdef WAL_DEBUG
if (XLOG_DEBUG)
{
! StringInfoData buf;
initStringInfo(buf);
appendStringInfo(buf, INSERT @ %X/%X: ,
--- 1033,1056
#ifdef WAL_DEBUG
if (XLOG_DEBUG)
{
! StringInfoData buf;
! char *full_rec = palloc(write_len), *ins_ptr;
!
! /*
! * We need assemble the entire record once, to be able to dump it out
! * properly.
! */
! rdt = rdata;
! ins_ptr = full_rec;
! while (rdt)
! {
! if (rdt-data != NULL)
! {
! memcpy(ins_ptr, rdt-data, rdt-len);
! ins_ptr += rdt-len;
! }
! rdt = rdt-next;
! }
initStringInfo(buf);
appendStringInfo(buf, INSERT @ %X/%X: ,
***
*** 1042,1051 begin:;
if (rdata-data != NULL)
{
appendStringInfo(buf, - );
! RmgrTable[rechdr-xl_rmid].rm_desc(buf, rechdr-xl_info, rdata-data);
}
elog(LOG, %s, buf.data);
pfree(buf.data);
}
#endif
--- 1059,1069
if (rdata-data != NULL)
{
appendStringInfo(buf, - );
! RmgrTable[rechdr-xl_rmid].rm_desc(buf, rechdr-xl_info, full_rec);
}
elog(LOG, %s, buf.data);
pfree(buf.data);
+ pfree(full_rec);
}
#endif
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecating RULES
Tom Lane t...@sss.pgh.pa.us writes: This is utter nonsense. We can't deprecate them until we have a substitute that is better. If you want to get rid of rules, build the replacement; don't just try to be a pain in the ass to users. My understanding is that the main reason why RULEs are bad™ is that they will not replace a SQL query, but possibly (or is it always) generate multiple queries out of it, then run all of them one after the other. Some programming languages offer much the same user level facility as what we call RULEs, only without the hazardous behavior we all came to hate, except for those few who actually understand it, who will then only hate them for more detailed reasons. That other facility is called an advice and allows users to attach code to run each time a function/method/command is called, either before, after or around the main call. In the around case, a syntactic facility is given that will explicitly call the adviced function/method/command, and you also have ways to change the arguments, then of course tweak the result. E.g an INSTEAD TRIGGER on a VIEW would be equivalent to an AROUND ADVICE on that view, where the user code would not use the aforementioned facility to still run the main command. 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] Making the planner more tolerant of implicit/explicit casts
I wrote: What I'm thinking about is modifying eval_const_expressions so that one of its responsibilities is to force CoercionForm fields to COERCE_DONTCARE in the output; I fooled around with that approach for awhile and didn't like the results, mainly because it caused EXPLAIN output to change in unpleasant ways (ruleutils.c needs the CoercionForm info to format its output nicely). However, on further reflection I realized that we could fix it just by making equal() ignore CoercionForm fields altogether. I remember having considered that and shied away from it back when I first invented the COERCE_DONTCARE hack, on the grounds that it would put too much semantic awareness into equal(). However, we've long since abandoned the idea that equal() should insist on full bitwise equality of nodes --- it's ignored location fields for some time without ill effects, and there are a number of other special cases in there too. So as long as we're willing to consider that equal() can mean just semantic equivalence of two node trees, this can be fixed by removing code rather than adding it, along the lines of the attached patch. We could take the further step of removing the COERCE_DONTCARE enum value altogether; the remaining uses are only to fill in CoercionForm fields in nodes that the planner creates out of whole cloth, and now we could make it fill in one of the more standard values instead. I didn't do that in the attached because it makes the patch longer but no more enlightening (and in any case I don't think that change would be good to back-patch). I'm reasonably convinced that this is a good fix for HEAD, but am of two minds whether to back-patch it or not. The problem complained of in bug #7598 may seem a bit narrow, but the real point is that whether you write a cast explicitly or not shouldn't affect planning if the semantics are the same. This might well be a significant though previously unrecognized performance issue, particularly for people who use varchar columns heavily. Thoughts? regards, tom lane diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 226b99a1d271d425fdc394dd3d3016661cecabf5..95a95f477bead7aee3b484c01f3802a6b81efc3b 100644 *** a/src/backend/nodes/equalfuncs.c --- b/src/backend/nodes/equalfuncs.c *** *** 83,88 --- 83,92 #define COMPARE_LOCATION_FIELD(fldname) \ ((void) 0) + /* Compare a CoercionForm field (also a no-op, per comment in primnodes.h) */ + #define COMPARE_COERCIONFORM_FIELD(fldname) \ + ((void) 0) + /* * Stuff from primnodes.h *** _equalFuncExpr(const FuncExpr *a, const *** 235,250 COMPARE_SCALAR_FIELD(funcid); COMPARE_SCALAR_FIELD(funcresulttype); COMPARE_SCALAR_FIELD(funcretset); ! ! /* ! * Special-case COERCE_DONTCARE, so that planner can build coercion nodes ! * that are equal() to both explicit and implicit coercions. ! */ ! if (a-funcformat != b-funcformat ! a-funcformat != COERCE_DONTCARE ! b-funcformat != COERCE_DONTCARE) ! return false; ! COMPARE_SCALAR_FIELD(funccollid); COMPARE_SCALAR_FIELD(inputcollid); COMPARE_NODE_FIELD(args); --- 239,245 COMPARE_SCALAR_FIELD(funcid); COMPARE_SCALAR_FIELD(funcresulttype); COMPARE_SCALAR_FIELD(funcretset); ! COMPARE_COERCIONFORM_FIELD(funcformat); COMPARE_SCALAR_FIELD(funccollid); COMPARE_SCALAR_FIELD(inputcollid); COMPARE_NODE_FIELD(args); *** _equalRelabelType(const RelabelType *a, *** 448,463 COMPARE_SCALAR_FIELD(resulttype); COMPARE_SCALAR_FIELD(resulttypmod); COMPARE_SCALAR_FIELD(resultcollid); ! ! /* ! * Special-case COERCE_DONTCARE, so that planner can build coercion nodes ! * that are equal() to both explicit and implicit coercions. ! */ ! if (a-relabelformat != b-relabelformat ! a-relabelformat != COERCE_DONTCARE ! b-relabelformat != COERCE_DONTCARE) ! return false; ! COMPARE_LOCATION_FIELD(location); return true; --- 443,449 COMPARE_SCALAR_FIELD(resulttype); COMPARE_SCALAR_FIELD(resulttypmod); COMPARE_SCALAR_FIELD(resultcollid); ! COMPARE_COERCIONFORM_FIELD(relabelformat); COMPARE_LOCATION_FIELD(location); return true; *** _equalCoerceViaIO(const CoerceViaIO *a, *** 469,484 COMPARE_NODE_FIELD(arg); COMPARE_SCALAR_FIELD(resulttype); COMPARE_SCALAR_FIELD(resultcollid); ! ! /* ! * Special-case COERCE_DONTCARE, so that planner can build coercion nodes ! * that are equal() to both explicit and implicit coercions. ! */ ! if (a-coerceformat != b-coerceformat ! a-coerceformat != COERCE_DONTCARE ! b-coerceformat != COERCE_DONTCARE) ! return false; ! COMPARE_LOCATION_FIELD(location); return true; --- 455,461 COMPARE_NODE_FIELD(arg); COMPARE_SCALAR_FIELD(resulttype); COMPARE_SCALAR_FIELD(resultcollid); ! COMPARE_COERCIONFORM_FIELD(coerceformat); COMPARE_LOCATION_FIELD(location); return
Re: [HACKERS] WAL_DEBUG logs spurious data
Markus Wanner mar...@bluegap.ch writes: I stumbled across a minor issue in xlog.c:1030: the WAL_DEBUG code block there passes rdata-data to the rm_desc() methode. However, that's only the first XLogRecData struct, not the entire XLog record. So the rm_desc() method effectively reports spurious data for any subsequent part. The original design intention was that rm_desc should not attempt to print any such data, but obviously some folks didn't get the word. The question is whether we're willing to add a lot of cycles to XLOG_DEBUG mode in order to make the full record available for printing purposes. Not sure if it's a good tradeoff or not. 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] Measure Theoretic Data Types in Postgresql
On 10/10/12 9:37 PM, Aaron Sheldon wrote: I have begun sketching these ideas in off the shelf pgSQL using composite types and constructor functions; but am far from tackling anything like building external C extensions to handle the data types. I can set-up a GitHub repository if anyone is interested in seeing where I am taking this. Please do. I can't even picture it right now, and I think I ought to be in the target audience for this feature. -- 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] Deprecating RULES
On 11 October 2012 20:50, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: Not many RULE-lovers out there, once you've tried to use them. Allowing RULEs complicates various things and can make security more difficult. For 9.3, I suggest we create a DDL trigger by default which prevents RULEs and throws an ERROR that explains they are now deprecated. This is utter nonsense. We can't deprecate them until we have a substitute that is better. We do, they're called views and triggers, both of which are SQL Standard. If you want to get rid of rules, build the replacement; don't just try to be a pain in the ass to users. Supporting broken and non-standard features *is* a pain in the ass to users, since they are sometimes persuaded to use them and then regret it. Or if they do, hit later problems. You recently rejected a partitioning related patch because it used rules... Anyway, lets start with a discussion of what rules give us that SQL standard features do not? -- 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 8/8] Introduce wal decoding via catalog timetravel
On 10/10/12 7:26 PM, Bruce Momjian wrote: How does Slony write its changes without causing serialization replay conflicts? Since nobody from the Slony team answered this: a) Slony replicates *rows*, not *statements* b) Slony uses serializable mode internally for row replication c) it's possible (though difficult) for creative usage to get Slony into a deadlock situation FWIW, I have always assumed that is is impossible (even theoretically) to have statement-based replication without some constraints on the statements you can run, or some replication failures. I think we should expect 9.3's logical replication out-the-gate to have some issues and impose constraints on users, and improve with time but never be perfect. The design Andres and Simon have advanced already eliminates a lot of the common failure cases (now(), random(), nextval()) suffered by pgPool and similar tools. But remember, this feature doesn't have to be *perfect*, it just has to be *better* than the alternatives. -- 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] Deprecating RULES
On Thu, Oct 11, 2012 at 6:25 PM, Simon Riggs si...@2ndquadrant.com wrote: Anyway, lets start with a discussion of what rules give us that SQL standard features do not? The somewhat broader question that this elicits is How would we go about deprecating a feature that seems to be troublesome? I think Josh Berkus describes this downthread in a reasonable way. There are pretty cool things you can do with rules, but they don't seem to scale very successfully to important/massive usage. I tried them out several times, and it was pretty cool that they worked as well as they did, but it sure didn't go to production. I'd be saddened if rules went away because they seemed pretty cool, but it wouldn't be practical disappointment. -- When confronted by a difficult problem, solve it by reducing it to the question, How would the Lone Ranger handle this? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecating RULES
On 11 October 2012 23:28, Josh Berkus j...@agliodbs.com wrote: For 9.3, I suggest we create a DDL trigger by default which prevents RULEs and throws an ERROR that explains they are now deprecated. Well, even if we were considering this, the sequence would need to be: 1. Announce in 9.3 that RULES will be going away RSN. 2. In 9.4, send a warning every time someone loads/edits a user-defined RULE. 3. In 10.0, get rid of CREATE RULE. With the DDL trigger, we're able to do that faster. The idea is you can still delete it if you need compatibility, so we get the message across without an extra release and without an annoying GUC (etc). -- 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] WAL_DEBUG logs spurious data
Tom, On 10/11/2012 03:11 PM, Tom Lane wrote: The original design intention was that rm_desc should not attempt to print any such data, but obviously some folks didn't get the word. FWIW: in case the source code contains comments explaining that intention, I certainly missed them so far. Regards Markus -- Sent 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 8/8] Introduce wal decoding via catalog timetravel
On Wed, Oct 10, 2012 at 10:26 PM, Bruce Momjian br...@momjian.us wrote: How does Slony write its changes without causing serialization replay conflicts? It uses a sequence to break any ordering conflicts at the time that data is inserted into its log tables. If there are two transactions, A and B, that were fighting over a tuple on the origin, then either: a) A went first, B went second, and the ordering in the log makes that order clear; b) A succeeds, then B fails, so there's no conflict; c) A is doing its thing, and B is blocked behind it for a while, then A fails, and B gets to go through, and there's no conflict. Switch A and B as needed. The sequence that is used establishes what is termed a compatible ordering. There are multiple possible compatible orderings; ours happen to interleave transactions together, with the sequence guaranteeing absence of conflict. If we could get commit orderings, then a different but still compatible ordering would be to have each transaction establish its own internal sequence, and apply things in order based on (commit_tx_order, sequence_within). -- When confronted by a difficult problem, solve it by reducing it to the question, How would the Lone Ranger handle this? -- Sent 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 8/8] Introduce wal decoding via catalog timetravel
On 11 October 2012 03:16, Greg Stark st...@mit.edu wrote: On Thu, Oct 11, 2012 at 2:40 AM, Tom Lane t...@sss.pgh.pa.us wrote: I think I've mentioned it before, but in the interest of not being seen to critique the bikeshed only after it's been painted: this design gives up something very important that exists in our current built-in replication solution, namely pipelining. Isn't there an even more serious problem, namely that this assumes *all* transactions are serializable? What happens when they aren't? Or even just that the effective commit order is not XID order? Firstly, I haven't read the code but I'm confident it doesn't make the elementary error of assuming commit order == xid order. I assume it's applying the reassembled transactions in commit order. I don't think it assumes the transactions are serializable because it's only concerned with writes, not reads. So the transaction it's replaying may or may not have been able to view the data written by other transactions that commited earlier but it doesn't matter when trying to reproduce the effects using constants. The data written by this transaction is either written or not when the commit happens and it's all written or not at that time. Even in non-serializable mode updates take row locks and nobody can see the data or modify it until the transaction commits. This uses Commit Serializability, which is valid, as you say. -- 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] Deprecating RULES
On Thu, Oct 11, 2012 at 3:39 PM, Simon Riggs si...@2ndquadrant.com wrote: On 11 October 2012 23:28, Josh Berkus j...@agliodbs.com wrote: For 9.3, I suggest we create a DDL trigger by default which prevents RULEs and throws an ERROR that explains they are now deprecated. Well, even if we were considering this, the sequence would need to be: 1. Announce in 9.3 that RULES will be going away RSN. 2. In 9.4, send a warning every time someone loads/edits a user-defined RULE. 3. In 10.0, get rid of CREATE RULE. With the DDL trigger, we're able to do that faster. The idea is you can still delete it if you need compatibility, so we get the message across without an extra release and without an annoying GUC (etc). This seems sane to me. The deprecation of standard-conforming strings was very smooth in our experience, because it seems like practically every client has adjusted in that long deprecation period. By contrast, the sudden bytea format change was not nearly so smooth -- it bit a lot of people and we have had to change our configuration to override Postgres's default (which we are loathe to do) to the old encoding as our platform-default when vending databases until we can age out all older libpqs, which is going to take quite some time. So, apparently, griping continuously in the logs does get the job done. -- fdr -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecating RULES
For 9.3, I suggest we create a DDL trigger by default which prevents RULEs and throws an ERROR that explains they are now deprecated. Well, even if we were considering this, the sequence would need to be: 1. Announce in 9.3 that RULES will be going away RSN. 2. In 9.4, send a warning every time someone loads/edits a user-defined RULE. 3. In 10.0, get rid of CREATE RULE. This is utter nonsense. We can't deprecate them until we have a substitute that is better. If you want to get rid of rules, build the replacement; don't just try to be a pain in the ass to users. I was thinking that this should start with making a list of all of the things you can currently do with RULEs and making sure that we have an equivalent. When it actually comes time to dump RULEs entirely, we would also need docs on how to migrate existing RULEs to the new equivalents (e.g. rewriting a RULE as a view or a trigger). Wiki page, anyone? -- 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] [PATCH 8/8] Introduce wal decoding via catalog timetravel
On 12-10-11 06:27 PM, Josh Berkus wrote: On 10/10/12 7:26 PM, Bruce Momjian wrote: How does Slony write its changes without causing serialization replay conflicts? Since nobody from the Slony team answered this: a) Slony replicates *rows*, not *statements* True, but the proposed logical replication also would replicate rows not the original statements. I don't consider this proposal to be an example of 'statement' replication like pgpool is. If the original SQL was 'update foo set x=x+1 where id 10'; there will be a WAL record to decode for each row modified by the table. In a million row table I'd expect the replica will have to apply a million records (whether they be binary heap tuples or SQL statements). b) Slony uses serializable mode internally for row replication Actually recent versions of slony apply transactions against the replica in read committed mode. Older versions used serializable mode but with the SSI changes in 9.1 we found slony tended to have serialization conflicts with itself on the slony internal tables resulting in a lot of aborted transactions. When slony applies changes on a replica table it does so in a single transaction. Slony finds a set of transactions that committed on the master in between two SYNC events. It then applies all of the rows changed by any of those transactions as part of a single transaction on the replica. Chris's post explains this in more detail. Conflicts with user transactions on the replica are possible. c) it's possible (though difficult) for creative usage to get Slony into a deadlock situation FWIW, I have always assumed that is is impossible (even theoretically) to have statement-based replication without some constraints on the statements you can run, or some replication failures. I think we should expect 9.3's logical replication out-the-gate to have some issues and impose constraints on users, and improve with time but never be perfect. The design Andres and Simon have advanced already eliminates a lot of the common failure cases (now(), random(), nextval()) suffered by pgPool and similar tools. But remember, this feature doesn't have to be *perfect*, it just has to be *better* than the alternatives. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecating RULES
With the DDL trigger, we're able to do that faster. The idea is you can still delete it if you need compatibility, so we get the message across without an extra release and without an annoying GUC (etc). You're seeing these things as bugs. I see them as features. And we don't need a GUC if you can't turn the warning off. I'm also not real keen on the idea that someone could dump a 9.2 database and be unable to load it into 9.3 because of the DDL trigger, especially if they might not encounter it until halfway through a restore. That seems rather user-hostile to me. Also, how would you picture that working with pg_upgrade? RULEs are a major feature we've had for over a decade. We've discussed deprecating them on -hackers, but believe it or don't, most of our users don't read -hackers. We need to warn people, loudly and repeatedly, for at *least* a year and a half before removing RULEs. So, to expand on my sequence of events: 1. Figure out how to 100% replace all functionality currently offered by RULEs (this may already exist, but nobody has accounted it) 2. Announce that RULES will be going away after 9.4 (in 2015). 3. Amend the documentation pages on RULEs with a fat header saying this is a deprecated feature and going away in 2 versions. 4. Write wiki pages describing how to migrate away from RULEs. 5. In 9.4, send a warning every time someone CREATEs/ALTERs a user-defined RULE. 6. In 10.0, get rid of CREATE RULE. Note that steps 1-4 would need to be complete at least a year before RULEs actually go away ... preferably 2 years. And 100% of the functionality required to replace RULEs needs to be available at least one version before RULEs go away, preferably 2 versions. -- 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] WAL_DEBUG logs spurious data
Markus Wanner mar...@bluegap.ch writes: On 10/11/2012 03:11 PM, Tom Lane wrote: The original design intention was that rm_desc should not attempt to print any such data, but obviously some folks didn't get the word. FWIW: in case the source code contains comments explaining that intention, I certainly missed them so far. Yeah, if we decide to stick with the limitation, some documentation would be called for. I remember having run into this and having removed functionality from an rm_desc function rather than question the premise. But maybe the extra functionality is worth the cycles. 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] WAL_DEBUG logs spurious data
On 10/11/2012 04:06 PM, Tom Lane wrote: Yeah, if we decide to stick with the limitation, some documentation would be called for. I remember having run into this and having removed functionality from an rm_desc function rather than question the premise. But maybe the extra functionality is worth the cycles. Well, I've been interested in getting it correct, and didn't care about performance. But I can certainly imagine someone enabling it on a production system to get more debug info. In which case performance would matter more. However, WAL_DEBUG being a #define, I bet only very few admins do that. So I tend towards sacrificing performance for better debug info in the WAL_DEBUG case. (Especially given that you can still disable it via GUC). Regards Markus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecating RULES
On Thu, Oct 11, 2012 at 3:59 PM, Josh Berkus j...@agliodbs.com wrote: With the DDL trigger, we're able to do that faster. The idea is you can still delete it if you need compatibility, so we get the message across without an extra release and without an annoying GUC (etc). You're seeing these things as bugs. I see them as features. And we don't need a GUC if you can't turn the warning off. I'm also not real keen on the idea that someone could dump a 9.2 database and be unable to load it into 9.3 because of the DDL trigger, especially if they might not encounter it until halfway through a restore. That seems rather user-hostile to me. Also, how would you picture that working with pg_upgrade? RULEs are a major feature we've had for over a decade. We've discussed deprecating them on -hackers, but believe it or don't, most of our users don't read -hackers. We need to warn people, loudly and repeatedly, for at *least* a year and a half before removing RULEs. So, to expand on my sequence of events: 1. Figure out how to 100% replace all functionality currently offered by RULEs (this may already exist, but nobody has accounted it) 2. Announce that RULES will be going away after 9.4 (in 2015). 3. Amend the documentation pages on RULEs with a fat header saying this is a deprecated feature and going away in 2 versions. 4. Write wiki pages describing how to migrate away from RULEs. 5. In 9.4, send a warning every time someone CREATEs/ALTERs a user-defined RULE. 6. In 10.0, get rid of CREATE RULE. I think this more realistic. One other thing I'd like to state is that one we move into hard-core deprecation mode that the DDL trigger may not be enough to incite the action we need to smoothly deprecate. Instead, any *use* of a rule that is not through a view (aka a non-deprecated feature) should spam you with a nice big warning to annoy you into taking action. This may be in a release following the DDL-trigger-warning. This may sound insane, but it worked for standards conforming strings, and that seems to have gone reasonably well...at least taken in comparison to bytea. -- fdr -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecating RULES
On 11 October 2012 20:28, Simon Riggs si...@2ndquadrant.com wrote: Not many RULE-lovers out there, once you've tried to use them. Allowing RULEs complicates various things and can make security more difficult. What exactly do they make more difficult? Are you particularly concerned with the overhead that rules impose when developing certain types of features? If so, since there's going to be a legacy compatibility mode for a long time, I don't know that deprecating rules will buy you much in the next 3 - 4 releases. For 9.3, I suggest we create a DDL trigger by default which prevents RULEs and throws an ERROR that explains they are now deprecated. Anybody that really cares can delete this and use them. Sometime in future, we hard code it, barring complaints. Well, rules have been around since the Berkeley days [1]. I don't think that anyone, including Tom, is willing to argue that user-defined rules are not a tar-pit (except perhaps ON SELECT DO INSTEAD SELECT rules - which are exactly equivalent to views anyway). Personally, I'd like to see them removed too. However, in order to be able to get behind your proposal, I'd like to see a reasonably developed cost/benefit analysis. People do use user-defined rules. For example, the xTuple open source ERP package uses ON INSERT DO INSTEAD rules [2]. [1] http://db.cs.berkeley.edu/papers/ERL-M89-82.pdf [2] http://www.xtuple.org/ApiDevelopment -- 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] Deprecating RULES
On 10/11/2012 03:59 PM, Josh Berkus wrote: I'm also not real keen on the idea that someone could dump a 9.2 database and be unable to load it into 9.3 because of the DDL trigger, especially if they might not encounter it until halfway through a restore. That seems rather user-hostile to me. Also, how would you picture that working with pg_upgrade? RULEs are a major feature we've had for over a decade. That nobody in the right mind would use in production for YEARS. That said there is a very real problem here. For a very, very long time the recommended way (wrong way in fact) to do partitioning was based on rules. Now, those in the know immediately said, WTF but I bet you that a lot of people that we don't know about are using rules for partitioning. We definitely need a warning period that this is going away. That said, I don't know that we need a whole release cycle. If we start announcing now (or before the new year) that in 9.3 we will not have rules, that gives people 9-10 months to deal with the issue and that is assuming that we are dealing with early adopters, which we aren't because early adopters are not going to be using rules. JD -- Command Prompt, Inc. - http://www.commandprompt.com/ PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, Postgres-XC @cmdpromptinc - 509-416-6579 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecating RULES
On Thu, Oct 11, 2012 at 5:07 PM, Joshua D. Drake j...@commandprompt.com wrote: On 10/11/2012 03:59 PM, Josh Berkus wrote: I'm also not real keen on the idea that someone could dump a 9.2 database and be unable to load it into 9.3 because of the DDL trigger, especially if they might not encounter it until halfway through a restore. That seems rather user-hostile to me. Also, how would you picture that working with pg_upgrade? RULEs are a major feature we've had for over a decade. That nobody in the right mind would use in production for YEARS. That said there is a very real problem here. For a very, very long time the recommended way (wrong way in fact) to do partitioning was based on rules. Now, those in the know immediately said, WTF but I bet you that a lot of people that we don't know about are using rules for partitioning. We definitely need a warning period that this is going away. That said, I don't know that we need a whole release cycle. If we start announcing now (or before the new year) that in 9.3 we will not have rules, that gives people 9-10 months to deal with the issue and that is assuming that we are dealing with early adopters, which we aren't because early adopters are not going to be using rules. My experience suggests that only ample annoyance for at least one full release cycle will provide a low-impact switch. This annoyance must not be able to be turned off. -- fdr -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecating RULES
On 10/11/2012 08:20 PM, Daniel Farina wrote: On Thu, Oct 11, 2012 at 5:07 PM, Joshua D. Drake j...@commandprompt.com wrote: On 10/11/2012 03:59 PM, Josh Berkus wrote: I'm also not real keen on the idea that someone could dump a 9.2 database and be unable to load it into 9.3 because of the DDL trigger, especially if they might not encounter it until halfway through a restore. That seems rather user-hostile to me. Also, how would you picture that working with pg_upgrade? RULEs are a major feature we've had for over a decade. That nobody in the right mind would use in production for YEARS. That said there is a very real problem here. For a very, very long time the recommended way (wrong way in fact) to do partitioning was based on rules. Now, those in the know immediately said, WTF but I bet you that a lot of people that we don't know about are using rules for partitioning. We definitely need a warning period that this is going away. That said, I don't know that we need a whole release cycle. If we start announcing now (or before the new year) that in 9.3 we will not have rules, that gives people 9-10 months to deal with the issue and that is assuming that we are dealing with early adopters, which we aren't because early adopters are not going to be using rules. My experience suggests that only ample annoyance for at least one full release cycle will provide a low-impact switch. This annoyance must not be able to be turned off. Spot on. All our experience is that just announcing things, especially in places other than release notes and similar, is ineffective as a way of communicating with our user base. I'm with Tom and Josh and Daniel on this, and to be honest I'm somewhat surprised at the willingness of some people to spring surprises on users. I still come across uses of rules in the wild, and not just for partitioning either. Personally I think if we start now the earliest we should even consider removing the support is 9.4. 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] Deprecating RULES
-BEGIN PGP SIGNED MESSAGE- Hash: RIPEMD160 Tom and Simon wrote: If you want to get rid of rules, build the replacement; don't just try to be a pain in the ass to users. Supporting broken and non-standard features *is* a pain in the ass to users, since they are sometimes persuaded to use them and then regret it. Or if they do, hit later problems. Broken? That's a strong word. Tricky perhaps. Best avoided by novices, yes. But broken? Do they not work exactly as described in the fine manual? FWIW, I still see them a lot in the wild. Anyway, lets start with a discussion of what rules give us that SQL standard features do not? Even if the answer is nothing, if we do not implement the SQL standard feature yet (exhibit A: updateable views), it's a moot point unless the goal is to spur development of those features just so we can deprecate rules. - -- Greg Sabino Mullane g...@turnstep.com End Point Corporation http://www.endpoint.com/ PGP Key: 0x14964AC8 201210112251 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iEYEAREDAAYFAlB3hx8ACgkQvJuQZxSWSshhwQCfdtKc7R2i0kz7eDUTXtik93k3 KyEAoK0dQVZsfcAD3OlHYDVhWMjst8QZ =xY2L -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecating RULES
FWIW I thought the stuff in commit 092d7ded2 might be pretty useful generally. -- Álvaro Herrerahttp://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] Deprecating RULES
-Original Message- From: pgsql-hackers-ow...@postgresql.org [mailto:pgsql-hackers- ow...@postgresql.org] On Behalf Of Andrew Dunstan Sent: Thursday, October 11, 2012 8:52 PM To: Daniel Farina Cc: Joshua D. Drake; Josh Berkus; Simon Riggs; pgsql-hackers@postgresql.org Subject: Re: [HACKERS] Deprecating RULES On 10/11/2012 08:20 PM, Daniel Farina wrote: On Thu, Oct 11, 2012 at 5:07 PM, Joshua D. Drake j...@commandprompt.com wrote: On 10/11/2012 03:59 PM, Josh Berkus wrote: I'm also not real keen on the idea that someone could dump a 9.2 database and be unable to load it into 9.3 because of the DDL trigger, especially if they might not encounter it until halfway through a restore. That seems rather user-hostile to me. Also, how would you picture that working with pg_upgrade? RULEs are a major feature we've had for over a decade. That nobody in the right mind would use in production for YEARS. That said there is a very real problem here. For a very, very long time the recommended way (wrong way in fact) to do partitioning was based on rules. Now, those in the know immediately said, WTF but I bet you that a lot of people that we don't know about are using rules for partitioning. We definitely need a warning period that this is going away. That said, I don't know that we need a whole release cycle. If we start announcing now (or before the new year) that in 9.3 we will not have rules, that gives people 9-10 months to deal with the issue and that is assuming that we are dealing with early adopters, which we aren't because early adopters are not going to be using rules. My experience suggests that only ample annoyance for at least one full release cycle will provide a low-impact switch. This annoyance must not be able to be turned off. Spot on. All our experience is that just announcing things, especially in places other than release notes and similar, is ineffective as a way of communicating with our user base. I'm with Tom and Josh and Daniel on this, and to be honest I'm somewhat surprised at the willingness of some people to spring surprises on users. I still come across uses of rules in the wild, and not just for partitioning either. Personally I think if we start now the earliest we should even consider removing the support is 9.4. cheers andrew Deprecation means that existing code will no longer work without refactoring. If CREATE RULE was a security hazard or unstable that may justify such an action but simply because using it properly (or at least safely) is difficult doesn't mean that those who have managed should be punished for their expertise. Late night rambling here but the risk mitigation that we seem to be caring about is new users searching for and using algorithms that they find on the web without understanding the intricacies of how those algorithms work. Do we really want to build something into the database to deal with this (by disallowing it outright) or do we do our best to provide authoritative and useful documentation so that when users go looking for the CREATE RULE command in our documentation they are provided with reasoning and alternatives to its use? RULEs may be difficult but maybe there are some rare use-cases where they would be appropriate. No one here is all-knowing and just maybe someone in the future will have an idea and decide to further improve them or at the least recognize a situation where the current implementation is useful. So, what actual harms are there to using CREATE RULE and are there less invasive means, via a more nuanced restriction implementation of CREATE RULE or simply via documentation, to mitigate those harms? Maybe there would not be enough benefits to CREATE RULE at this point in time to consider implementing in from scratch but given that it already exists it should be worth some effort to keep it functioning even if only for forward-compatibility reasons. And regardless, the whole what do you use instead of CREATE RULE documentation needs to be created no matter the eventual decision to fully remove the feature from the system. David J. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecating RULES
Josh Berkus wrote: For 9.3, I suggest we create a DDL trigger by default which prevents RULEs and throws an ERROR that explains they are now deprecated. Well, even if we were considering this, the sequence would need to be: 1. Announce in 9.3 that RULES will be going away RSN. 2. In 9.4, send a warning every time someone loads/edits a user-defined RULE. 3. In 10.0, get rid of CREATE RULE. I think we can easily move up the first 2 steps. 1. Announce right now, effectively in 9.2, that RULES are going away soon. 2. In 9.3, send the warning. Then optionally 3. In 9.4 can be where CREATE RULE is removed, or stay with 10.0 there and we have a solid 2 years of warnings instead of one. It seems to me that step 1 is completely outside the release cycle, as it doesn't involve changing any code. Since 9.2 just came out, we can just do #1 as of 9.2. The only reason I see to delay #1 is if we aren't sure we're going to go ahead with it, and give a few months to think about it before announcing this major thing suddenly. Waiting until 9.3 just to make an announcement is silly. -- Darren Duncan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] line type
What's the deal with the line type? It's installed in the catalogs and listed in the documentation, varyingly as not implemented or not fully implemented, but all the support functions throw an error. Is there any known list of things that would need to be done to make it fully implemented? Or should we just get rid of it? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]
Thanks for looking at this. Attached is a rebased patch using new OIDs. On 11 October 2012 02:39, Peter Eisentraut pete...@gmx.net wrote: Compiler warning needs to be fixed: rewriteHandler.c: In function 'RewriteQuery': rewriteHandler.c:2153:29: error: 'view_rte' may be used uninitialized in this function [-Werror=maybe-uninitialized] rewriteHandler.c:2015:17: note: 'view_rte' was declared here Ah, my version of gcc doesn't give that warning. Looking at the code afresh though, I think that code block is pretty ugly. The attached version rewrites that block in a more compact form, which I think is also much more readable, and should cure the compiler warning. Maybe we should distinguish updatable from insertable in error messages like this one: ERROR: cannot insert into view foov2 DETAIL: Views containing DISTINCT are not updatable. The SQL standard distinguishes the two, so there could be differences. I'm not sure what they are right now, though. This hint could use some refreshing: HINT: You need an unconditional ON INSERT DO INSTEAD rule or an INSTEAD OF INSERT trigger. Maybe something along the lines of HINT: To make the view insertable anyway, supply an unconditional ... etc. I've not updated the error messages - I need to think about that a bit more. Regards, Dean auto-update-views.patch.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
