Re: [PROPOSAL] Shared Ispell dictionaries - Mailing list pgsql-hackers

From Tomas Vondra
Subject Re: [PROPOSAL] Shared Ispell dictionaries
Date
Msg-id 2ae9588a-f3cb-777d-1958-c76c22225823@2ndquadrant.com
Whole thread Raw
In response to Re: [PROPOSAL] Shared Ispell dictionaries  (Arthur Zakirov <a.zakirov@postgrespro.ru>)
Responses Re: [PROPOSAL] Shared Ispell dictionaries  (Arthur Zakirov <a.zakirov@postgrespro.ru>)
List pgsql-hackers
Hi Arthur,

I went through the patch - just skimming through the diffs, will do more
testing tomorrow. Here are a few initial comments.

1) max_shared_dictionaries_size / PGC_POSTMASTER

I'm not quite sure why the GUC is defined as PGC_POSTMASTER, i.e. why it
can't be changed after server start. That seems like a fairly useful
thing to do (e.g. increase the limit while the server is running), and
after looking at the code I think it shouldn't be difficult to change.

The other thing I'd suggest is handling "-1" as "no limit".


2) max_shared_dictionaries_size / size of number

Some of the comments dealing with the GUC treat it as a number of
dictionaries (instead of a size). I suppose that's due to how the
original patch was implemented.


3) Assert(max_shared_dictionaries_size);

I'd say that assert is not very clear - it should be

    Assert(max_shared_dictionaries_size > 0);

or something along the lines. It's also a good idea to add a comment
explaining the assert, say

    /* we can only get here when shared dictionaries are enabled */
    Assert(max_shared_dictionaries_size > 0);

4) I took the liberty of rewording some of the docs/comments. See the
attached diffs, that should apply on top of 0003 and 0004 patches.
Please, treat those as mere suggestions.


regards

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

Attachment

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Compile error while building postgresql 10.3
Next
From: Robert Haas
Date:
Subject: Re: [HACKERS] why not parallel seq scan for slow functions