Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-29 Thread Noah Misch
On Fri, Jan 28, 2011 at 04:49:39PM -0500, Noah Misch wrote: On Fri, Jan 28, 2011 at 01:52:32PM -0500, Robert Haas wrote: On Wed, Jan 26, 2011 at 6:06 PM, Tom Lane t...@sss.pgh.pa.us wrote: I'm not sure how important that concern is though, because it's hard to see how any such change

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-29 Thread Robert Haas
On Fri, Jan 28, 2011 at 4:49 PM, Noah Misch n...@leadboat.com wrote: I'm not necessarily signing on to the viewpoint that we should wait to do any of this work until after we refactor typemods, but it does strike me that the fact that Tom and I have completely different ideas of how this will

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-29 Thread Robert Haas
On Sat, Jan 29, 2011 at 3:26 AM, Noah Misch n...@leadboat.com wrote: One other thing: #7 does not depend on #3,4,5,6 or any design problems raised thus far, so there's no need to treat it the same as that group. Well, if you want to post an updated patch that's independent of the rest of the

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-28 Thread Robert Haas
On Wed, Jan 26, 2011 at 6:06 PM, Tom Lane t...@sss.pgh.pa.us wrote: I'm not sure how important that concern is though, because it's hard to see how any such change wouldn't break existing cast implementation functions anyway.  If the API for length-coercing cast functions changes, breaking

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-28 Thread Noah Misch
On Fri, Jan 28, 2011 at 01:52:32PM -0500, Robert Haas wrote: On Wed, Jan 26, 2011 at 6:06 PM, Tom Lane t...@sss.pgh.pa.us wrote: I'm not sure how important that concern is though, because it's hard to see how any such change wouldn't break existing cast implementation functions anyway. ?If

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-27 Thread Robert Haas
On Thu, Jan 27, 2011 at 1:14 AM, Noah Misch n...@leadboat.com wrote: Based on downthread discussion, I figure this will all change a good deal.   I'll still briefly explain the patch as written.  Most of the patch is plumbing to support the new syntax, catalog entries, and FuncExpr field.  The

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-27 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: OK. I was thinking that instead moving this into eval_const_expressions(), we just make the logic in find_coercion_pathway() call the exemptor function (or whatever we call it) right around here: No, no, no, no. Didn't you read yesterday's

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-27 Thread Robert Haas
On Thu, Jan 27, 2011 at 9:46 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: OK. I was thinking that instead moving this into eval_const_expressions(), we just make the logic in find_coercion_pathway() call the exemptor function (or whatever we call it) right

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-27 Thread Robert Haas
On Thu, Jan 27, 2011 at 10:42 AM, Robert Haas robertmh...@gmail.com wrote: On Thu, Jan 27, 2011 at 9:46 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: OK. I was thinking that instead moving this into eval_const_expressions(), we just make the logic in

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-27 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: Is it not OK for me to have a different idea about which way to go with something than you do? Well, in this case I firmly believe that you're mistaken. My personal view is that we ought to try to be increasing the number of places where type

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-27 Thread Robert Haas
On Thu, Jan 27, 2011 at 11:35 AM, Tom Lane t...@sss.pgh.pa.us wrote: My personal view is that we ought to try to be increasing the number of places where type modifiers behave like they're really part of the type, rather than being an afterthought that we deal with when convenient and

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-27 Thread Noah Misch
On Thu, Jan 27, 2011 at 09:02:26AM -0500, Robert Haas wrote: OK. I was thinking that instead moving this into eval_const_expressions(), we just make the logic in find_coercion_pathway() call the exemptor function (or whatever we call it) right around here: switch

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Robert Haas
On Mon, Jan 24, 2011 at 11:13 PM, Noah Misch n...@leadboat.com wrote: This helps on conversions like varchar(4)-varchar(8) and text-xml. I've read through this patch somewhat.  As I believe Tom also commented previously, exemptor is a pretty bad choice of name. I spent awhile with a

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: ... A side issue is that I really want to avoid adding a new parser keyword for this. It doesn't bother me too much to add keywords for really important and user-visible features, but when we're adding stuff that's only going to be used by 0.1% of

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Robert Haas
On Wed, Jan 26, 2011 at 12:47 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: ... A side issue is that I really want to avoid adding a new parser keyword for this.  It doesn't bother me too much to add keywords for really important and user-visible features,

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Wed, Jan 26, 2011 at 12:47 PM, Tom Lane t...@sss.pgh.pa.us wrote: If you didn't mind inverting the sense of the result then we could use EXECUTE IF function_name. What about borrowing from the trigger syntax? WITH FUNCTION function_name

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Robert Haas
On Wed, Jan 26, 2011 at 3:08 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Wed, Jan 26, 2011 at 12:47 PM, Tom Lane t...@sss.pgh.pa.us wrote: If you didn't mind inverting the sense of the result then we could use EXECUTE IF function_name. What about

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Wed, Jan 26, 2011 at 3:08 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: It's not obvious to me that it has a use case outside of casts, but it's certainly possible I'm missing something. A possible example is

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Robert Haas
On Wed, Jan 26, 2011 at 4:02 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Wed, Jan 26, 2011 at 3:08 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: It's not obvious to me that it has a use case outside of casts, but it's

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Wed, Jan 26, 2011 at 4:02 PM, Tom Lane t...@sss.pgh.pa.us wrote: I don't mind confining the feature to casts to start with, but it might be a good idea to specify the check-function API in a way that would let it be plugged into a more generally

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Robert Haas
On Wed, Jan 26, 2011 at 5:01 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Wed, Jan 26, 2011 at 4:02 PM, Tom Lane t...@sss.pgh.pa.us wrote: I don't mind confining the feature to casts to start with, but it might be a good idea to specify the check-function

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: Well, if you're positive we're eventually going to want this in pg_proc, we may as well add it now. But I'm not too convinced it's the right general API. The number of people writing exactly x + 0 or x * 0 in a query has got to be vanishingly small;

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Robert Haas
On Wed, Jan 26, 2011 at 5:32 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Well, if you're positive we're eventually going to want this in pg_proc, we may as well add it now.  But I'm not too convinced it's the right general API.  The number of people writing

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: Oh, really? I was thinking the logic should go into find_coercion_pathway(). Well, I've been saying right along that it should be in eval_const_expressions. Putting this sort of optimization in the parser is invariably the wrong thing, because it fails

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Robert Haas
On Wed, Jan 26, 2011 at 5:41 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Oh, really?  I was thinking the logic should go into find_coercion_pathway(). Well, I've been saying right along that it should be in eval_const_expressions.  Putting this sort of

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Tom Lane
I wrote: ... Another issue is that premature optimization in the parser creates headaches if conditions change such that a previous optimization is no longer valid --- you may have stored rules wherein the optimization was already applied. (Not sure that specific issue applies to casting,

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Noah Misch
On Wed, Jan 26, 2011 at 05:55:37PM -0500, Tom Lane wrote: I wrote: ... Another issue is that premature optimization in the parser creates headaches if conditions change such that a previous optimization is no longer valid --- you may have stored rules wherein the optimization was already

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: Well, I guess my thought was that we what we were doing is extending the coercion system to be able to make decisions based on both type OID and typemod. Well, that's an interesting thought, but the proposal at hand is far more limited than that ---

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Tom Lane
Noah Misch n...@leadboat.com writes: On Wed, Jan 26, 2011 at 05:55:37PM -0500, Tom Lane wrote: Actually, I can construct a concrete example where applying this optimization in the parser will do the wrong thing: CREATE TABLE base (f1 varchar(4)); CREATE VIEW vv AS SELECT f1::varchar(8) FROM

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Noah Misch
On Wed, Jan 26, 2011 at 05:32:00PM -0500, Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: Well, if you're positive we're eventually going to want this in pg_proc, we may as well add it now. But I'm not too convinced it's the right general API. The number of people writing

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Tom Lane
Noah Misch n...@leadboat.com writes: If we hook this into eval_const_expressions, it definitely seems cleaner to attach the auxiliary function to the pg_proc. Otherwise, we'd reconstruct which cast led to each function call -- is there even enough information available to do so unambiguously?

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Noah Misch
On Wed, Jan 26, 2011 at 06:29:57PM -0500, Tom Lane wrote: Noah Misch n...@leadboat.com writes: If we hook this into eval_const_expressions, it definitely seems cleaner to attach the auxiliary function to the pg_proc. Otherwise, we'd reconstruct which cast led to each function call -- is

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Tom Lane
Noah Misch n...@leadboat.com writes: On Wed, Jan 26, 2011 at 06:29:57PM -0500, Tom Lane wrote: I remain completely unexcited about optimizing that case, especially if it doesn't fit into a general framework. The bang for the buck ratio is not good: too much work, too much uglification, too

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Tom Lane
Noah Misch n...@leadboat.com writes: text - xml BTW, that reminds me of something that I think was mentioned way back when, but absolutely fails to fit into any of the frameworks discussed here: the mere fact that a type is binary-compatible (with or without checking) doesn't make it compatible

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Robert Haas
On Wed, Jan 26, 2011 at 6:06 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Well, I guess my thought was that we what we were doing is extending the coercion system to be able to make decisions based on both type OID and typemod. Well, that's an interesting

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Robert Haas
On Wed, Jan 26, 2011 at 7:44 PM, Tom Lane t...@sss.pgh.pa.us wrote: But how often do those really come up?  And do you really save that much?  The table still has to be locked against other users, so you're still down, and you're still doing all the reads and computation.  I don't deny that

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Noah Misch
On Wed, Jan 26, 2011 at 07:44:43PM -0500, Tom Lane wrote: numeric(8,2) - numeric(7,2) varbit(8) - varbit(7) text - xml But how often do those really come up? I'll speak from my own experience, having little idea of the larger community experience on this one. I usually don't even

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Noah Misch
On Wed, Jan 26, 2011 at 07:52:10PM -0500, Tom Lane wrote: Noah Misch n...@leadboat.com writes: text - xml BTW, that reminds me of something that I think was mentioned way back when, but absolutely fails to fit into any of the frameworks discussed here: the mere fact that a type is

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-26 Thread Noah Misch
On Wed, Jan 26, 2011 at 09:35:24AM -0500, Robert Haas wrote: On Mon, Jan 24, 2011 at 11:13 PM, Noah Misch n...@leadboat.com wrote: This helps on conversions like varchar(4)-varchar(8) and text-xml. I've read through this patch somewhat. ?As I believe Tom also commented previously,

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-24 Thread Robert Haas
On Sun, Jan 9, 2011 at 5:03 PM, Noah Misch n...@leadboat.com wrote: Here I add the notion of an exemptor function, a property of a cast that determines when calls to the cast would be superfluous.  Such calls can be removed, reduced to RelabelType expressions, or annotated (via a new field in

Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases

2011-01-24 Thread Noah Misch
On Mon, Jan 24, 2011 at 07:18:47PM -0500, Robert Haas wrote: On Sun, Jan 9, 2011 at 5:03 PM, Noah Misch n...@leadboat.com wrote: Here I add the notion of an exemptor function, a property of a cast that determines when calls to the cast would be superfluous. ?Such calls can be removed,