Julien Rouhaud <rjuju123@gmail.com> writes:
> This all looks good to me. I'm wondering about this chunk though:
> + bool rel_is_partitioned = (rel->part_scheme && rel->part_rels);
> IIUC it' safe for now (according to f069c91a579), but should we use
> IS_PARTITIONED_REL macro instead? If yes, probably
> create_ordinary_grouping_paths() should be updated too.
Hmm, I was just copying the existing test in that function, but I agree
that it seems pretty random to not be using the macro.
Also, given that IS_PARTITIONED_REL is explicitly testing IS_DUMMY_REL,
it doesn't really seem like we need the hack about forcing nparts etc
to 0. I'd transferred that out of apply_scanjoin_target_to_paths into
set_dummy_rel_pathlist and mark_dummy_rel, but that doesn't make it any
less of an ugly kluge. In particular, that's not consistent with the
idea that an appendrel automatically becomes dummy if we generate a
zero-child path for it. So I'm now thinking we should drop that bit
and instead make sure that everyplace that's testing for partitioned-ness
is using this macro.
One more thing --- after sleeping in it, I'm questioning my earlier
feeling that apply_scanjoin_target_to_paths should flush existing paths
for partitioned rels. Maybe it was done like that with the idea of
letting paths with tlist computations done above the Append compete
with paths doing the tlist computations below? That would be a fine
idea if we had any costing factors that would produce a meaningful
choice between the two; but I'm afraid that what we're probably getting
right now is a quasi-random choice between paths whose costs differ
only by rounding errors.
regards, tom lane