Re: [HACKERS] FuncExpr.collid/OpExpr.collid unworkably serving double duty

2011-03-15 Thread Peter Eisentraut
On tor, 2011-03-10 at 17:16 -0500, Tom Lane wrote: On the other hand ... one thing that's been bothering me is that select_common_collation assumes that explicit collation derivation doesn't bubble up in the tree, ie a COLLATE is only a forcing function for the immediate parent expression

Re: [HACKERS] FuncExpr.collid/OpExpr.collid unworkably serving double duty

2011-03-15 Thread Tom Lane
Peter Eisentraut pete...@gmx.net writes: On tor, 2011-03-10 at 17:16 -0500, Tom Lane wrote: On the other hand ... one thing that's been bothering me is that select_common_collation assumes that explicit collation derivation doesn't bubble up in the tree, ie a COLLATE is only a forcing function

Re: [HACKERS] FuncExpr.collid/OpExpr.collid unworkably serving double duty

2011-03-15 Thread Peter Eisentraut
On tis, 2011-03-15 at 16:16 -0400, Tom Lane wrote: But what did you think of the idea of setting collations during a post-pass, so that the collation derivation values need only be local storage during that one recursive routine? That sounds reasonable. We do need the collation value in the

Re: [HACKERS] FuncExpr.collid/OpExpr.collid unworkably serving double duty

2011-03-14 Thread Tom Lane
I wrote: In any case, I am sure that that what this describes is not what our current code does :-(, and that we can't get anywhere close to this with the existing infrastructure. So at this point I'm thinking that the safest approach is to rip out the result-collation caching fields and

Re: [HACKERS] FuncExpr.collid/OpExpr.collid unworkably serving double duty

2011-03-12 Thread Martijn van Oosterhout
On Fri, Mar 11, 2011 at 04:56:31PM -0500, Tom Lane wrote: In my implementation I needed to expand this to the general set of operators postgresql supported and relaxed this to only consider arguments to the function/operator that had the same type as the resulting type of the

Re: [HACKERS] FuncExpr.collid/OpExpr.collid unworkably serving double duty

2011-03-11 Thread Martijn van Oosterhout
On Thu, Mar 10, 2011 at 05:51:31PM -0500, Tom Lane wrote: No, that's not what I'm on about. Consider (((A COLLATE X) || B) || (C COLLATE Y)) (D COLLATE Z) (I've spelled out the parenthesization in full for clarity, but most of these parens could be omitted.) Is this expression

Re: [HACKERS] FuncExpr.collid/OpExpr.collid unworkably serving double duty

2011-03-11 Thread Tom Lane
Martijn van Oosterhout klep...@svana.org writes: On Thu, Mar 10, 2011 at 05:51:31PM -0500, Tom Lane wrote: No, that's not what I'm on about. Consider (((A COLLATE X) || B) || (C COLLATE Y)) (D COLLATE Z) The rules are essentially as described here:

Re: [HACKERS] FuncExpr.collid/OpExpr.collid unworkably serving double duty

2011-03-10 Thread Martijn van Oosterhout
On Wed, Mar 09, 2011 at 04:49:28PM -0500, Tom Lane wrote: So I was moving some error checks around and all of a sudden the regression tests blew up on me, with lots of errors about how type X didn't support collations (which indeed it didn't). After some investigation I realized what should

Re: [HACKERS] FuncExpr.collid/OpExpr.collid unworkably serving double duty

2011-03-10 Thread Tom Lane
Martijn van Oosterhout klep...@svana.org writes: On Wed, Mar 09, 2011 at 04:49:28PM -0500, Tom Lane wrote: There are basically two things we could do about this: 1. Add two fields not one to nodes representing function/operator calls. 2. Change exprCollation() to do a type_is_collatable

Re: [HACKERS] FuncExpr.collid/OpExpr.collid unworkably serving double duty

2011-03-10 Thread Martijn van Oosterhout
On Thu, Mar 10, 2011 at 10:34:00AM -0500, Tom Lane wrote: Hmm. That suggests a third solution: revert the addition of *all* the collid fields except the ones that represent collation-to-apply-during- function-execution. (So they'd still be there in FuncExpr/OpExpr, but not most other

Re: [HACKERS] FuncExpr.collid/OpExpr.collid unworkably serving double duty

2011-03-10 Thread Tom Lane
Martijn van Oosterhout klep...@svana.org writes: On Thu, Mar 10, 2011 at 10:34:00AM -0500, Tom Lane wrote: I suspect this is probably not a good idea because of the added cost in select_common_collation: aside from probably needing more syscache lookups, there's a potential for

Re: [HACKERS] FuncExpr.collid/OpExpr.collid unworkably serving double duty

2011-03-10 Thread Tom Lane
I wrote: A post-pass is not out of the question, but it's enough unlike everything else the parser does that I'm not too thrilled about it. On the other hand ... one thing that's been bothering me is that select_common_collation assumes that explicit collation derivation doesn't bubble up in

Re: [HACKERS] FuncExpr.collid/OpExpr.collid unworkably serving double duty

2011-03-10 Thread Martijn van Oosterhout
On Thu, Mar 10, 2011 at 05:16:52PM -0500, Tom Lane wrote: On the other hand ... one thing that's been bothering me is that select_common_collation assumes that explicit collation derivation doesn't bubble up in the tree, ie a COLLATE is only a forcing function for the immediate parent

Re: [HACKERS] FuncExpr.collid/OpExpr.collid unworkably serving double duty

2011-03-10 Thread Tom Lane
Martijn van Oosterhout klep...@svana.org writes: On Thu, Mar 10, 2011 at 05:16:52PM -0500, Tom Lane wrote: On the other hand ... one thing that's been bothering me is that select_common_collation assumes that explicit collation derivation doesn't bubble up in the tree, ie a COLLATE is only a

[HACKERS] FuncExpr.collid/OpExpr.collid unworkably serving double duty

2011-03-09 Thread Tom Lane
So I was moving some error checks around and all of a sudden the regression tests blew up on me, with lots of errors about how type X didn't support collations (which indeed it didn't). After some investigation I realized what should have been apparent much earlier: the collations patch is trying