Re: BUG #18764: server closed the connection unexpectedly

David Rowley <dgrowleyml@gmail.com>

From: David Rowley <dgrowleyml@gmail.com>
To: Richard Guo <guofenglinux@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, dqetool@126.com, pgsql-bugs@lists.postgresql.org
Date: 2025-01-09T03:44:17Z
Lists: pgsql-bugs

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Fix UNION planner datatype issue

  2. Allow planner to use Merge Append to efficiently implement UNION

Attachments

On Tue, 7 Jan 2025 at 02:14, Richard Guo <guofenglinux@gmail.com> wrote:
>
> On Sat, Jan 4, 2025 at 2:58 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > I'm thinking at this point that the bug boils down to trying to
> > push pathkeys into the subplan without regard for the type
> > conversion that occurs at the set-operation level.  Once we've
> > done that, the lower level will generate this incorrectly-sorted
> > Path, and that probably wins the add_path tournament on the basis
> > of being better sorted and fuzzily the same cost as the unsorted
> > path.  So that's how come that path gets chosen even though
> > the sort is useless in context.
>
> I've reached the same conclusion.  I'm thinking about whether we
> should refrain from pushing pathkeys into the subplan when type
> conversion occurs at the set-operation level.  Maybe we can do this
> check in generate_setop_child_grouplist, like below.

So I guess this must mean that we cannot assume that it's ever safe to
assume that a type that is implicitly castable to the top setop's type
sort order matches, so we must ensure we don't generate any
setop_pathkeys for the subquery in this case.

> --- a/src/backend/optimizer/plan/planner.c
> +++ b/src/backend/optimizer/plan/planner.c
> @@ -8091,6 +8091,9 @@ generate_setop_child_grouplist(SetOperationStmt
> *op, List *targetlist)
>     {
>         TargetEntry *tle = (TargetEntry *) lfirst(lt);
>         SortGroupClause *sgc;
> +       Oid         opfamily,
> +                   opcintype;
> +       int16       strategy;
>
>         /* resjunk columns could have sortgrouprefs.  Leave these alone */
>         if (tle->resjunk)
> @@ -8101,6 +8104,18 @@ generate_setop_child_grouplist(SetOperationStmt
> *op, List *targetlist)
>         sgc = (SortGroupClause *) lfirst(lg);
>         lg = lnext(grouplist, lg);
>
> +       if (!OidIsValid(sgc->sortop))
> +           return NIL;
> +
> +       /* Find the operator in pg_amop --- failure shouldn't happen */
> +       if (!get_ordering_op_properties(sgc->sortop,
> +                                       &opfamily, &opcintype, &strategy))
> +           elog(ERROR, "operator %u is not a valid ordering operator",
> +                sgc->sortop);
> +
> +       if (exprType((Node *) tle->expr) != opcintype)
> +           return NIL;
> +
>         /* assign a tleSortGroupRef, or reuse the existing one */
>         sgc->tleSortGroupRef = assignSortGroupRef(tle, targetlist);
>     }

Can't you just look up the setop's type from op->colTypes instead of
looking the type up via the sortop? i.e. the attached?

David