Re: Reorganize collation lookup time and place

From: Andres Freund <andres(at)anarazel(dot)de>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Reorganize collation lookup time and place
Date: 2018-12-13 19:40:21
Message-ID: 20181213194021.7igjj7ebt3jenfy5@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2018-12-13 15:05:40 +0100, Peter Eisentraut wrote:
> On 12/12/2018 18:56, Andres Freund wrote:
> >> This makes the collation lookup more similar to the function lookup. In
> >> many cases they now happen right next to each other.
> >> pg_newlocale_from_collation() becomes analogous to fmgr_info() and
> >> pg_locale_t to FmgrInfo *.
> > Why isn't this integrated into fmgr_info()?
>
> fmgr_info() looks up stuff in pg_proc. pg_newlocale_...() looks up
> stuff in pg_collation. There is no overlap between them.

It looks up stuff necessary for calling a function, that doesn't fit
looking up the collation necessary to do so too badly. A lot of the the
changes you made are rote changes to each caller, taking the collation
oid and expanding it with pg_newlocale_from_collation(). It seems not
necessary to force every external user to do rote changes like:

fmgr_info(opexpr->opfuncid, finfo);
fmgr_info_set_expr((Node *) node, finfo);
InitFunctionCallInfoData(*fcinfo, finfo, 2,
- opexpr->inputcollid, NULL, NULL);
+ pg_newlocale_from_collation(opexpr->inputcollid), NULL, NULL);
...
- h = FunctionCall1Coll(array_extra_data->hash, DEFAULT_COLLATION_OID, d);
+ h = FunctionCall1Coll(array_extra_data->hash, pg_newlocale_from_collation(DEFAULT_COLLATION_OID), d);

A lot of the new pg_newlocale_from_collation() calls go a fair bit over
the customary line length limit.

As it stands I have another patch that wants to change the function call
interface, including for extensions:
https://www.postgresql.org/message-id/20180605172952.x34m5uz6ju6enaem%40alap3.anarazel.de
so perhaps we can just bite the bullet and do both.

But I'm somewhat doubtful it's useful to expose having to lookup
pg_newlocale_from_collation() at the callsites to every single caller of
functions in the codebase. You say that's a separate conern of
initializing function calls, for me it's unnecessarily exposing details
that seem likely to change.

> There is also not necessarily a one-to-one correspondence between
> function and collation lookup calls. For example, in some cases you
> need to look up a sorting and a hashing functions, but only one
> collation for both.

That seems rare enough not to matter, performancewise.

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Korotkov 2018-12-13 19:40:59 Re: Connections hang indefinitely while taking a gin index's LWLock buffer_content lock
Previous Message David Steele 2018-12-13 19:29:04 Re: Remove Deprecated Exclusive Backup Mode