I wrote: > 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; On further thought, maybe it's the special case for non-foreign Vars that is busted. That is, non-foreign Vars should do /* Var belongs to some other table */ if (var->varcollid != InvalidOid && var->varcollid != DEFAULT_COLLATION_OID) return false; - /* We can consider that it doesn't set collation */ - collation = InvalidOid; - state = FDW_COLLATE_NONE; + collation = var->varcollid; + state = OidIsValid(collation) ? FDW_COLLATE_SAFE : FDW_COLLATE_NONE; and likewise for Consts, Params, etc. This would basically mean clarifying the meaning of the state values as: FDW_COLLATE_NONE: the expression is of a noncollatable type, period. FDW_COLLATE_SAFE: the expression has default collation, or a nondefault collation that is traceable to a foreign Var. FDW_COLLATE_UNSAFE: the expression has a nondefault collation that is not traceable to a foreign Var. Hm ... actually, we probably need *both* types of changes if that's what we believe the state values mean. An alternative definition would be that FDW_COLLATE_NONE subsumes the "collation doesn't trace to a foreign Var, but it's default so we don't really care" case. I think the problem we've got is that the non-foreign-Var code thinks that's what the definition is, but the rest of the code isn't quite consistent with it. In any case I think we want to end up with a clearer specification of what the states mean. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers