On Tue, Jan 8, 2019 at 3:04 PM Tom Lane <t...@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?
