From: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> |
---|---|
To: | Arthur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru> |
Cc: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Ildus Kurbangaliev <i(dot)kurbangaliev(at)postgrespro(dot)ru>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [PROPOSAL] Shared Ispell dictionaries |
Date: | 2018-03-19 19:50:46 |
Message-ID: | 2ae9588a-f3cb-777d-1958-c76c22225823@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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 | Content-Type | Size |
---|---|---|
0003.diff | text/x-patch | 3.9 KB |
0004.diff | text/x-patch | 2.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2018-03-19 19:53:48 | Re: [HACKERS] why not parallel seq scan for slow functions |
Previous Message | Tom Lane | 2018-03-19 19:50:10 | Re: Compile error while building postgresql 10.3 |