Re: [PROPOSAL] Shared Ispell dictionaries

Hello,

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.

Fixed.

> 0002:
> - 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.

> 0003:
> - ts_dict_shmem_location could return IspellDictData, it makes more
> sense.

I assume that ts_dict_shmem_location can be used by various types of dictionaries, not only by Ispell. So void * more suitable here.

> 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.

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.

--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company