Thread

1. WIP patch: collation assignment algorithm rewrite

   Tom Lane <tgl@sss.pgh.pa.us> — 2011-03-19T21:22:39Z

   Attached is a WIP patch to split the expression-tree representation of
   collations into separate fields for input and output collation, and to
   replace the parser's current method of assigning collations with a
   recursive post-pass.  I need to read over it again before committing,
   but I think it's pretty close to ready.
   
   One slightly annoying aspect of this change is that there are assorted
   utility commands that call transformExpr() for themselves, and these
   places now have to remember to call assign_expr_collations() as well.
   I thought about pushing the collations call into transformExpr(), but
   most of those call sites also call coerce_to_boolean or other functions
   that modify the output of transformExpr, and the collation traversal
   has to happen after that.  In any case, the parser uses transformExpr
   recursively internally, and a whole lot of further changes would have
   been needed to make those internal calls go to a separate entry point.
   
   Other than that it seemed to go pretty smoothly.  One thing I was happy
   about was that after adding an output-collation field to RelabelType,
   it was possible for the planner to get rid of CollateExpr nodes by
   rewriting them as RelabelType.  That means that the existing logic for
   ignoring RelabelType when comparing subexpressions is sufficient to
   cover cases involving COLLATE labels that might appear on only some of
   the subexpressions.  Much of the planner was pretty broken for such
   cases before, but I think it mostly works now.
   
   Any comments?
   
   			regards, tom lane
   
   

2. Re: WIP patch: collation assignment algorithm rewrite

   Martijn van Oosterhout <kleptog@svana.org> — 2011-03-20T19:44:43Z

   On Sat, Mar 19, 2011 at 05:22:39PM -0400, Tom Lane wrote:
   > Attached is a WIP patch to split the expression-tree representation of
   > collations into separate fields for input and output collation, and to
   > replace the parser's current method of assigning collations with a
   > recursive post-pass.  I need to read over it again before committing,
   > but I think it's pretty close to ready.
   
   I've looked over the main code and it looks good. While I'm not totally
   conviced it has to be done as a seperate pass, this way is exceedingly
   readable and clear as to what is going on, which makes me much more
   confident of its correctness.
   
   Thanks for going over this.
   
   Have a nice day,
   -- 
   Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
   > Patriotism is when love of your own people comes first; nationalism,
   > when hate for people other than your own comes first. 
   >                                       - Charles de Gaulle
   

3. Re: WIP patch: collation assignment algorithm rewrite

   Tom Lane <tgl@sss.pgh.pa.us> — 2011-03-21T03:07:51Z

   Martijn van Oosterhout <kleptog@svana.org> writes:
   > I've looked over the main code and it looks good. While I'm not totally
   > conviced it has to be done as a seperate pass, this way is exceedingly
   > readable and clear as to what is going on, which makes me much more
   > confident of its correctness.
   
   FWIW, I'm not at all confident that it's "correct".  What I do think
   is that this method will make it a lot easier to tweak the behavioral
   details until we like them ...
   
   			regards, tom lane