Chao Li <[email protected]> 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
