Re: [HACKERS] path toward faster partition pruning

Hi,

I think there's a bug in generate_pruning_steps_from_opexprs, which does
this for PARTITION_STRATEGY_HASH:

for_each_cell(lc1, lc)
{
pc = lfirst(lc1);

/*
* Note that we pass nullkeys for step_nullkeys,
* because we need to tell hash partition bound search
* function which of the keys are NULL.
*/
Assert(pc->op_strategy == HTEqualStrategyNumber);
pc_steps =
get_steps_using_prefix(context,
HTEqualStrategyNumber,
pc->expr,
pc->cmpfn,
pc->keyno,
nullkeys,
prefix);
}

opsteps = list_concat(opsteps, list_copy(pc_steps));

Notice that the list_concat() is outside the for_each_cell loop. Doesn't
that mean we fail to consider some of the clauses (all except the very
last clause) for pruning? I haven't managed to come up with an example,
but I haven't tried very hard.

FWIW I've noticed this because gcc complains that pg_steps might be used
uninitialized:

partprune.c: In function ‘generate_partition_pruning_steps_internal’:
partprune.c:992:16: warning: ‘pc_steps’ may be used uninitialized in
this function [-Wmaybe-uninitialized]
opsteps = list_concat(opsteps, list_copy(pc_steps));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
partprune.c:936:14: note: ‘pc_steps’ was declared here
List *pc_steps;
^~~~~~~~
All of PostgreSQL successfully made. Ready to install.

So even if it's not a bug, we probably need to fix the code somehow.

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services