Re: [PROPOSAL] Shared Ispell dictionaries

From: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
To: Arthur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Ildus Kurbangaliev <i(dot)kurbangaliev(at)postgrespro(dot)ru>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PROPOSAL] Shared Ispell dictionaries
Date: 2019-01-16 00:23:41
Message-ID: 68aaaff6-0efe-c14b-7aee-fb110bb97f69@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello Arthur,

I've looked at the patch today, and in general is seems quite solid to
me. I do have a couple of minor points

1) I think the comments need more work. Instead of describing all the
individual changes here, I've outlined those improvements in attached
patches (see the attached "tweaks" patches). Some of it is formatting,
minor rewording or larger changes. Some comments are rather redundant
(e.g. the one before calls to release the DSM segment).

2) It's not quite clear to me why we need DictInitData, which simply
combines DictPointerData and list of options. It seems as if the only
point is to pass a single parameter to the init function, but is it
worth it? Why not to get rid of DictInitData entirely and pass two
parameters instead?

3) I find it a bit cumbersome that before each ts_dict_shmem_release
call we construct a dummy DickPointerData value. Why not to pass
individual parameters and construct the struct in the function?

4) The reference to max_shared_dictionaries_size is obsolete, because
there's no such limit anymore.

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment Content-Type Size
0001-Fix-ispell-memory-handling.patch text/x-patch 1.5 KB
0002-Change-tmplinit-argument.patch text/x-patch 13.3 KB
0003-Change-tmplinit-argument-tweaks.patch text/x-patch 6.7 KB
0004-Retrieve-shared-location-for-dict.patch text/x-patch 17.2 KB
0005-Retrieve-shared-location-for-dict-tweak.patch text/x-patch 2.7 KB
0006-Store-ispell-in-shared-location.patch text/x-patch 90.6 KB
0007-Store-ispell-in-shared-location-tweaks.patch text/x-patch 2.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kuroda, Hayato 2019-01-16 00:40:15 RE: Log a sample of transactions
Previous Message David Rowley 2019-01-15 23:38:55 Re: BUG #15572: Misleading message reported by "Drop function operation" on DB with functions having same name