On Wed, Apr 10, 2024 at 1:13 PM David Rowley <dgrowle...@gmail.com> wrote:
> I looked at the patch and I don't think it's a good idea to skip > recording NOT NULL constraints to fix based on the fact that it > happens to result in this particular optimisation working correctly. > It seems that just makes this work in favour of possibly being wrong > for some future optimisation where we have something else that looks > at the RelOptInfo.notnullattnums and makes some decision that assumes > the lack of corresponding notnullattnums member means the column is > NULLable. Hmm, I have thought about your point, but I may have a different perspective. I think the oversight discussed here occurred because we mistakenly recorded NOT NULL columns that are actually nullable for traditional inheritance parents. Take the query from my first email as an example. There are three RTEs: p(inh), p(non-inh) and c(non-inh). And we've added a NOT NULL constraint on the column a of 'p' but not of 'c'. So it seems to me that while we can mark column a of p(non-inh) as non-nullable, we cannot mark column a of p(inh) as non-nullable, because there might be NULL values in 'c' and that makes column a of p(inh) nullable. 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. Thanks Richard
