Re: Patch for 8.5, transformationHook
От | Robert Haas |
---|---|
Тема | Re: Patch for 8.5, transformationHook |
Дата | |
Msg-id | 603c8f070909171944v12d2d309ra6115aeb1aec6c0e@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Patch for 8.5, transformationHook (Robert Haas <robertmhaas@gmail.com>) |
Список | pgsql-hackers |
On Sun, Sep 13, 2009 at 10:32 PM, Robert Haas <robertmhaas@gmail.com> wrote: > On Tue, Aug 11, 2009 at 12:09 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote: >> 2009/8/10 Tom Lane <tgl@sss.pgh.pa.us>: >>> Robert Haas <robertmhaas@gmail.com> writes: >>>> On Sun, Jul 26, 2009 at 9:29 AM, Pavel Stehule<pavel.stehule@gmail.com> wrote: >>>>> new patch add new contrib "transformations" with three modules >>>>> anotation, decode and json. >>> >>>> These are pretty good examples, but the whole thing still feels a bit >>>> grotty to me. The set of syntax transformations that can be performed >>>> with a hook of this type is extremely limited - in particular, it's >>>> the set of things where the parser thinks it's valid and that the >>>> structure is reasonably similar to what you have in mind, but the >>>> meaning is somewhat different. The fact that two of your three >>>> examples require your named and mixed parameters patch seems to me to >>>> be evidence of that. >>> >>> I finally got around to looking at these examples, and I still don't >>> find them especially compelling. Both the decode and the json example >>> could certainly be done with regular function definitions with no need >>> for this hook. The => to AS transformation maybe not, but so what? >>> The reason we don't have that one in core is not technological. >>> >>> The really fundamental problem with this hook is that it can't do >>> anything except create syntactic sugar, and a pretty darn narrow class >>> of syntactic sugar at that. Both the raw parse tree and the transformed >>> tree still have to be valid within the core system's understanding. >>> What's more, since there's no hook in ruleutils.c, what is going to come >>> out of the system (when dumping, examining a view, etc) is the >>> transformed expression --- so you aren't really hiding any complexity >>> from the user, you're just providing a one-time shorthand that will be >>> expanded into a notation he also has to be familiar with. >>> >> >> I agree - so this could be a problem >> >>> Now you could argue that we've partly created that restriction by >>> insisting that the hook be in transformFuncCall and not transformExpr. >>> But that only restricts the subset of raw parse trees that you can play >>> with; it doesn't change any of the other restrictions. >>> >>> Lastly, I don't think the problem of multiple hook users is as easily >>> solved as Pavel claims. These contrib modules certainly fail to solve >>> it. Try unloading (or re-LOADing) them in a different order than they >>> were loaded. >>> >> >> There are two possible solution >> >> a) all modules should be loaded only from configuration >> b) modules should be loaded in transformation time - transformation of >> functions should be substituted some registered function for some >> functions. This little bit change sense of this patch. But it's enough >> for use cases like DECODE, JSON, SOAP. It's mean one new column to >> pg_proc - like protransformfunc. >> >> ??? >> Pavel >> >>> So on the whole I still think this is a solution looking for a problem, >>> and that any problems it could solve are better solved elsewhere. > > I am in the process of looking through the patches to be assigned for > the September CommitFest, and it seems to me that we really haven't > made any progress here since the last CommitFest. Jeff Davis provided > a fairly good summary of the issues: > > http://archives.postgresql.org/message-id/1249784508.9256.892.camel@jdavis > > I don't think we really gain much by assigning yet another reviewer to > this patch. The patch is simple enough and doesn't really need any > further code review AFAICS, but nobody except the patch author seems > confident that this is all that useful.[1] I'm biased by the fact that > I reviewed this patch and didn't particularly like it either, but I > think we need more than to think about committing this in the face of > Tom Lane's opinion (which I share, FWIW) that this is of very limited > usefulness. > > ...Robert > > [1] Indeed, the few supportive responses were along the lines of "oh - > this should help with X" to which the response was, in at least two > cases, "well actually no it won't". Based on the above reasoning, and hearing no contrary hue and cry from the masses, I am marking this patch as rejected. ...Robert
В списке pgsql-hackers по дате отправления: