Re: Fixing some ancient errors in hash join costing

Chao Li <li.evan.chao@gmail.com> writes:
> Just a few small comments on v2:

> This is not a concern, just curious why switch the setting order of enable_hashjoin and enable_sort?

The need to disable hashjoin only applies to one of the two tests, so
it seemed to me that this way is more nicely nested.  Judgment call
of course.

> As flag 0 is passed to get_attstatsslot(), free_attstatsslot() is not needed.

True.  I wrote it like that so people wouldn't wonder if I'd forgotten
free_attstatsslot(), but if other call sites passing flags == 0 don't
use it then it'd be better to be consistent.  (I didn't check that.)

> Maybe worth adding a short comment like “0.0 doesn’t mean zero frequency, instead 0.0 means no data or unknown
frequency”,which might help code readers to quickly understand the logic. 

Doesn't the function's header comment cover that adequately?

            regards, tom lane