|From:||Arthur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>|
|To:||Ildus Kurbangaliev <i(dot)kurbangaliev(at)postgrespro(dot)ru>|
|Cc:||Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: [PROPOSAL] Shared Ispell dictionaries|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
Thank you for your review! Good catches.
On Thu, Jan 25, 2018 at 03:26:46PM +0300, Ildus Kurbangaliev wrote:
> In 0001 there are few lines where is only indentation has changed.
> - TsearchShmemSize - calculating size using hash_estimate_size seems
> redundant since you use DSA hash now.
Fixed. True, there is no need in hash_estimate_size anymore.
> - ts_dict_shmem_release - LWLockAcquire in the beginning makes no
> sense, since dict_table couldn't change anyway.
Fixed. In earlier version tsearch_ctl was used here, but I forgot to remove LWLockAcquire.
> - ts_dict_shmem_location could return IspellDictData, it makes more
I assume that ts_dict_shmem_location can be used by various types of dictionaries, not only by Ispell. So void * more suitable here.
> 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.
Agree. In our offline conversation we came to Shareable, that is a dictionary can be shared. It may be more appropriate because setting Shareable=true doesn't guarantee that a dictionary will be allocated in shared memory due to max_shared_dictionaries_size GUC.
Attached new version of the patch.
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
|Next Message||Tom Lane||2018-01-25 17:02:59||Re: Further cleanup of pg_dump/pg_restore item selection code|
|Previous Message||Tom Lane||2018-01-25 16:29:34||Re: [PATCH][PROPOSAL] Refuse setting toast.* reloptions when TOAST table does not exist|