On Wed, Sep 23, 2015 at 7:29 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Removing that entirely would be quite incorrect, because then you'd be lying to the parent node about what collation your node outputs.
Yes. I too thought so and thus wanted to fix that code block by considering the default collation.
After thinking a bit more about the existing special case for non-foreign Vars, I wonder if what we should do is change these code blocks to look like
collation = r->resultcollid; if (collation == InvalidOid) state = FDW_COLLATE_NONE; else if (inner_cxt.state == FDW_COLLATE_SAFE && collation == inner_cxt.collation) state = FDW_COLLATE_SAFE; + else if (collation == DEFAULT_COLLATION_OID) + state = FDW_COLLATE_NONE; else state = FDW_COLLATE_UNSAFE;
That is, only explicit introduction of a non-default collation causes a subexpression to get labeled FDW_COLLATE_UNSAFE. Default collations would lose out when getting merged with a nondefault collation from a foreign Var, so they should work all right.
Agree.
I had suggested similar changes in approach (2) but you put that check at exact required place.
regards, tom lane
--
Jeevan B Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company
