HI Japin
> Just a quick note: I think `foreach_ptr` is more appropriate here than
`foreach`.
Thank you review this path , I think using foreach_node should be safe
```
#define foreach_ptr(type, var, lst) foreach_internal(type, *, var, lst,
lfirst)
#define foreach_int(var, lst) foreach_internal(int, , var, lst, lfirst_int)
#define foreach_oid(var, lst) foreach_internal(Oid, , var, lst, lfirst_oid)
#define foreach_xid(var, lst) foreach_internal(TransactionId, , var, lst,
lfirst_xid)
/*
* The internal implementation of the above macros. Do not use directly.
*
* This macro actually generates two loops in order to declare two variables
of
* different types. The outer loop only iterates once, so we expect
optimizing
* compilers will unroll it, thereby optimizing it away.
*/
#define foreach_internal(type, pointer, var, lst, func) \
for (type pointer var = 0, pointer var##__outerloop = (type pointer) 1; \
var##__outerloop; \
var##__outerloop = 0) \
for (ForEachState var##__state = {(lst), 0}; \
(var##__state.l != NIL && \
var##__state.i < var##__state.l->length && \
(var = (type pointer) func(&var##__state.l->elements[var##__state.i]), true));
\
var##__state.i++)
/*
* foreach_node -
* The same as foreach_ptr, but asserts that the element is of the specified
* node type.
*/
#define foreach_node(type, var, lst) \
for (type * var = 0, *var##__outerloop = (type *) 1; \
var##__outerloop; \
var##__outerloop = 0) \
for (ForEachState var##__state = {(lst), 0}; \
(var##__state.l != NIL && \
var##__state.i < var##__state.l->length && \
(var = lfirst_node(type, &var##__state.l->elements[var##__state.i]), true));
\
var##__state.i++)
```
On Thu, Mar 5, 2026 at 9:58 AM Japin Li <[email protected]> wrote:
>
> Hi, Richard
>
> On Wed, 04 Mar 2026 at 18:52, Richard Guo <[email protected]> wrote:
> > On Sat, Feb 14, 2026 at 4:37 PM Richard Guo <[email protected]>
> wrote:
> >> On Thu, Feb 5, 2026 at 3:51 PM Richard Guo <[email protected]>
> wrote:
> >> > Attached is the updated patch, which adds the check requiring the
> >> > operator to be a member of a btree or hash opfamily.
> >
> >> Attached is another updated patch rebased on current master, with the
> >> addition of support for RowCompareExpr to handle multi-column ordering
> >> operations; otherwise unchanged.
> >
> > Attached is another updated patch rebased on current master, with some
> > minor cosmetic adjustments; nothing essential has changed.
> >
>
> Thank you for working on this!
>
> Just a quick note: I think `foreach_ptr` is more appropriate here than
> `foreach`.
>
> diff --git a/src/backend/optimizer/plan/subselect.c
> b/src/backend/optimizer/plan/subselect.c
> index 299b3354f6d..0d31861da7f 100644
> --- a/src/backend/optimizer/plan/subselect.c
> +++ b/src/backend/optimizer/plan/subselect.c
> @@ -1484,7 +1484,6 @@ sublink_testexpr_is_not_nullable(PlannerInfo *root,
> SubLink *sublink)
> {
> Node *testexpr = sublink->testexpr;
> List *outer_exprs = NIL;
> - ListCell *lc;
>
> /* Punt if sublink is not in the expected format */
> if (sublink->subLinkType != ANY_SUBLINK || testexpr == NULL)
> @@ -1514,10 +1513,8 @@ sublink_testexpr_is_not_nullable(PlannerInfo *root,
> SubLink *sublink)
> /* multi-column equality or inequality checks */
> BoolExpr *bexpr = (BoolExpr *) testexpr;
>
> - foreach(lc, bexpr->args)
> + foreach_ptr(OpExpr, opexpr, bexpr->args)
> {
> - OpExpr *opexpr = (OpExpr *) lfirst(lc);
> -
> if (!IsA(opexpr, OpExpr))
> return false;
>
> @@ -1537,10 +1534,8 @@ sublink_testexpr_is_not_nullable(PlannerInfo *root,
> SubLink *sublink)
> /* multi-column ordering checks */
> RowCompareExpr *rcexpr = (RowCompareExpr *) testexpr;
>
> - foreach(lc, rcexpr->opnos)
> + foreach_oid(opno, rcexpr->opnos)
> {
> - Oid opno = lfirst_oid(lc);
> -
> /* verify operator safety; see comment above */
> if (!op_is_safe_index_member(opno))
> return false;
> @@ -1566,10 +1561,8 @@ sublink_testexpr_is_not_nullable(PlannerInfo *root,
> SubLink *sublink)
> flatten_join_alias_vars(root, root->parse, (Node *)
> outer_exprs);
>
> /* Check that every outer expression is non-nullable */
> - foreach(lc, outer_exprs)
> + foreach_ptr(Expr, expr, outer_exprs)
> {
> - Expr *expr = (Expr *) lfirst(lc);
> -
> /*
> * We have already collected relation-level not-null
> constraints for
> * the outer query, so we can consult the global hash
> table for
> diff --git a/src/backend/optimizer/util/clauses.c
> b/src/backend/optimizer/util/clauses.c
> index c47c9da4a9b..3f3baf2149a 100644
> --- a/src/backend/optimizer/util/clauses.c
> +++ b/src/backend/optimizer/util/clauses.c
> @@ -2052,7 +2052,6 @@ query_outputs_are_not_nullable(Query *query)
> List *safe_quals = NIL;
> List *nonnullable_vars = NIL;
> bool computed_nonnullable_vars = false;
> - ListCell *tl;
>
> /*
> * If the query contains set operations, punt. The set ops
> themselves
> @@ -2083,9 +2082,8 @@ query_outputs_are_not_nullable(Query *query)
> /*
> * Examine each targetlist entry to prove that it can't produce
> NULL.
> */
> - foreach(tl, query->targetList)
> + foreach_ptr(TargetEntry, tle, query->targetList)
> {
> - TargetEntry *tle = (TargetEntry *) lfirst(tl);
> Expr *expr = tle->expr;
>
> /* Resjunk columns can be ignored: they don't produce
> output values */
> @@ -2194,11 +2192,10 @@ find_subquery_safe_quals(Node *jtnode, List
> **safe_quals)
> else if (IsA(jtnode, FromExpr))
> {
> FromExpr *f = (FromExpr *) jtnode;
> - ListCell *lc;
>
> /* All elements of the FROM list are allowable */
> - foreach(lc, f->fromlist)
> - find_subquery_safe_quals((Node *) lfirst(lc),
> safe_quals);
> + foreach_ptr(Node, node, f->fromlist)
> + find_subquery_safe_quals(node, safe_quals);
> /* ... and its WHERE quals are too */
> if (f->quals)
> *safe_quals = lappend(*safe_quals, f->quals);
>
>
> > - Richard
> >
> > [2. text/x-diff;
> v4-0001-Convert-NOT-IN-sublinks-to-anti-joins-when-safe.patch]...
>
> --
> Regards,
> Japin Li
> ChengDu WenWu Information Technology Co., Ltd.
>
>
>
