From: Ildus Kurbangaliev <i.kurbangaliev@postgrespro.ru>
To: Arthur Zakirov <a.zakirov@postgrespro.ru>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: [PROPOSAL] Shared Ispell dictionaries
Date: Thu, 25 Jan 2018 15:26:46 +0300
Message-ID: <20180125152646.3b4d2850@wp.localdomain> (raw)
In-Reply-To: <20180124172039.GA11210@zakirov.localdomain>
References: <20171226164825.GA29922@zakirov.localdomain>
<20171231152811.GA4233@arthur.localdomain>
<20180107190526.GA27803@arthur.localdomain>
<d12d9395-922c-64c9-c87d-dd0e1d31440e@2ndquadrant.com>
<CAKNkYnzQ8SJCiFityn+4B_0zZKOmfw1R1M0KHXaFF8BYf9DJ=g@mail.gmail.com>
<20180124172039.GA11210@zakirov.localdomain>
On Wed, 24 Jan 2018 20:20:41 +0300
Arthur Zakirov <a.zakirov@postgrespro.ru> wrote:
Hi, I did some review of the patch.
In 0001 there are few lines where is only indentation has changed.
0002:
- TsearchShmemSize - calculating size using hash_estimate_size seems
redundant since you use DSA hash now.
- ts_dict_shmem_release - LWLockAcquire in the beginning makes no
sense, since dict_table couldn't change anyway.
0003:
- ts_dict_shmem_location could return IspellDictData, it makes more
sense.
0006:
It's very subjective, but I think it would nicer to call option as
Shared (as property of dictionary) or UseSharedMemory, the boolean
option called SharedMemory sounds weird.
Overall the patches look good, all tests passed. I tried to broke it in
few places where I thought it could be unsafe but not succeeded.
--
---
Ildus Kurbangaliev
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: i.kurbangaliev@postgrespro.ru, a.zakirov@postgrespro.ru, tomas.vondra@2ndquadrant.com
Subject: Re: [PROPOSAL] Shared Ispell dictionaries
In-Reply-To: <20180125152646.3b4d2850@wp.localdomain>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox