Обсуждение: Patch for 8.5, transformationHook
Hello, I am sending small patch, that allows hooking transformation stage of parser. Regards Pavel Stehule
Вложения
Pavel Stehule <pavel.stehule@gmail.com> writes: > I am sending small patch, that allows hooking transformation stage of parser. Isn't this the exact same patch we rejected several months ago? regards, tom lane
2009/4/11 Tom Lane <tgl@sss.pgh.pa.us>: > Pavel Stehule <pavel.stehule@gmail.com> writes: >> I am sending small patch, that allows hooking transformation stage of parser. > > Isn't this the exact same patch we rejected several months ago? > > regards, tom lane What I remember, You had some objections about different behave before and after loading an library. In this time I hadn't good arguments, and my proposal was using GUC. What is maybe wrong. I thing, I found better solution. We found, so isn't possible raise exception in _PG_init function. But I can raise warning when library will be loaded in normal runtime. And I can raise warning (or exception) when every function from library is called. When library is loaded from configuration (share_preloaded_libraries), then PostgreSQL's behave will be stable. So I am able to ensure, so anybody doesn't forgot load any library based on transformatio hook. regards Pavel Stehule >
Pavel Stehule <pavel.stehule@gmail.com> writes: > 2009/4/11 Tom Lane <tgl@sss.pgh.pa.us>: >> Pavel Stehule <pavel.stehule@gmail.com> writes: >>> I am sending small patch, that allows hooking transformation stage of parser. >> >> Isn't this the exact same patch we rejected several months ago? > What I remember, You had some objections about different behave before > and after loading an library. No, I was complaining that a hook right there is useless and expensive. transformExpr() is executed multiple times per query, potentially a very large number of times per query; so even testing to see if a hook exists is not a negligible cost. And I have not seen anything I regard as a convincing demonstration of use-case that can't be handled as well or better in some other way. regards, tom lane
2009/4/11 Tom Lane <tgl@sss.pgh.pa.us>: > Pavel Stehule <pavel.stehule@gmail.com> writes: >> 2009/4/11 Tom Lane <tgl@sss.pgh.pa.us>: >>> Pavel Stehule <pavel.stehule@gmail.com> writes: >>>> I am sending small patch, that allows hooking transformation stage of parser. >>> >>> Isn't this the exact same patch we rejected several months ago? > >> What I remember, You had some objections about different behave before >> and after loading an library. > > No, I was complaining that a hook right there is useless and expensive. > transformExpr() is executed multiple times per query, potentially a very > large number of times per query; so even testing to see if a hook exists > is not a negligible cost. And I have not seen anything I regard as a > convincing demonstration of use-case that can't be handled as well or > better in some other way. > I will do some performance testing. But effect of empty hook should be similar to testing some GUC now. But I have to do some metering. Actually transformExpr contains relative big case now, and empty hook has similar performance effect as new parser node. I sent some examples, that helps to people with database migration (some are obscure, I know - Oracle empty string support - it's +/- joke, there are more serious samples ). And I am preparing JSON support as example of some comfortable libraries. Next use case should be in enhancing of db-link functions. http://archives.postgresql.org/pgsql-hackers/2009-03/msg01239.php regards Pavel Stehule > regards, tom lane >
Hello 2009/4/11 Tom Lane <tgl@sss.pgh.pa.us>: > Pavel Stehule <pavel.stehule@gmail.com> writes: >> 2009/4/11 Tom Lane <tgl@sss.pgh.pa.us>: >>> Pavel Stehule <pavel.stehule@gmail.com> writes: >>>> I am sending small patch, that allows hooking transformation stage of parser. >>> >>> Isn't this the exact same patch we rejected several months ago? > >> What I remember, You had some objections about different behave before >> and after loading an library. > > No, I was complaining that a hook right there is useless and expensive. > transformExpr() is executed multiple times per query, potentially a very > large number of times per query; so even testing to see if a hook exists > is not a negligible cost. And I have not seen anything I regard as a > convincing demonstration of use-case that can't be handled as well or > better in some other way. > > regards, tom lane > I did some tests based on pgbench. The test base was initialised with scaling factor 10. I tested high transaction number with single client. Result is not clean, but doesn't show significant slowness for patched parser. In both cases pbbench and postresql was installed on single computer. First I tested on 4years old notebook prestigio nobile 156 (Pentium M, 1.6). I tested pgbench (-t 100000) with/without switch -S without patch 6950+/-13 (-S) 660 +/- 11 patched 6879+/-30 672 +/- 21 -------------------------------------------------- diff -1.02% +1.79% Next test I did on Dell 830 Core(TM)2 Duo 2.4 withhout patch 9253+/-47 (-S) 209 +/- 4 patched 9299+/-14 214+/- 1 --------------------------------------------------- diff +0.49% +2.33% Result: The most worst case - pgbench -S -t100000 is 1% slower then unpatched code (on older computer). With some more similar to normal traffic, the patched code was 2% faster. I don't know why patched code should be faster - but this is result from pgbench - on linux fedora 10, Intel, without GUI I though about different position of hook, but only in this place the hook is useful (because expressions are recursive). Elsewhere the hook hasn't sense :(. So transformationHook doesn't do significant slowness. Other possibility is an callback, or some, but I dislike it. Regards Pavel Stehule
Pavel Stehule <pavel.stehule@gmail.com> writes: > 2009/4/11 Tom Lane <tgl@sss.pgh.pa.us>: >> No, I was complaining that a hook right there is useless and expensive. >> transformExpr() is executed multiple times per query, potentially a very >> large number of times per query; so even testing to see if a hook exists >> is not a negligible cost. > I did some tests based on pgbench. The queries done by pgbench are completely trivial and do not stress parser performance. Even if they did (consider cases likw an IN with a few thousand list items), the parser is normally not a bottleneck compared to transaction overhead, network round trips, and pgbench itself. > I though about different position of hook, but only in this place the > hook is useful (because expressions are recursive). As I keep saying, a hook there is useless, at least by itself. You have no control over the grammar and no ability to modify what the rest of the system understands. The only application I can think of is to fool with the transformation of FuncCall nodes, which you could do in a much lower-overhead way by hooking into transformFuncCall. Even that seems pretty darn marginal for real-world problems. regards, tom lane
2009/4/18 Tom Lane <tgl@sss.pgh.pa.us>: > Pavel Stehule <pavel.stehule@gmail.com> writes: >> 2009/4/11 Tom Lane <tgl@sss.pgh.pa.us>: >>> No, I was complaining that a hook right there is useless and expensive. >>> transformExpr() is executed multiple times per query, potentially a very >>> large number of times per query; so even testing to see if a hook exists >>> is not a negligible cost. > >> I did some tests based on pgbench. > > The queries done by pgbench are completely trivial and do not stress > parser performance. Even if they did (consider cases likw an IN with a > few thousand list items), the parser is normally not a bottleneck > compared to transaction overhead, network round trips, and pgbench > itself. > >> I though about different position of hook, but only in this place the >> hook is useful (because expressions are recursive). > > As I keep saying, a hook there is useless, at least by itself. You > have no control over the grammar and no ability to modify what the > rest of the system understands. There are lot of things, that should be done with current grammar only on transformation stage. Currently pg do it now. There are lot of pseudo functions, that are specially transformed: least, greatest, coalesce. After hooking we should do some similar work from outer libraries. The only application I can think of is > to fool with the transformation of FuncCall nodes, which you could do in > a much lower-overhead way by hooking into transformFuncCall. Even that > seems pretty darn marginal for real-world problems. FuncCall should be. The base what I want is possible via transformFuncCall. Probably we cannot emulate Oracle's empty string behave, but it wasn't important :). regards Pavel Stehule > > regards, tom lane >
On Saturday 18 April 2009 18:09:00 Pavel Stehule wrote: > There are lot of things, that should be done with current grammar only > on transformation stage. Currently pg do it now. There are lot of > pseudo functions, that are specially transformed: least, greatest, > coalesce. After hooking we should do some similar work from outer > libraries. There are surely other ways to accomplish this than an expression transformation hook. Adding a property or two to the function definition to do what you want could do it.
2009/4/19 Peter Eisentraut <peter_e@gmx.net>: > On Saturday 18 April 2009 18:09:00 Pavel Stehule wrote: >> There are lot of things, that should be done with current grammar only >> on transformation stage. Currently pg do it now. There are lot of >> pseudo functions, that are specially transformed: least, greatest, >> coalesce. After hooking we should do some similar work from outer >> libraries. > > There are surely other ways to accomplish this than an expression > transformation hook. Adding a property or two to the function definition to > do what you want could do it. > should you describe it little bit more? regards Pavel
On Sunday 19 April 2009 20:47:37 Pavel Stehule wrote: > 2009/4/19 Peter Eisentraut <peter_e@gmx.net>: > > On Saturday 18 April 2009 18:09:00 Pavel Stehule wrote: > >> There are lot of things, that should be done with current grammar only > >> on transformation stage. Currently pg do it now. There are lot of > >> pseudo functions, that are specially transformed: least, greatest, > >> coalesce. After hooking we should do some similar work from outer > >> libraries. > > > > There are surely other ways to accomplish this than an expression > > transformation hook. Adding a property or two to the function definition > > to do what you want could do it. > > should you describe it little bit more? The question we should be asking is, what is it that prevents us from implementing least, greatest, and coalesce in user space now? And then design a solution for that, if we wanted to pursue this. Instead of writing transformation hooks and then force every problem to fit that solution.
2009/4/20 Peter Eisentraut <peter_e@gmx.net>: > On Sunday 19 April 2009 20:47:37 Pavel Stehule wrote: >> 2009/4/19 Peter Eisentraut <peter_e@gmx.net>: >> > On Saturday 18 April 2009 18:09:00 Pavel Stehule wrote: >> >> There are lot of things, that should be done with current grammar only >> >> on transformation stage. Currently pg do it now. There are lot of >> >> pseudo functions, that are specially transformed: least, greatest, >> >> coalesce. After hooking we should do some similar work from outer >> >> libraries. >> > >> > There are surely other ways to accomplish this than an expression >> > transformation hook. Adding a property or two to the function definition >> > to do what you want could do it. >> >> should you describe it little bit more? > > The question we should be asking is, what is it that prevents us from > implementing least, greatest, and coalesce in user space now? And then design > a solution for that, if we wanted to pursue this. Instead of writing > transformation hooks and then force every problem to fit that solution. > I don't believe so is possible to find other general solution. (or better I didn't find any other solution). Tom has true, transformationHook on expression is expensive. I thing, so hook on function should be simple and fast - not all transformation's should be simple defined via property - classic sample is "decode" like functions, it needs procedural code. regards Pavel Stehule
2009/4/18 Tom Lane <tgl@sss.pgh.pa.us>: > Pavel Stehule <pavel.stehule@gmail.com> writes: >> 2009/4/11 Tom Lane <tgl@sss.pgh.pa.us>: >>> No, I was complaining that a hook right there is useless and expensive. >>> transformExpr() is executed multiple times per query, potentially a very >>> large number of times per query; so even testing to see if a hook exists >>> is not a negligible cost. > >> I did some tests based on pgbench. > > The queries done by pgbench are completely trivial and do not stress > parser performance. Even if they did (consider cases likw an IN with a > few thousand list items), the parser is normally not a bottleneck > compared to transaction overhead, network round trips, and pgbench > itself. > >> I though about different position of hook, but only in this place the >> hook is useful (because expressions are recursive). > > As I keep saying, a hook there is useless, at least by itself. You > have no control over the grammar and no ability to modify what the > rest of the system understands. The only application I can think of is > to fool with the transformation of FuncCall nodes, which you could do in > a much lower-overhead way by hooking into transformFuncCall. Even that > seems pretty darn marginal for real-world problems. > Hello I am sending modified patch - it hooking parser via transformFuncCall regards Pavel Stehule > regards, tom lane >
Вложения
On Monday 20 April 2009 09:52:05 Pavel Stehule wrote: > I don't believe so is possible to find other general solution. (or > better I didn't find any other solution). Tom has true, > transformationHook on expression is expensive. I thing, so hook on > function should be simple and fast - not all transformation's should > be simple defined via property - classic sample is "decode" like > functions, it needs procedural code. I find this all a bit premature, given that you haven't clearly defined what sort of user-visible functionality you hope to end up implementing. Which makes it hard to argue why this or that approach might be better.
Peter Eisentraut <peter_e@gmx.net> writes: > I find this all a bit premature, given that you haven't clearly defined what > sort of user-visible functionality you hope to end up implementing. That sums up my reaction too --- this looks like a solution in search of a problem. The hook itself might be relatively harmless as long as it's not in a performance-critical place, but I think people would tend to contort their thinking to match what they can do with the hook rather than think about what an ideal solution might be. I'm also concerned that a hook like this is not usable unless there are clear conventions about how multiple shared libraries should hook into it simultaneously. The other hooks we have mostly aren't intended for purposes that might need concurrent users of the hook, but it's hard to argue that the case won't come up if this hook actually gets used. regards, tom lane
2009/4/20 Peter Eisentraut <peter_e@gmx.net>: > On Monday 20 April 2009 09:52:05 Pavel Stehule wrote: >> I don't believe so is possible to find other general solution. (or >> better I didn't find any other solution). Tom has true, >> transformationHook on expression is expensive. I thing, so hook on >> function should be simple and fast - not all transformation's should >> be simple defined via property - classic sample is "decode" like >> functions, it needs procedural code. > > I find this all a bit premature, given that you haven't clearly defined what > sort of user-visible functionality you hope to end up implementing. Which > makes it hard to argue why this or that approach might be better. > a) it allows procedural setting for parameter's transformation and checking like fce(int, varchar, int, varchar, ....), fce(int, int, int, varchar, varchar, varchar) ... there should be hundred patterns b) it allows constructors for data types (ANSI SQL) datatype(typefield1[, typefiedl2[, typefiedl3[, ...]]]) returns type c) it allows named parameters with different syntax like Oracle fcecall(a => 10, b => 30), like Informix fcecall(a = 10, b = 30) d) with patch that allows named parameters with PostgreSQL syntax (value AS name) it allows "smart parameters" - name isn't name of variable, but label like SQL/XML xmlforest(user_name, user_name AS "user name") I hope so this is enough :) Regards Pavel
2009/4/20 Tom Lane <tgl@sss.pgh.pa.us>: > Peter Eisentraut <peter_e@gmx.net> writes: >> I find this all a bit premature, given that you haven't clearly defined what >> sort of user-visible functionality you hope to end up implementing. > > That sums up my reaction too --- this looks like a solution in search of > a problem. The hook itself might be relatively harmless as long as it's > not in a performance-critical place, but I think people would tend to > contort their thinking to match what they can do with the hook rather > than think about what an ideal solution might be. see mail to Peter, please > > I'm also concerned that a hook like this is not usable unless there are > clear conventions about how multiple shared libraries should hook into > it simultaneously. The other hooks we have mostly aren't intended for > purposes that might need concurrent users of the hook, but it's hard > to argue that the case won't come up if this hook actually gets used. > I though about it. The first rule is probably - handler have to work as filter, and should be (if is possible) independent on order. It is very similar to triggers. regards Pavel Stehule > regards, tom lane >
On Mon, Apr 20, 2009 at 8:45 AM, Pavel Stehule<pavel.stehule@gmail.com> wrote: > 2009/4/18 Tom Lane <tgl@sss.pgh.pa.us>: >> Pavel Stehule <pavel.stehule@gmail.com> writes: >>> 2009/4/11 Tom Lane <tgl@sss.pgh.pa.us>: >>>> No, I was complaining that a hook right there is useless and expensive. >>>> transformExpr() is executed multiple times per query, potentially a very >>>> large number of times per query; so even testing to see if a hook exists >>>> is not a negligible cost. >> >>> I did some tests based on pgbench. >> >> The queries done by pgbench are completely trivial and do not stress >> parser performance. Even if they did (consider cases likw an IN with a >> few thousand list items), the parser is normally not a bottleneck >> compared to transaction overhead, network round trips, and pgbench >> itself. >> >>> I though about different position of hook, but only in this place the >>> hook is useful (because expressions are recursive). >> >> As I keep saying, a hook there is useless, at least by itself. You >> have no control over the grammar and no ability to modify what the >> rest of the system understands. The only application I can think of is >> to fool with the transformation of FuncCall nodes, which you could do in >> a much lower-overhead way by hooking into transformFuncCall. Even that >> seems pretty darn marginal for real-world problems. >> > > I am sending modified patch - it hooking parser via transformFuncCall I am reviewing this patch. It seems to me upon rereading the thread that the objections Tom and Peter had to inserting a hook into transformExpr() mostly still apply to a hook in transformFuncCall(): namely, that there's no proof that putting a hook here is actually useful. I think we should apply the same criteria to this that we have to some other patches that have been rejected (like the extensible-rmgr patch Simon submitted for CommitFest 2008-11), namely, requiring that the extension mechanism be submitted together with at least two examples of how it can be used to interesting and useful things, bundled as one or more contrib modules. There is some discussion on this thread of things that you think that this patch can be used to do, but I think it would be much easier to see whether it's (a) possible and (b) not too ugly to do those things if you reduce them to code. ...Robert
Robert Haas <robertmhaas@gmail.com> writes: > I think we should apply the same criteria to this that we > have to some other patches that have been rejected (like the > extensible-rmgr patch Simon submitted for CommitFest 2008-11), namely, > requiring that the extension mechanism be submitted together with at > least two examples of how it can be used to interesting and useful > things, bundled as one or more contrib modules. I wouldn't necessarily insist on actual contrib modules. But fully worked-out example uses would certainly go a long way toward proving that the hook is good for something. In previous cases we've sometimes found out that a proposed hook definition isn't quite right after we try to use it. regards, tom lane
Hello 2009/7/25 Robert Haas <robertmhaas@gmail.com>: > On Mon, Apr 20, 2009 at 8:45 AM, Pavel Stehule<pavel.stehule@gmail.com> wrote: >> 2009/4/18 Tom Lane <tgl@sss.pgh.pa.us>: >>> Pavel Stehule <pavel.stehule@gmail.com> writes: >>>> 2009/4/11 Tom Lane <tgl@sss.pgh.pa.us>: >>>>> No, I was complaining that a hook right there is useless and expensive. >>>>> transformExpr() is executed multiple times per query, potentially a very >>>>> large number of times per query; so even testing to see if a hook exists >>>>> is not a negligible cost. >>> >>>> I did some tests based on pgbench. >>> >>> The queries done by pgbench are completely trivial and do not stress >>> parser performance. Even if they did (consider cases likw an IN with a >>> few thousand list items), the parser is normally not a bottleneck >>> compared to transaction overhead, network round trips, and pgbench >>> itself. >>> >>>> I though about different position of hook, but only in this place the >>>> hook is useful (because expressions are recursive). >>> >>> As I keep saying, a hook there is useless, at least by itself. You >>> have no control over the grammar and no ability to modify what the >>> rest of the system understands. The only application I can think of is >>> to fool with the transformation of FuncCall nodes, which you could do in >>> a much lower-overhead way by hooking into transformFuncCall. Even that >>> seems pretty darn marginal for real-world problems. >>> >> >> I am sending modified patch - it hooking parser via transformFuncCall > > I am reviewing this patch. It seems to me upon rereading the thread > that the objections Tom and Peter had to inserting a hook into > transformExpr() mostly still apply to a hook in transformFuncCall(): > namely, that there's no proof that putting a hook here is actually > useful. I think we should apply the same criteria to this that we > have to some other patches that have been rejected (like the > extensible-rmgr patch Simon submitted for CommitFest 2008-11), namely, > requiring that the extension mechanism be submitted together with at > least two examples of how it can be used to interesting and useful > things, bundled as one or more contrib modules. I have in my plan add to contrib JSON support similar to Bauman design: http://www.mysqludf.org/lib_mysqludf_json/index.php It's will be sample of "smart" functions. Because this need more then less work I am waiting on commit. Other simple intrduction contrib module should be real Oracle decode function - I sent source code some time ago. But this code needs some modification. I should send this code if you need it. Pavel > > There is some discussion on this thread of things that you think that > this patch can be used to do, but I think it would be much easier to > see whether it's (a) possible and (b) not too ugly to do those things > if you reduce them to code. > > ...Robert >
On Sat, Jul 25, 2009 at 11:38 PM, Pavel Stehule<pavel.stehule@gmail.com> wrote: > Hello > > 2009/7/25 Robert Haas <robertmhaas@gmail.com>: >> On Mon, Apr 20, 2009 at 8:45 AM, Pavel Stehule<pavel.stehule@gmail.com> wrote: >>> 2009/4/18 Tom Lane <tgl@sss.pgh.pa.us>: >>>> Pavel Stehule <pavel.stehule@gmail.com> writes: >>>>> 2009/4/11 Tom Lane <tgl@sss.pgh.pa.us>: >>>>>> No, I was complaining that a hook right there is useless and expensive. >>>>>> transformExpr() is executed multiple times per query, potentially a very >>>>>> large number of times per query; so even testing to see if a hook exists >>>>>> is not a negligible cost. >>>> >>>>> I did some tests based on pgbench. >>>> >>>> The queries done by pgbench are completely trivial and do not stress >>>> parser performance. Even if they did (consider cases likw an IN with a >>>> few thousand list items), the parser is normally not a bottleneck >>>> compared to transaction overhead, network round trips, and pgbench >>>> itself. >>>> >>>>> I though about different position of hook, but only in this place the >>>>> hook is useful (because expressions are recursive). >>>> >>>> As I keep saying, a hook there is useless, at least by itself. You >>>> have no control over the grammar and no ability to modify what the >>>> rest of the system understands. The only application I can think of is >>>> to fool with the transformation of FuncCall nodes, which you could do in >>>> a much lower-overhead way by hooking into transformFuncCall. Even that >>>> seems pretty darn marginal for real-world problems. >>>> >>> >>> I am sending modified patch - it hooking parser via transformFuncCall >> >> I am reviewing this patch. It seems to me upon rereading the thread >> that the objections Tom and Peter had to inserting a hook into >> transformExpr() mostly still apply to a hook in transformFuncCall(): >> namely, that there's no proof that putting a hook here is actually >> useful. I think we should apply the same criteria to this that we >> have to some other patches that have been rejected (like the >> extensible-rmgr patch Simon submitted for CommitFest 2008-11), namely, >> requiring that the extension mechanism be submitted together with at >> least two examples of how it can be used to interesting and useful >> things, bundled as one or more contrib modules. > > I have in my plan add to contrib JSON support similar to Bauman design: > > http://www.mysqludf.org/lib_mysqludf_json/index.php > > It's will be sample of "smart" functions. Because this need more then > less work I am waiting on commit. > > Other simple intrduction contrib module should be real Oracle decode > function - I sent source code some time ago. But this code needs some > modification. I should send this code if you need it. Sure, post it and let's discuss. ...Robert
Hello new patch add new contrib "transformations" with three modules anotation, decode and json. These modules are ported from my older work. Before applying this patch, please use named-fixed patch too. The hook doesn't need it, but modules anotation and json depend on it. Regards Pavel Stehule 2009/7/26 Robert Haas <robertmhaas@gmail.com>: > On Sat, Jul 25, 2009 at 11:38 PM, Pavel Stehule<pavel.stehule@gmail.com> wrote: >> Hello >> >> 2009/7/25 Robert Haas <robertmhaas@gmail.com>: >>> On Mon, Apr 20, 2009 at 8:45 AM, Pavel Stehule<pavel.stehule@gmail.com> wrote: >>>> 2009/4/18 Tom Lane <tgl@sss.pgh.pa.us>: >>>>> Pavel Stehule <pavel.stehule@gmail.com> writes: >>>>>> 2009/4/11 Tom Lane <tgl@sss.pgh.pa.us>: >>>>>>> No, I was complaining that a hook right there is useless and expensive. >>>>>>> transformExpr() is executed multiple times per query, potentially a very >>>>>>> large number of times per query; so even testing to see if a hook exists >>>>>>> is not a negligible cost. >>>>> >>>>>> I did some tests based on pgbench. >>>>> >>>>> The queries done by pgbench are completely trivial and do not stress >>>>> parser performance. Even if they did (consider cases likw an IN with a >>>>> few thousand list items), the parser is normally not a bottleneck >>>>> compared to transaction overhead, network round trips, and pgbench >>>>> itself. >>>>> >>>>>> I though about different position of hook, but only in this place the >>>>>> hook is useful (because expressions are recursive). >>>>> >>>>> As I keep saying, a hook there is useless, at least by itself. You >>>>> have no control over the grammar and no ability to modify what the >>>>> rest of the system understands. The only application I can think of is >>>>> to fool with the transformation of FuncCall nodes, which you could do in >>>>> a much lower-overhead way by hooking into transformFuncCall. Even that >>>>> seems pretty darn marginal for real-world problems. >>>>> >>>> >>>> I am sending modified patch - it hooking parser via transformFuncCall >>> >>> I am reviewing this patch. It seems to me upon rereading the thread >>> that the objections Tom and Peter had to inserting a hook into >>> transformExpr() mostly still apply to a hook in transformFuncCall(): >>> namely, that there's no proof that putting a hook here is actually >>> useful. I think we should apply the same criteria to this that we >>> have to some other patches that have been rejected (like the >>> extensible-rmgr patch Simon submitted for CommitFest 2008-11), namely, >>> requiring that the extension mechanism be submitted together with at >>> least two examples of how it can be used to interesting and useful >>> things, bundled as one or more contrib modules. >> >> I have in my plan add to contrib JSON support similar to Bauman design: >> >> http://www.mysqludf.org/lib_mysqludf_json/index.php >> >> It's will be sample of "smart" functions. Because this need more then >> less work I am waiting on commit. >> >> Other simple intrduction contrib module should be real Oracle decode >> function - I sent source code some time ago. But this code needs some >> modification. I should send this code if you need it. > > Sure, post it and let's discuss. > > ...Robert >
Вложения
Hello note about SQL:201x http://blogs.mysql.com/peterg/2009/06/07/soothsaying-sql-standardization-stuff/ regards Pavel Stehule 2009/7/26 Pavel Stehule <pavel.stehule@gmail.com>: > Hello > > new patch add new contrib "transformations" with three modules > anotation, decode and json. > > These modules are ported from my older work. > > Before applying this patch, please use named-fixed patch too. The hook > doesn't need it, but modules anotation and json depend on it. > > Regards > Pavel Stehule > > 2009/7/26 Robert Haas <robertmhaas@gmail.com>: >> On Sat, Jul 25, 2009 at 11:38 PM, Pavel Stehule<pavel.stehule@gmail.com> wrote: >>> Hello >>> >>> 2009/7/25 Robert Haas <robertmhaas@gmail.com>: >>>> On Mon, Apr 20, 2009 at 8:45 AM, Pavel Stehule<pavel.stehule@gmail.com> wrote: >>>>> 2009/4/18 Tom Lane <tgl@sss.pgh.pa.us>: >>>>>> Pavel Stehule <pavel.stehule@gmail.com> writes: >>>>>>> 2009/4/11 Tom Lane <tgl@sss.pgh.pa.us>: >>>>>>>> No, I was complaining that a hook right there is useless and expensive. >>>>>>>> transformExpr() is executed multiple times per query, potentially a very >>>>>>>> large number of times per query; so even testing to see if a hook exists >>>>>>>> is not a negligible cost. >>>>>> >>>>>>> I did some tests based on pgbench. >>>>>> >>>>>> The queries done by pgbench are completely trivial and do not stress >>>>>> parser performance. Even if they did (consider cases likw an IN with a >>>>>> few thousand list items), the parser is normally not a bottleneck >>>>>> compared to transaction overhead, network round trips, and pgbench >>>>>> itself. >>>>>> >>>>>>> I though about different position of hook, but only in this place the >>>>>>> hook is useful (because expressions are recursive). >>>>>> >>>>>> As I keep saying, a hook there is useless, at least by itself. You >>>>>> have no control over the grammar and no ability to modify what the >>>>>> rest of the system understands. The only application I can think of is >>>>>> to fool with the transformation of FuncCall nodes, which you could do in >>>>>> a much lower-overhead way by hooking into transformFuncCall. Even that >>>>>> seems pretty darn marginal for real-world problems. >>>>>> >>>>> >>>>> I am sending modified patch - it hooking parser via transformFuncCall >>>> >>>> I am reviewing this patch. It seems to me upon rereading the thread >>>> that the objections Tom and Peter had to inserting a hook into >>>> transformExpr() mostly still apply to a hook in transformFuncCall(): >>>> namely, that there's no proof that putting a hook here is actually >>>> useful. I think we should apply the same criteria to this that we >>>> have to some other patches that have been rejected (like the >>>> extensible-rmgr patch Simon submitted for CommitFest 2008-11), namely, >>>> requiring that the extension mechanism be submitted together with at >>>> least two examples of how it can be used to interesting and useful >>>> things, bundled as one or more contrib modules. >>> >>> I have in my plan add to contrib JSON support similar to Bauman design: >>> >>> http://www.mysqludf.org/lib_mysqludf_json/index.php >>> >>> It's will be sample of "smart" functions. Because this need more then >>> less work I am waiting on commit. >>> >>> Other simple intrduction contrib module should be real Oracle decode >>> function - I sent source code some time ago. But this code needs some >>> modification. I should send this code if you need it. >> >> Sure, post it and let's discuss. >> >> ...Robert >> >
On Sun, Jul 26, 2009 at 9:29 AM, Pavel Stehule<pavel.stehule@gmail.com> wrote: > Hello > > new patch add new contrib "transformations" with three modules > anotation, decode and json. > > These modules are ported from my older work. > > Before applying this patch, please use named-fixed patch too. The hook > doesn't need it, but modules anotation and json depend on it. 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. The JSON transformation provides functionality which is very similar to what we also offer for XML. I sort of think we ought to just provide that, rather than making it an add-on. I have found it to be a tremendously attractive alternative to XML. With regard to the annotation transformation, if we're about to diverge from SQL:201x, do we want to rethink our oppostion to foo(bar => baz)? Just asking. I'm not dead set against this patch. But I'm not really sold either. I think we need some more input from other people. ...Robert
Hello 2009/7/30 Robert Haas <robertmhaas@gmail.com>: > On Sun, Jul 26, 2009 at 9:29 AM, Pavel Stehule<pavel.stehule@gmail.com> wrote: >> Hello >> >> new patch add new contrib "transformations" with three modules >> anotation, decode and json. >> >> These modules are ported from my older work. >> >> Before applying this patch, please use named-fixed patch too. The hook >> doesn't need it, but modules anotation and json depend on it. > > 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 see the main hook using as open door to functionality like decode and json. Anotation is little bit corner use case. We don't need a change of syntax or rules in parser. But I need to get some info for functions from parser stage - like JSON or replace standard coercion rules like decode. Hook is the most simple and general technique for it (what I found). I thing, so there are other techniques - but it needs more invasive patch and are not too general - what is values of any hooking. I doesn't thing, so there will be any real extended parser based on bison in near or far future. I thing, so this is theoretically possible, but nobody work on it. What more - with extensible parser we still need the transformation hook, because we need the change in transformation - decode, json. > The JSON transformation provides functionality which is very similar > to what we also offer for XML. I sort of think we ought to just > provide that, rather than making it an add-on. I have found it to be > a tremendously attractive alternative to XML. The JSON is only one use case (there should be output to any format), and I agree, so this should be in core. But every integration similar function to core needs one or two years. Hook allows do this things faster and from external library. It's little bit lighter process to put your project to pgfoundry than to PostgreSQL core. Pavel > > With regard to the annotation transformation, if we're about to > diverge from SQL:201x, do we want to rethink our oppostion to foo(bar > => baz)? Just asking. > > I'm not dead set against this patch. But I'm not really sold either. > I think we need some more input from other people. > > ...Robert >
On Thu, Jul 30, 2009 at 1:22 AM, Pavel Stehule<pavel.stehule@gmail.com> wrote: > 2009/7/30 Robert Haas <robertmhaas@gmail.com>: >> On Sun, Jul 26, 2009 at 9:29 AM, Pavel Stehule<pavel.stehule@gmail.com> wrote: >>> Hello >>> >>> new patch add new contrib "transformations" with three modules >>> anotation, decode and json. >>> >>> These modules are ported from my older work. >>> >>> Before applying this patch, please use named-fixed patch too. The hook >>> doesn't need it, but modules anotation and json depend on it. >> >> 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 see the main hook using as open door to functionality like decode > and json. Anotation is little bit corner use case. We don't need a > change of syntax or rules in parser. But I need to get some info for > functions from parser stage - like JSON or replace standard coercion > rules like decode. Hook is the most simple and general technique for > it (what I found). I thing, so there are other techniques - but it > needs more invasive patch and are not too general - what is values of > any hooking. > > I doesn't thing, so there will be any real extended parser based on > bison in near or far future. I thing, so this is theoretically > possible, but nobody work on it. What more - with extensible parser we > still need the transformation hook, because we need the change in > transformation - decode, json. > >> The JSON transformation provides functionality which is very similar >> to what we also offer for XML. I sort of think we ought to just >> provide that, rather than making it an add-on. I have found it to be >> a tremendously attractive alternative to XML. > > The JSON is only one use case (there should be output to any format), > and I agree, so this should be in core. But every integration similar > function to core needs one or two years. Hook allows do this things > faster and from external library. It's little bit lighter process to > put your project to pgfoundry than to PostgreSQL core. I don't really believe that JSON is "only one use case". XML and JSON are in a class of their own; there's nothing else out there that is really comparable. So I guess I'm back to feeling like this is of pretty marginal benefit. But I would still like some opinions from others. ...Robert
2009/8/4 Robert Haas <robertmhaas@gmail.com>: > On Thu, Jul 30, 2009 at 1:22 AM, Pavel Stehule<pavel.stehule@gmail.com> wrote: >> 2009/7/30 Robert Haas <robertmhaas@gmail.com>: >>> On Sun, Jul 26, 2009 at 9:29 AM, Pavel Stehule<pavel.stehule@gmail.com> wrote: >>>> Hello >>>> >>>> new patch add new contrib "transformations" with three modules >>>> anotation, decode and json. >>>> >>>> These modules are ported from my older work. >>>> >>>> Before applying this patch, please use named-fixed patch too. The hook >>>> doesn't need it, but modules anotation and json depend on it. >>> >>> 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 see the main hook using as open door to functionality like decode >> and json. Anotation is little bit corner use case. We don't need a >> change of syntax or rules in parser. But I need to get some info for >> functions from parser stage - like JSON or replace standard coercion >> rules like decode. Hook is the most simple and general technique for >> it (what I found). I thing, so there are other techniques - but it >> needs more invasive patch and are not too general - what is values of >> any hooking. >> >> I doesn't thing, so there will be any real extended parser based on >> bison in near or far future. I thing, so this is theoretically >> possible, but nobody work on it. What more - with extensible parser we >> still need the transformation hook, because we need the change in >> transformation - decode, json. >> >>> The JSON transformation provides functionality which is very similar >>> to what we also offer for XML. I sort of think we ought to just >>> provide that, rather than making it an add-on. I have found it to be >>> a tremendously attractive alternative to XML. >> >> The JSON is only one use case (there should be output to any format), >> and I agree, so this should be in core. But every integration similar >> function to core needs one or two years. Hook allows do this things >> faster and from external library. It's little bit lighter process to >> put your project to pgfoundry than to PostgreSQL core. > > I don't really believe that JSON is "only one use case". XML and JSON > are in a class of their own; there's nothing else out there that is > really comparable. XML and JSON are well known formats. But everybody can use it for custom formats, for binary formats, for direct communication, for loging, ... Pavel > > So I guess I'm back to feeling like this is of pretty marginal > benefit. But I would still like some opinions from others. > > ...Robert >
Hi, Robert Haas <robertmhaas@gmail.com> writes: > I don't really believe that JSON is "only one use case". XML and JSON > are in a class of their own; there's nothing else out there that is > really comparable. You might want to hear about the UBF specs from Joe Armstrong, let me quote its page about it: UBF is a language for transporting and describing complex data structures across a network. It has three components: * UBF(A) is a data transport format, roughly equivalent to well-formed XML. * UBF(B) is a programming langauge for describing types in UBF(A) and protocols between clients and servers. UBF(B)is roughly equivalent to to Verified XML, XML-schemas, SOAP and WDSL. * UBF(C) is a meta-level protocol between used between UBF servers. While the XML series of languages had the goal of having a human readable format the UBF languages take the opposite viewand provide a "machine friendly" format. http://www.sics.se/~joe/ubf/site/home.html It seems there's an ongoing revision to adapt this work to JSON nowadays: http://armstrongonsoftware.blogspot.com/2009/02/json-protocols-part-1.html Oh and now I'm wondering about ASN.1... Regards, -- dim
Hello > > * UBF(B) is a programming langauge for describing types in UBF(A) > and protocols between clients and servers. UBF(B) is roughly > equivalent to to Verified XML, XML-schemas, SOAP and WDSL. > SOAP is nice sample for Parser Hook - is soap call there are some immutable fields (uri, proxy, ...) and some mutable fields (parameters). So with hook is possible to write module Soap SELECT soap_call('10.0.0.1/blabla/' as uri, 'calculate' as func, 10 as p1, 20 as p2) Regards Pavel Stehule
On Thu, 2009-07-30 at 00:01 -0400, Robert Haas wrote: > The JSON transformation provides functionality which is very similar > to what we also offer for XML. I sort of think we ought to just > provide that, rather than making it an add-on. I have found it to be > a tremendously attractive alternative to XML. It's worthwhile to think about how we can fit our special cases into general APIs -- particularly when we have two similar special cases like JSON and XML. Regards,Jeff Davis
On Mon, 2009-04-20 at 18:53 +0200, Pavel Stehule wrote: > b) it allows constructors for data types (ANSI SQL) > > datatype(typefield1[, typefiedl2[, typefiedl3[, ...]]]) returns type Can you describe this case in more detail? What section of SQL are you referring to? Regards,Jeff Davis
On Sat, Aug 8, 2009 at 9:11 PM, Jeff Davis<pgsql@j-davis.com> wrote: > On Thu, 2009-07-30 at 00:01 -0400, Robert Haas wrote: >> The JSON transformation provides functionality which is very similar >> to what we also offer for XML. I sort of think we ought to just >> provide that, rather than making it an add-on. I have found it to be >> a tremendously attractive alternative to XML. > > It's worthwhile to think about how we can fit our special cases into > general APIs -- particularly when we have two similar special cases like > JSON and XML. I agree. The way we handle XML is with special syntax that is hard-coded into the parser. If there is a more flexible solution I'm all for it, but I'm not sure this is it. ...Robert
On Sun, 2009-07-26 at 15:29 +0200, Pavel Stehule wrote: > Hello > > new patch add new contrib "transformations" with three modules > anotation, decode and json. > > These modules are ported from my older work. > > Before applying this patch, please use named-fixed patch too. The hook > doesn't need it, but modules anotation and json depend on it. This is not a complete review of the patches, but I have read through the discussion and taken a brief look at the code from a use-case point of view (not a technical review). My general feeling for the use case of the patch is positive. Pavel showed a reasonable variety of valid use cases, and the possibility to make existing special cases (like XML) no longer special cases. However, there are causes for concern: 1. Robert Haas is concerned that the kind of transformations allowed might be too limited: http://archives.postgresql.org/pgsql-hackers/2009-07/msg01947.php 2. Tom Lane is concerned about multiple hooks working together: http://archives.postgresql.org/pgsql-hackers/2009-04/msg01038.php 3. All throughout the thread, there is a general concern that this might not be exactly the right solution. I think we need to wait on this patch. Waiting will hopefully provide better answers to the following questions: * What other similar features exist in the SQL spec that require a similar special case now? If we added this hook, would those still require a special case? * Can anyone think of a better hook or API change that would answer these use cases? * Can anyone think of other features that almost fit this model, but that the hook won't quite work for? * If the hook can implement XML, should we refactor the XML support (and COALESCE, etc.) to use the hook for the sake of consistency? If the hook is not good enough for those features, that might indicate a problem. Considering that the next commitfest is only about a month away, I don't think that it is too much of a burden to wait. I didn't have time to do a complete review, so I can't provide much better direction than this right now. Regards,Jeff Davis
Jeff Davis escribió: > On Mon, 2009-04-20 at 18:53 +0200, Pavel Stehule wrote: > > b) it allows constructors for data types (ANSI SQL) > > > > datatype(typefield1[, typefiedl2[, typefiedl3[, ...]]]) returns type > > Can you describe this case in more detail? What section of SQL are you > referring to? Hmm, I see them in 4.7 "user-defined types". However what's in SQL2003 and the 2008 draft I have is: 3.1.6.6 constructor function: A niladic SQL-invoked function of which exactly one is implicitly specified for every structured type. An invocation of the constructor function for data type T returns a value V of the most specific type T such that V is not null and, for every observer function O defined for T, the invocation O(V) returns the default value of the attribute corresponding to O. and later: 4.7.4 Constructors Associated with each structured type ST is one implicitly defined constructor function, if and only if ST is instantiable. Let TN be the name of a structured type T. The signature of the constructor function for T is TN() and its result data type is T. The invocation TN() returns a value V such that V is not null and, for every attribute A of T, A(V) returns the default value of A. The most specific type of V is T. For every structured type ST that is instantiable, zero or more SQL-invoked constructor methods can be specified. The names of those methods shall be equivalent to the name of the type for which they are specified. So I'm not seeing those typefields anywhere. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
2009/8/9 Alvaro Herrera <alvherre@commandprompt.com>: > Jeff Davis escribió: >> On Mon, 2009-04-20 at 18:53 +0200, Pavel Stehule wrote: >> > b) it allows constructors for data types (ANSI SQL) >> > >> > datatype(typefield1[, typefiedl2[, typefiedl3[, ...]]]) returns type >> >> Can you describe this case in more detail? What section of SQL are you >> referring to? > > Hmm, I see them in 4.7 "user-defined types". However what's in SQL2003 > and the 2008 draft I have is: > > 3.1.6.6 constructor function: A niladic SQL-invoked function of which exactly > one is implicitly specified for every structured type. An invocation of the > constructor function for data type T returns a value V of the most specific > type T such that V is not null and, for every observer function O defined for > T, the invocation O(V) returns the default value of the attribute corresponding > to O. > > and later: > > 4.7.4 Constructors > Associated with each structured type ST is one implicitly defined constructor > function, if and only if ST is instantiable. > Let TN be the name of a structured type T. The signature of the constructor > function for T is TN() and its result data type is T. The invocation TN() > returns a value V such that V is not null and, for every attribute A of T, A(V) > returns the default value of A. The most specific type of V is T. > For every structured type ST that is instantiable, zero or more SQL-invoked > constructor methods can be specified. The names of those methods shall be > equivalent to the name of the type for which they are specified. > yes - it is Thank You > So I'm not seeing those typefields anywhere. > > -- > Alvaro Herrera http://www.CommandPrompt.com/ > PostgreSQL Replication, Consulting, Custom Development, 24x7 support >
2009/8/9 Jeff Davis <pgsql@j-davis.com>: > On Sun, 2009-07-26 at 15:29 +0200, Pavel Stehule wrote: >> Hello >> >> new patch add new contrib "transformations" with three modules >> anotation, decode and json. >> >> These modules are ported from my older work. >> >> Before applying this patch, please use named-fixed patch too. The hook >> doesn't need it, but modules anotation and json depend on it. > > This is not a complete review of the patches, but I have read through > the discussion and taken a brief look at the code from a use-case point > of view (not a technical review). > > My general feeling for the use case of the patch is positive. Pavel > showed a reasonable variety of valid use cases, and the possibility to > make existing special cases (like XML) no longer special cases. > > However, there are causes for concern: > > 1. Robert Haas is concerned that the kind of transformations allowed > might be too limited: > > http://archives.postgresql.org/pgsql-hackers/2009-07/msg01947.php gram.y is hard limit of everything what we can do in parser. I thing so there is possible mix two grams together (like enterprisedb do it - integration plpgsql), but still first gram has to have some static entry points - we can't do define new keyword and cannot define new rules, because all is hardly static. It is bison limit. And without changes parser's engine we cannot do some more. I see some possibility in future - to add some like preprocessor for main parser, or postprocessor (for badly processed statements). These creatures allows to define new SQL statement pseudo integrated to core. But this is different task absolutely independent to function transformation hook. But I don't thing so this is real limit. Really I don't would to create new SQL statements now. With hook I am able to work with param list and named param list. This allows lot of games over standard function syntax. > > 2. Tom Lane is concerned about multiple hooks working together: > > http://archives.postgresql.org/pgsql-hackers/2009-04/msg01038.php > with well written hooks it isn't problem. You can check sample hooks together. I agree, so bad hook can be wrong, but this is general problem of all hooks in postgresql (all hooks in the world). > 3. All throughout the thread, there is a general concern that this might > not be exactly the right solution. > > I think we need to wait on this patch. Waiting will hopefully provide > better answers to the following questions: > > * What other similar features exist in the SQL spec that require a > similar special case now? If we added this hook, would those still > require a special case? > > * Can anyone think of a better hook or API change that would answer > these use cases? > If somebody find any general solution different than hook I for it. > * Can anyone think of other features that almost fit this model, but > that the hook won't quite work for? > > * If the hook can implement XML, should we refactor the XML support (and > COALESCE, etc.) to use the hook for the sake of consistency? If the hook > is not good enough for those features, that might indicate a problem. > Some XML functions (not all) and COALESCE should be refactorized. But the range for hook is external modules. It's same like executor hooks or some other hooks in PostgreSQL. It's more readable to use direct access to code than hooks when it's possible. > Considering that the next commitfest is only about a month away, I don't > think that it is too much of a burden to wait. > ok I agree. Pavel > I didn't have time to do a complete review, so I can't provide much > better direction than this right now. > > Regards, > Jeff Davis > >
On Sunday 09 August 2009 05:21:48 Jeff Davis wrote: > * If the hook can implement XML, should we refactor the XML support (and > COALESCE, etc.) to use the hook for the sake of consistency? If the hook > is not good enough for those features, that might indicate a problem. Well, for 8.4, I proposed to rewrite xmlconcat, which is currently part of that hardcoded XML support, into a variadic function. That was shot down for some unclear backwards compatibility reason. (I guess, someone might have created their own xmlconcat function in a public schema and would now be surprised that it's actually callable?!?) With that in mind, what chances of success will a plan have that proposes to reimplement a bunch of core functionality like COALESCE in user space? Another example that was mentioned during PGCon and that these hooks may or may not be useful for is somehow de-hardcoding various SQL-standard parentheses-less functions such as current_timestamp (thus opening the door for implementing Oracle's sysdate in userspace), but it's again unclear to me whether that would not be objected to if those functions became subject to the schema search path.
2009/8/10 Peter Eisentraut <peter_e@gmx.net>: > On Sunday 09 August 2009 05:21:48 Jeff Davis wrote: >> * If the hook can implement XML, should we refactor the XML support (and >> COALESCE, etc.) to use the hook for the sake of consistency? If the hook >> is not good enough for those features, that might indicate a problem. > > Well, for 8.4, I proposed to rewrite xmlconcat, which is currently part of > that hardcoded XML support, into a variadic function. That was shot down for > some unclear backwards compatibility reason. (I guess, someone might have > created their own xmlconcat function in a public schema and would now be > surprised that it's actually callable?!?) With that in mind, what chances of > success will a plan have that proposes to reimplement a bunch of core > functionality like COALESCE in user space? > > Another example that was mentioned during PGCon and that these hooks may or > may not be useful for is somehow de-hardcoding various SQL-standard > parentheses-less functions such as current_timestamp (thus opening the door > for implementing Oracle's sysdate in userspace), but it's again unclear to me > whether that would not be objected to if those functions became subject to the > schema search path. > This patch doesn't help with it. But I thing so we will have other hook in transformation - column name. This hook will serve for detection plpgsql variables in SQL statement. And this hook should be used for some parentheses-less functions. regards Pavel >
Peter Eisentraut <peter_e@gmx.net> wrote: > reimplement a bunch of core functionality like COALESCE If such an effort could reduce the astonishment factor for the following, it would justify a certain amount of effort, in my view: test=# select pg_typeof('x');pg_typeof -----------unknown (1 row) test=# select pg_typeof(null);pg_typeof -----------unknown (1 row) test=# select pg_typeof(coalesce(null, null));pg_typeof -----------text (1 row) We now have workarounds in place for everywhere this bit us on conversion to PostgreSQL, but it was actually one of the greater sources of pain in that process.... -Kevin
"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes: > Peter Eisentraut <peter_e@gmx.net> wrote: >> reimplement a bunch of core functionality like COALESCE > If such an effort could reduce the astonishment factor for the > following, it would justify a certain amount of effort, in my view: > test=# select pg_typeof('x'); > pg_typeof > ----------- > unknown > (1 row) > test=# select pg_typeof(null); > pg_typeof > ----------- > unknown > (1 row) > test=# select pg_typeof(coalesce(null, null)); > pg_typeof > ----------- > text > (1 row) The astonishment factor there has nothing to do with how the behavior is inserted into the parser; it's a property of our type resolution rules. regards, tom lane
On Mon, Aug 10, 2009 at 5:54 PM, Kevin Grittner<Kevin.Grittner@wicourts.gov> wrote: > > We now have workarounds in place for everywhere this bit us on > conversion to PostgreSQL, but it was actually one of the greater > sources of pain in that process.... Given that pg_typeof() is a relatively new and pg-specific piece of machinery how did this bite you on on your conversion to Postgres some years ago? -- greg http://mit.edu/~gsstark/resume.pdf
Greg Stark <gsstark@mit.edu> wrote: > Given that pg_typeof() is a relatively new and pg-specific piece of > machinery how did this bite you on on your conversion to Postgres > some years ago? It wasn't the use of pg_typeof which caused us problems, but the types the example demonstrated. Primarily that bit us when our framework substituted values from the application or user selection windows into complex queries, with the result that a coalesce of two NULLs was used in a context where numbers or dates were expected. Our initial hack, which got us up and running fine, was to modify the JDBC driver to substitute a bare NULL for the COALESCE of two NULLs in the JDBC compatibility code which mapped to COALESCE. As a longer- term, less fragile fix we pushed type information deeper into the code making the JDBC requests and had it explicitly wrap a NULL with a CAST. Still, it rates pretty high on my astonishment scale that a COALESCE of two untyped NULLs (or for that matter, any two values of unknown type) returns a text value. It's one of those things which apparently seems unsurprising for those viewing the product from the inside out. -Kevin
"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes: > Still, it rates pretty high on my astonishment scale that a > COALESCE of two untyped NULLs (or for that matter, any two values of > unknown type) returns a text value. What would you have it do instead, throw an error? The current behavior is a lot less astonishing for this example:COALESCE('a', 'b') which is the same from the type system's point of view. regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> wrote: > "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes: >> Still, it rates pretty high on my astonishment scale that a >> COALESCE of two untyped NULLs (or for that matter, any two values >> of unknown type) returns a text value. > > What would you have it do instead, throw an error? Return a value of unknown type. > The current behavior is a lot less astonishing for this example: > COALESCE('a', 'b') > which is the same from the type system's point of view. I understand that it is. I see that as a flaw in the implementation. It would surprise me less if the above resulted in exactly the same value and type as a bare 'a'. -Kevin
"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes: > Tom Lane <tgl@sss.pgh.pa.us> wrote: >> "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes: >>> Still, it rates pretty high on my astonishment scale that a >>> COALESCE of two untyped NULLs (or for that matter, any two values >>> of unknown type) returns a text value. >> >> What would you have it do instead, throw an error? > Return a value of unknown type. That would require doing actual computation on values of unknown type. In the specific case of COALESCE, we could theoretically do that, since the only computation it needs is "IS NULL" which is datatype-independent. In most situations, however, you can't evaluate the function without knowledge of the datatype semantics. As an example, consider NULLIF('0', '00'). This gives different answers if you suppose the literals are text than if you suppose they are integers. So yeah, we could make COALESCE into a special-case wart in the type system and have it able to execute without inferring a type for the arguments. I don't think that would be a net improvement in the system's astonishment quotient, however; people would just be confused why COALESCE behaves differently from everything else. regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> wrote: > In the specific case of COALESCE, we could theoretically do that, > since the only computation it needs is "IS NULL" which is > datatype-independent. Well, in the SQL specification, COALESCE is defined as an abbreviation of the CASE predicate, so to the extent that anyone pays attention to the spec, this: COALESCE(a, b) should be treated identically to: CASE WHEN a IS NULL THEN a ELSE b END > In most situations, however, you can't evaluate the function without > knowledge of the datatype semantics. As an example, consider > NULLIF('0', '00'). This gives different answers if you suppose the > literals are text than if you suppose they are integers. That is the other CASE abbreviation. (The only other one.) So, according to how I read the spec, it should be identical to CASE WHEN '0' = '00' THEN NULL ELSE '0' END > So yeah, we could make COALESCE into a special-case wart in the type > system and have it able to execute without inferring a type for the > arguments. I don't think that would be a net improvement in the > system's astonishment quotient, however; people would just be > confused why COALESCE behaves differently from everything else. Not if they notice that COALESCE and NULLIF are documented (quite properly) on the "conditional expressions" page, along with the CASE predicate: http://www.postgresql.org/docs/8.4/interactive/functions-conditional.html It is probably a poor choice on the part of the standards committee to implement these abbreviations for the CASE predicate in a way the causes them to look so much like functions. -Kevin
I wrote: > COALESCE(a, b) > > should be treated identically to: > > CASE WHEN a IS NULL THEN a ELSE b END In case it's not obvious that the above has a typo, I meant: CASE WHEN a IS NOT NULL THEN a ELSE b END -Kevin
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. 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. 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. regards, tom lane
"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes: > Tom Lane <tgl@sss.pgh.pa.us> wrote: >> In the specific case of COALESCE, we could theoretically do that, >> since the only computation it needs is "IS NULL" which is >> datatype-independent. > Well, in the SQL specification, COALESCE is defined as an abbreviation > of the CASE predicate, so to the extent that anyone pays attention to > the spec, this: > COALESCE(a, b) > should be treated identically to: > CASE WHEN a IS NULL THEN a ELSE b END ... as indeed we do. That CASE will be handled the same way as the COALESCE is, ie, resolve as text output for lack of a better idea. >> In most situations, however, you can't evaluate the function without >> knowledge of the datatype semantics. As an example, consider >> NULLIF('0', '00'). This gives different answers if you suppose the >> literals are text than if you suppose they are integers. > That is the other CASE abbreviation. (The only other one.) So, > according to how I read the spec, it should be identical to > CASE WHEN '0' = '00' THEN NULL ELSE '0' END Yes, and you're begging the question: what are the semantics of that = operator? Without imputing a datatype to the literals, you can't resolve it. > It is probably a poor choice on the part of the standards committee to > implement these abbreviations for the CASE predicate in a way the > causes them to look so much like functions. Whether it's a function has nothing to do with this. It's a question of datatype-dependent semantics, and it would be the same no matter what the visual appearance of the constructs was. regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> wrote: > "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes: [Correcting typo below.] >> Well, in the SQL specification, COALESCE is defined as an >> abbreviation of the CASE predicate, so to the extent that anyone >> pays attention to the spec, this: >> COALESCE(a, b) >> should be treated identically to: >> CASE WHEN a IS [NOT] NULL THEN a ELSE b END > > ... as indeed we do. That CASE will be handled the same way as the > COALESCE is, ie, resolve as text output for lack of a better idea. I'm surprised to find that CASE behaves this way, too. At least there's an internal consistency to this, even if I think it's wrong on all counts. test=# select pg_typeof(case when null is not null then null else null end);pg_typeof -----------text (1 row) I think the better idea is to say that the type is still unknown. >> That is the other CASE abbreviation. (The only other one.) So, >> according to how I read the spec, it should be identical to >> CASE WHEN '0' = '00' THEN NULL ELSE '0' END > > Yes, and you're begging the question: what are the semantics > of that = operator? Without imputing a datatype to the literals, > you can't resolve it. Yeah -- my argument would be that the = operator in NULLIF should be treated the same as if the function-like abbreviation were rewritten to the full CASE predicate. It doesn't surprise me that that is taken as text, given that they are both unadorned character string literals. The surprise here (for me at least) that the following generates a null of type text instead of matching the non-NULL input argument or (failing that) unknown, assuming the rewrite of NULLIF(a, b) to the equivalent CASE predicate: test=# select pg_typeof(case when null = 0 then null else null end);pg_typeof -----------text (1 row) Frankly, I'm dubious about treating a character string literal as being of unknown type in the first place, but I can see where it is a useful convenience. Where the wheels really come off for me is in automagically going from unknown type to text on any form of CASE predicate. -Kevin
"Kevin Grittner" <Kevin.Grittner@wicourts.gov> wrote: > Yeah -- my argument would be that the = operator in NULLIF should be > treated the same as if the function-like abbreviation were rewritten > to the full CASE predicate. It doesn't surprise me that that is > taken as text, given that they are both unadorned character string > literals. The surprise here (for me at least) that the following > generates a null of type text instead of matching the non-NULL input > argument or (failing that) unknown, assuming the rewrite of > NULLIF(a, b) to the equivalent CASE predicate: > > test=# select pg_typeof(case when null = 0 then null else null end); > pg_typeof > ----------- > text > (1 row) Symmetry fails here -- NULLIF is *not* treated the same as the CASE predicate for which it is the abbreviation, which is arguably a bug-level deviation from the SQL standard. Compare the above to: test=# select nullif(null, 0);nullif -------- (1 row) -Kevin
Resending to correct a copy/paste error. Apologies. "Kevin Grittner" <Kevin.Grittner@wicourts.gov> wrote: > Yeah -- my argument would be that the = operator in NULLIF should be > treated the same as if the function-like abbreviation were rewritten > to the full CASE predicate. It doesn't surprise me that that is > taken as text, given that they are both unadorned character string > literals. The surprise here (for me at least) that the following > generates a null of type text instead of matching the non-NULL input > argument or (failing that) unknown, assuming the rewrite of > NULLIF(a, b) to the equivalent CASE predicate: > > test=# select pg_typeof(case when null = 0 then null else null end); > pg_typeof > ----------- > text > (1 row) Symmetry fails here -- NULLIF is *not* treated the same as the CASE predicate for which it is the abbreviation, which is arguably a bug-level deviation from the SQL standard. Compare the above to: test=# select pg_typeof(nullif(null, 0));pg_typeof -----------integer (1 row) Which is the result I would want and expect, but is inconsistent with treating it as an abbreviation of CASE. -Kevin
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. please, show it. regards Pavel Stehule 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. > > 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. > > 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. > > regards, tom lane >
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. > > regards, tom lane >
On Mon, Aug 10, 2009 at 03:43:45PM -0400, Tom Lane wrote: > "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes: > > Tom Lane <tgl@sss.pgh.pa.us> wrote: > >> "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes: > >>> Still, it rates pretty high on my astonishment scale that a > >>> COALESCE of two untyped NULLs (or for that matter, any two values > >>> of unknown type) returns a text value. > >> > >> What would you have it do instead, throw an error? > > > Return a value of unknown type. > > That would require doing actual computation on values of unknown type. A better way would be to say it's of polymorphic type. PG's support of polymorphism is currently a bit ad-hoc, but this would be something I'd love to change. It would be quite a big change and I've not thought through all the details yet. > In the specific case of COALESCE, we could theoretically do that, > since the only computation it needs is "IS NULL" which is > datatype-independent. Yes, this would be the only valid operator I can see working. COUNT would work as an aggregate. > In most situations, however, you can't evaluate > the function without knowledge of the datatype semantics. As an > example, consider NULLIF('0', '00'). This gives different answers if > you suppose the literals are text than if you suppose they are integers. Yup, which is when it gets fun and I think would mean we'd end up throwing out a few more queries as ambiguous if I had my way! As long as there was *one* type in the above expression then it would be OK, for example it would be unambiguous in either of the following cases: SELECT NULLIF(INT '0', '00'); SELECT NULLIF('0', INT '00'); and I'd also like the following to be OK: SELECT NULLIF('0', '00') + 5; SELECT n+5 FROM (SELECT NULLIF('0', '00')) x(n); But PG currently throws these out as it's type resolution (also known as type unification) is too eager. The same arguments would obviously apply to any polymorphic function. For example, I'd expect to be able to do: SELECT ('{1,2}')[1] + 5; and have PG figure out that the literal is of type INT[]. Not sure what ambiguity is being prevented that causes PG to need the brackets, but that's a side issue. It also raises the issue of the fact that there's no general way to ascribe types in PG. You can cast (using a couple of different syntaxes) but this isn't the same as type ascription. For example, I'd like to be able to do things like: SELECT NULLIF('0', '00')::INT + 5; But I'm doing a cast here, I'm not saying that the NULLIF function evaluates to a value of type INT which is what I want to be doing. So this currently results in 5 being returned and not NULL as I really want. The above obviously isn't the syntax to use as it would break code, but the functionality would be useful. -- Sam http://samason.me.uk/
On Tue, Aug 11, 2009 at 6:35 AM, Sam Mason<sam@samason.me.uk> wrote: > On Mon, Aug 10, 2009 at 03:43:45PM -0400, Tom Lane wrote: >> "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes: >> > Tom Lane <tgl@sss.pgh.pa.us> wrote: >> >> "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes: >> >>> Still, it rates pretty high on my astonishment scale that a >> >>> COALESCE of two untyped NULLs (or for that matter, any two values >> >>> of unknown type) returns a text value. >> >> >> >> What would you have it do instead, throw an error? >> >> > Return a value of unknown type. >> >> That would require doing actual computation on values of unknown type. > > A better way would be to say it's of polymorphic type. PG's support of > polymorphism is currently a bit ad-hoc, but this would be something I'd > love to change. It would be quite a big change and I've not thought > through all the details yet. > >> In the specific case of COALESCE, we could theoretically do that, >> since the only computation it needs is "IS NULL" which is >> datatype-independent. > > Yes, this would be the only valid operator I can see working. COUNT > would work as an aggregate. > >> In most situations, however, you can't evaluate >> the function without knowledge of the datatype semantics. As an >> example, consider NULLIF('0', '00'). This gives different answers if >> you suppose the literals are text than if you suppose they are integers. > > Yup, which is when it gets fun and I think would mean we'd end up > throwing out a few more queries as ambiguous if I had my way! > > As long as there was *one* type in the above expression then it would > be OK, for example it would be unambiguous in either of the following > cases: > > SELECT NULLIF(INT '0', '00'); > SELECT NULLIF('0', INT '00'); > > and I'd also like the following to be OK: > > SELECT NULLIF('0', '00') + 5; > SELECT n+5 FROM (SELECT NULLIF('0', '00')) x(n); > > But PG currently throws these out as it's type resolution (also known > as type unification) is too eager. The same arguments would obviously > apply to any polymorphic function. For example, I'd expect to be able > to do: > > SELECT ('{1,2}')[1] + 5; > > and have PG figure out that the literal is of type INT[]. Not sure what > ambiguity is being prevented that causes PG to need the brackets, but > that's a side issue. > > It also raises the issue of the fact that there's no general way > to ascribe types in PG. You can cast (using a couple of different > syntaxes) but this isn't the same as type ascription. For example, I'd > like to be able to do things like: > > SELECT NULLIF('0', '00')::INT + 5; > > But I'm doing a cast here, I'm not saying that the NULLIF function > evaluates to a value of type INT which is what I want to be doing. So > this currently results in 5 being returned and not NULL as I really > want. The above obviously isn't the syntax to use as it would break > code, but the functionality would be useful. What you're talking about here is called "type inference". http://en.wikipedia.org/wiki/Type_inference ...Robert
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".
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