Hi David.
On 2018/03/29 20:08, David Rowley wrote:
> I've looked at the v44 patch.
Thank you.
> Thanks for making those changes.
>
> The new not-equal handling code is not quite right.
>
> DROP TABLE listp;
> CREATE TABLE listp (a INT) PARTITION BY LIST(a);
> CREATE TABLE listp1_3 PARTITION OF listp FOR VALUES IN(1,3);
> CREATE TABLE listp_default PARTITION OF listp DEFAULT;
>
> EXPLAIN SELECT * FROM listp WHERE a <> 1;
> QUERY PLAN
> ------------------------------------------------------------------
> Append (cost=0.00..54.56 rows=2537 width=4)
> -> Seq Scan on listp1_3 (cost=0.00..41.88 rows=2537 width=4)
> Filter: (a <> 1)
> (3 rows)
>
> The default should be included here.
>
> INSERT INTO listp VALUES(1),(2),(3);
> SELECT * FROM listp WHERE a <> 1;
> a
> ---
> 3
> (1 row)
Good catch! Indeed, the default partition should not have been pruned
away in this case.
> This code assumes its fine to just reverse the setting for default:
>
> result->scan_default = !source->scan_default;
>
> More complex handling is needed here.
Hmm, I thought about this and came to a conclusion that we should *always*
scan the default partition in this case. Inversion step removes all the
datums selected by the source step from the set of *all* datums that the
currently defined set of partitions allow. If there's a default partition
in the mix, that means the latter contains all the datums of the partition
key's data type. Irrespective of whether or not the source step selected
the default partition, there would be datums that would be in the set
after inversion which in turn would be in the default partition, if not in
some non-default partition that would've been selected. I have written a
comment there trying to explain this, but I may not have been able to
articulate it properly. Please check. Or does this sound just wrong?
> I've attached a diff for a small set of other things I noticed while
reviewing.
Thanks, merged.
Please find attached the updated patches.
Thanks,
Amit
