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
suitablehere.
> 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
becausesetting Shareable=true doesn't guarantee that a dictionary will be allocated in shared memory due to
max_shared_dictionaries_sizeGUC.
Attached new version of the patch.
--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
