On Thu, 11 Apr 2024 at 18:49, Richard Guo <guofengli...@gmail.com> wrote: > I agree with both of your points. But I also think we do not need to > skip applying the NOT NULL qual optimization for partitioned tables. > For partitioned tables, if the parent is marked NOT NULL, then all its > children must also be marked NOT NULL. And we've already populated > notnullattnums for partitioned tables in get_relation_info. Applying > this optimization for partitioned tables can help save some cycles in > apply_child_basequals if we've reduced or skipped some restriction > clauses for a partitioned table. This means in add_base_clause_to_rel > we need to also check rte->relkind: > > - if (!rte->inh) > + if (!rte->inh || rte->relkind == RELKIND_PARTITIONED_TABLE)
I was skipping this on purpose as I wasn't sure that we'd never expand restriction_is_always_false() and restriction_is_always_true() to also handle partition constraints. However, after thinking about that more, the partition constraint can only become more strict the deeper down the partition levels you go. If it was possible to insert a row directly into a leaf partition that wouldn't be allowed when inserting via the parent then there's a bug in the partition constraint. I also considered if we'd ever add support to remove redundant quals in CHECK constraints and if there was room for problematic mismatches between partitioned table and leaf partitions, but I see we've thought about that: postgres=# alter table only p add constraint p_check check (a = 0); ERROR: constraint must be added to child tables too > I also think we should update the related comments for > apply_child_basequals and its caller, as my v1 patch does, since now we > might reduce or skip some of the resulting clauses. I felt the comments you wanted to add at the call site of apply_child_basequals() knew too much about what lies within that function. The caller needn't know anything about what optimisations are applied in apply_child_basequals(). In my book, that's just as bad as documenting things about the calling function from within a function. Since functions are designed to be reused, you're just asking for such comments to become outdated as soon as we teach apply_child_basequals() some new tricks. In this case, all the caller needs to care about is properly handling a false return value. After further work on the comments, I pushed the result. Thanks for working on this. David