ExecBuildGroupingEqual versus collations

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: ExecBuildGroupingEqual versus collations
Date: 2018-12-14 19:25:30
Message-ID: 21825.1544815530@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

In pursuit of places that might not be on board with non-default
collations, I tried sticking

Assert(PG_GET_COLLATION() == C_COLLATION_OID);

into nameeq() and the other name comparison functions, in my build with
type name's typcollation changed to "C". I'm down to one place in the
core regression tests that falls over because of that:
ExecBuildGroupingEqual() figures it can get away with

InitFunctionCallInfoData(*fcinfo, finfo, 2,
InvalidOid, NULL, NULL);

i.e. passing collation = InvalidOid to the type-specific equality
functions.

Now, it's certainly true that nameeq() doesn't need a collation spec
today, any more than texteq() does, because both types legislate that
equality is bitwise. But if we leave ExecBuildGroupingEqual like this,
we're mandating that no type anywhere, ever, can have a
collation-dependent notion of equality. Is that really a restriction
we're comfortable with? citext is sort of the poster child here,
because it already wishes it could have collation-dependent equality.

We'd have to do some work to pass a reasonable collation choice through
to this code from wherever we have the info available, but I don't
think it'd be a huge amount of work.

BTW, this case is perhaps a bit of a counterexample to Peter's idea about
doing collation lookups earlier: if we change this, we'd presumably have
to do collation lookup right here, even though we know very well that
common types' equality functions won't use the information.

In any case, I think we need a policy decision about whether it's our
intent to allow collation-dependent equality or not.

Comments?

regards, tom lane

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-12-14 19:27:20 Re: Ryu floating point output patch
Previous Message Andrew Gierth 2018-12-14 19:22:29 Re: Ryu floating point output patch