On 10 March 2018 at 20:21, Tom Lane <t...@sss.pgh.pa.us> wrote: > I wrote: >> Whilst fooling about with predtest.c, I noticed a rather embarrassing >> error. Consider the following, rather silly, CHECK constraint: >> ... >> So, what to do? We have a few choices, none ideal: > > I'd been assuming that we need to back-patch a fix for this, but after > further reflection, I'm not so sure. The bug is only triggered by fairly > silly CHECK constraints, and given that it's been there a long time (at > least since 9.2 according to my tests) without any field reports, it seems > likely that nobody is writing such silly CHECK constraints. > > If we suppose that we only need to fix it in HEAD, the most attractive > answer is to add a parameter distinguishing WHERE and CHECK arguments > to canonicalize_qual. That allows symmetrical simplification of constant- > NULL subexpressions in the two cases, and the fact that the caller now > has to make an explicit choice of WHERE vs CHECK semantics might help > discourage people from applying the function in cases where it's not > clear which one applies. PFA a patch that does it like that. >
I agree that this looks like the best choice, but it feels a little unsatisfactory to not back-patch a fix for such a glaring bug. You could perhaps leave the signature of canonicalize_qual() the same, but add a new canonicalize_check() function, and make both thin wrappers on top of a local function accepting the is_check parameter. Regards, Dean
