On Tue, Jan 8, 2019 at 3:04 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > I'll take a crack at separating into a module. I'll wait a bit in
> > case there are any stylistic suggestions on the patch as it stands.
>
> I had a go at that myself. I'm sure there's plenty to criticize in
> the result, but at least it passes make check-world ;-)
Just a couple comments about the module:
-If you qualify the function's module name as you did
(PerfectHash::generate_hash_function), you don't have to export the
function into the callers namespace, so you can skip the @EXPORT_OK
setting. Most of our modules don't export.
-There is a bit of a cognitive clash between $case_sensitive in
gen_keywordlist.pl and $case_insensitive in PerfectHash.pm. They each
make sense in their own file, but might it be worth using one or the
other?
-As for the graph algorithm, I'd have to play with it to understand
how it works.
In the committed keyword patch, I noticed that in common/keywords.c,
the array length is defined with
ScanKeywordCategories[SCANKEYWORDS_NUM_KEYWORDS]
but other keyword arrays just have ...[]. Is there a reason for the difference?