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