The comment /* not needed for Consts */ may be more better close to if (!IsA(expr, Const)). Others look good to me.
David Rowley <dgrowle...@gmail.com> 于2023年10月9日周一 07:28写道: > On Sat, 7 Oct 2023 at 03:11, Sergei Glukhov <s.gluk...@postgrespro.ru> > wrote: > > I noticed that combination of prepared statement with generic plan and > > 'IS NULL' clause could lead partition pruning to crash. > > > Test case: > > ------ > > set plan_cache_mode to force_generic_plan; > > prepare stmt AS select * from hp where a is null and b = $1; > > explain execute stmt('xxx'); > > Thanks for the detailed report and proposed patch. > > I think your proposed fix isn't quite correct. I think the problem > lies in InitPartitionPruneContext() where we assume that the list > positions of step->exprs are in sync with the keyno. If you look at > perform_pruning_base_step() the code there makes a special effort to > skip over any keyno when a bit is set in opstep->nullkeys. > > It seems that your patch is adjusting the keyno that's given to the > PruneCxtStateIdx() and it looks like (for your test case) it'll end up > passing keyno==0 when it should be passing keyno==1. keyno is the > index of the partition key, so you can't pass 0 when it's for key > index 1. > > I wonder if it's worth expanding the tests further to cover more of > the pruning cases to cover run-time pruning too. > I think it's worth doing that. David >