On Wed, 10 Apr 2024 at 19:12, Richard Guo <guofenglinux@gmail.com> wrote:
> And I think recording NOT NULL columns for traditional inheritance
> parents can be error-prone for some future optimization where we look
> at an inheritance parent's notnullattnums and make decisions based on
> the assumption that the included columns are non-nullable. The issue
> discussed here serves as an example of this potential problem.
I admit that it seems more likely that having a member set in
notnullattnums for an inheritance parent is more likely to cause
future bugs than if we just leave them blank. But I also don't
believe leaving them all blank is the right thing unless we document
that the field isn't populated for traditional inheritance parent
tables and if the code needs to not the NOT NULL status of a column
for that table ONLY, then the code should look at the RelOptInfo
corresponding to the inh==false RangeTblEntry for that relation. If we
don't document the fact that we don't set the notnullattnums field
then someone might write some code thinking we correctly populate it.
If the parent and all children have NOT NULL constraints for a
column, then unless we document we don't populate notnullattnums, it
seems reasonable to assume that's a bug.
If we skip populating notnullattnums for inh==true non-partitioned
tables, I think we also still need to skip applying the NOT NULL qual
optimisation for inh==true RTEs as my version of the code did.
Reasons being: 1) it's a pointless exercise since we'll always end up
adding the RestrictInfo without modification to the RelOptInfo's
baserestrictinfo, and 2) The optimisation in question would be looking
at the notnullattnums that isn't correctly populated.
Resulting patch attached.
David