Subject: A wrong comment about search_indexed_tlist_for_var

From: Richard Guo <[email-masked]>
Date: Fri, 1 Dec 2023 19:12:34 +0800

The comment of search_indexed_tlist_for_var says:

 * In debugging builds, we cross-check the varnullingrels of the subplan
 * output Var based on nrm_match.

However, this cross-check will also be performed in non-debug builds
ever since commit 867be9c07, which converts this check from Asserts to
test-and-elog.  The commit message there also says:

    Committed separately with the idea that eventually we'll revert
    this.  It might be awhile though.

I wonder if now is the time to revert it, since there have been no
related bugs reported for quite a while.  Otherwise I think we may need
to revise the comment of search_indexed_tlist_for_var to clarify that
the cross-check is not limited to debugging builds.

Please note that if we intend to revert commit 867be9c07, we need to
revert 69c430626 too.

Thanks
Richard

---

From: Alvaro Herrera <[email-masked]>
Date: Fri, 1 Dec 2023 13:25:31 +0100

On 2023-Dec-01, Richard Guo wrote:

> related bugs reported for quite a while.

I don't know anything about this, but maybe it would be better to let
these elogs there for longer, so that users have time to upgrade and
test.  This new code has proven quite tricky, and if I understand
correctly, if we do run some query with wrong varnullingrels in
production code without elog and where Assert() does nothing, that might
silently lead to wrong results.

OTOH keeping the elog there might impact performance.  Would that be
significant?

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Debido a que la velocidad de la luz es mucho mayor que la del sonido,
 algunas personas nos parecen brillantes un minuto antes
 de escuchar las pelotudeces que dicen." (Roberto Fontanarrosa)

---

From: Matt Skelley <[email-masked]>
Date: Mon, 18 Dec 2023 15:26:40 +0000

The following review has been posted through the commitfest application:
make installcheck-world:  not tested
Implements feature:       tested, passed
Spec compliant:           tested, passed
Documentation:            tested, passed

Comment is updated correctly.