Re: Reorganize collation lookup time and place - Mailing list pgsql-hackers

From Andres Freund
Subject Re: Reorganize collation lookup time and place
Date
Msg-id 20181213194021.7igjj7ebt3jenfy5@alap3.anarazel.de
Whole thread Raw
In response to Re: Reorganize collation lookup time and place  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
Responses Re: Reorganize collation lookup time and place
List 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


pgsql-hackers by date:

Previous
From: David Steele
Date:
Subject: Re: Remove Deprecated Exclusive Backup Mode
Next
From: Alexander Korotkov
Date:
Subject: Re: Connections hang indefinitely while taking a gin index's LWLockbuffer_content lock