Re: BUG #17975: Nested Loop Index Scan returning wrong result

On Thu, 15 Jun 2023 at 12:50, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> David Rowley <dgrowleyml(at)gmail(dot)com> writes:
> > On Thu, 15 Jun 2023 at 12:28, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> I concur that we'd better just not use partial indexes in
> >> relation_has_unique_index_for.
>
> > I wonder if that's ok for a backpatch. This affects both left join
> > removals and unique joins. Seems like suddenly making a left join
> > removal not work might cause someone some pain.
>
> I kind of doubt that this will affect any large number of users.
> If it did, we'd have had reports sooner.

OK, I've attached a patch for just not using partial indexes as
uniqueness proofs.

> > Is it worth trying to jam in a new boolean field into IndexOptInfo
> > into some spare padding to that we run predicate_implied_by() just
> > using baserestrictinfo and use those in
> > relation_has_unique_index_for()?
>
> How will that work with the caching in innerrel_is_unique?
> I also seriously doubt that we can make such a thing work
> without adding parameters to any externally-visible functions.

relation_has_unique_index_for() would just check this new field that
gets set only from baserestrictinfo quals. We'd just completely
ignore join quals for the new boolean field that'll be used in
relation_has_unique_index_for().

You're probably thinking that we could use join quals up to the level
that we've already joined to for unique joins. That's probably true,
but seems more complex than it might be worth to do that. I imagined
the majority of predOK are set to true based on baserestrictinfo
quals. I've no data to back that up, however. I was just trying to
think of ways to lessen the cases we regress from fixing this bug.

David