On 2013-01-18 11:42:47 -0500, Robert Haas wrote: > On Fri, Jan 18, 2013 at 10:47 AM, Andres Freund <and...@2ndquadrant.com> > wrote: > >> No, there's one also in heap_create_with_catalog. Took me a minute to > >> find it, as it does not use InvokeObjectAccessHook. The idea is that > >> OAT_POST_CREATE fires once per object creation, regardless of the > >> object type - table, column, whatever. > > > > Ah. Chose the wrong term to grep for. I am tempted to suggest adding a > > comment explaining why we InvokeObjectAccessHook isn't used just for > > enhanced grepability. > > I cannot parse that sentence, but I agree that the ungreppability of > it is suboptimal. I resorted to git log -SInvokeObjectAccessHook -p > to find it.
I was thinking of adding a comment like /* We don't use InvokeObjectAccessHook here because we need to ... */ not because the comment in itself is important but because it allows to grep ;) > >> I have been involved in PostgreSQL development for about 4.5 years > >> now. This is less time than many people here, but it's still long > >> enough to say a whole lot of people ask for some variant of this idea, > >> and yet I have yet to see anybody produce a complete, working version > >> of this functionality and maintain it outside of the PostgreSQL tree > >> for one release cycle (did I miss something?). > > > > I don't really think thats a fair argument. > > > > For one, I didn't really ask for a libpgdump - I said that I don't see > > any way to generate re-executable SQL without it if we don't get the > > parse-tree but only the oid of the created object. Not to speak of the > > complexities of supporting ALTER that way (which is, as you note, > > basically not the way to do this). > > Oh, OK. I see. Well, I've got no problem making the parse tree > available via InvokeObjectAccessHook. Isn't that just a matter of > adding a new hook type and a new call site? Sure. Its probably not easy to choose the correct callsites though, they need to be somewhat different for create & alter in comparison to drop. create & alter * after the object is created/altered drop: * after the object is locked, but *before* its dropped As far as I understood thats basically the behind the ddl_trace event trigger mentioned earlier. > > Also, even though I don't think its the right way *for this*, I think > > pg_dump and pgadmin pretty much prove that it's possible? The latter > > even is out-of-core and has existed for multiple years. > > Fair point. Imo its already a good justification for a libpgdump, not that I am volunteering, but ... > > Its also not really fair to compare out-of-tree effort of maintaining > > such a library to in-core support. pg_dump *needs* to be maintained, so > > the additional maintenance overhead once the initial development is done > > shouldn't really be higher than now. Lower if anything, due to getting > > rid of a good bit of ugly code ;). There's no such effect out of core. > > This I don't follow. Nothing we might add to core to reverse-parse > either the catalogs or the parse tree is going to make pg_dump go > away. I was still talking about the hypothetical libpgdump we don't really need for this ;) > > If youre also considering parsetree->SQL transformation under the > > libpgdump umbrella its even less fair. The backend already has a *lot* > > of infrastructure to regenerate sql from trees, even if its mostly > > centered arround around DQL. A noticeable amount of that code is > > unavailable to extensions (i.e. many are static funcs). > > Imo its completely unreasonable to expose that code to the outside and > > expect it to have a stable interface. We *will* need to rewrite parts of > > that regularly. > > For those reasons I think the only reasonable way to create textual DDL > > is in the backend trying to allow outside code to do that will impose > > far greater limitations. > > I'm having trouble following this. Can you restate? I wasn't sure > what you meant by libpqdump. I assumed you were speaking of a > parsetree->DDL or catalog->DDL facility. Yea, that wasn't really clear, sorry for that. What I was trying to say that I think doing parstree->text conversion out of tree is noticeably more complex and essentially places a higher maintenance burden on the project because 1) the core already has a lot of infrastructure for such conversions 2) any external project would either need to copy that infrastructure or make a large number of functions externally visible 3) any refactoring in that area would now break external tools even if it would be trivial to adjust potential in-core support for such a conversion > > Some version of the event trigger patch contained partial support for > > regenerating the DDL so it seems like a justified point there. Your > > complained that suggestions about reusing object access hooks were > > ignored, so mentioning that they currently don't provide *enough* (they > > *do* provide a part, but it doesn't seem to be the most critical one) > > also seems justifyable. > > Sure, but we could if we wanted decide to commit the partial support > for regenerating the DDL and tell people to use it via object access > hooks. Yes, thats a possibility. > Of course, that would thin out even further the number of > people who would actually be using that code, which I fear will > already be too small to achieve bug-free operation in a reasonable > time. Well, replication solutions seem to be the main potential consumer of such a capability and while the C level stuff wouldn't be used by many, hopefully the stuff built on top will. > If we add some hooks now and someone maintains the DDL > reverse-parsing code as an out-of-core replication solution for a few > releases, and that doesn't turn out to be hideous, I'd be a lot more > sanguine about incorporating it at that point. As I said above, I think due to the available infrastructure doing this out of tree is *far* harder and I fear that a good part of that code would turn out not to be applicable for inclusion into core. > I basically don't > think that there's any way we're going to commit something along those > lines now and not have it turn out to be a far bigger maintenance > headache than anyone wants. What that tends to end up meaning in > practice is that Tom gets suckered into fixing it because nobody else > can take the time, and that's neither fair nor good for the project. Now as is in "at this point of the cycle" or now as in "without a several year old out-of-core project"? > > NP, its good to keep the technical stuff here anyway... Stupid > > technology. In which business are we in again? > > I don't know, maybe we can figure it out based on the objects in our > immediate vicinity. I see twelve cans of paint, most of them opened, > and something that looks sort of like a badly-made shed. Does that > help at all? :-) By the smell and looks from where I am sitting I seem to be a barrista making rather deliciously smelling coffee and my boss seems to suck at decorating ;). Oh, and I seem to have a terrible taste in music. Regards, 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
