David Rowley <dgrowle...@gmail.com> writes:
> One minor detail... I think the only thing I'd like to see is the
> moving of the enable_hashagg checks to increment the disabled_nodes
> count in create_setop_path() instead of where it's being called. I
> understand there's only 1 caller of that function that passes
> SETOP_HASHED, but it does seem nicer to put that logic where it
> belongs. With how you have it now, if we were ever to grow any more
> places that built SETOP_HASHED SetOpPaths, they'd also need to adjust
> disabled_nodes manually and that seems easy to forget. Also, looking
> around for references to "disabled_nodes", it looks like all other
> places where we fiddle with the value of disabled_nodes are in
> costsize.c.
Looks like costsize.c and pathnode.c to me, but either way I take your
point. I'd not realized that Robert set it up that way, but now I see
he did. I agree that moving that bit of logic into
create_setop_path() seems better. I'll make it so and push.
regards, tom lane
