I think we should add this to open items list so that it gets tracked. On Thu, Jul 12, 2018 at 6:31 PM, Ashutosh Bapat <ashutosh.ba...@enterprisedb.com> wrote: > On Thu, Jul 12, 2018 at 11:10 AM, Amit Langote > <langote_amit...@lab.ntt.co.jp> wrote: >>> >>> I think your fix is correct. I slightly modified it along with updating >>> nearby comments and added regression tests. >> >> I updated regression tests to reduce lines. There is no point in >> repeating tests like v2 patch did. > > + * > + * For hash partitioning however, it is possible to combine null and non- > + * null keys in a pruning step, so do this only if *all* partition keys > + * are involved in IS NULL clauses. > > I don't think this is true. When equality conditions and IS NULL clauses cover > all partition keys of a hash partitioned table and do not have contradictory > clauses, we should be able to find the partition which will remain unpruned. I > see that we already have this supported in get_matching_hash_bounds() > /* > * For hash partitioning we can only perform pruning based on equality > * clauses to the partition key or IS NULL clauses. We also can only > * prune if we got values for all keys. > */ > if (nvalues + bms_num_members(nullkeys) == partnatts) > { > > */ > - if (!generate_opsteps) > + if (!bms_is_empty(nullkeys) && > + (part_scheme->strategy != PARTITION_STRATEGY_HASH || > + bms_num_members(nullkeys) == part_scheme->partnatts)) > > So, it looks like we don't need bms_num_members(nullkeys) == > part_scheme->partnatts there. > > Also, I think, we don't know how some new partition strategy will treat NULL > values so above condition looks wrong to me. Instead it should explicitly > check > the strategies for which we know that the NULL values go to a single > partition. > > /* > - * Note that for IS NOT NULL clauses, simply having step suffices; > - * there is no need to propagate the exact details of which keys are > - * required to be NOT NULL. Hash partitioning expects to see actual > - * values to perform any pruning. > + * There are no OpExpr's, but there are IS NOT NULL clauses, which > + * can be used to eliminate the null-partition-key-only partition. > > I don't understand this. When there are IS NOT NULL clauses for all the > partition keys, it's only then that we could eliminate the partition > containing > NULL values, not otherwise. > > > -- > Best Wishes, > Ashutosh Bapat > EnterpriseDB Corporation > The Postgres Database Company
-- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company