Horton

Re: Pg18 Recursive Crash

David Rowley <dgrowleyml@gmail.com>

View thread
From: David Rowley <dgrowleyml@gmail.com>
To: Richard Guo <guofenglinux@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Nathan Bossart <nathandbossart@gmail.com>, Paul Ramsey <pramsey@cleverelephant.ca>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2024-12-18T10:45:40Z
Lists: pgsql-hackers

Commits

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

  1. Optimize grouping equality checks with virtual slots

  2. Fix Assert failure in WITH RECURSIVE UNION queries

  3. Fix incorrect slot type in BuildTupleHashTableExt

Attachments

On Wed, 18 Dec 2024 at 22:29, Richard Guo <guofenglinux@gmail.com> wrote:
> Should we be concerned about passing a NULL TupleTableSlotOps in
> nodeRecursiveUnion.c?

I made it pass NULL on purpose because the slot type on the recursive
union can be different on the inner and outer sides. Do you see issues
with that?

> The query below triggers the same assert
> failure: the slot is expected to be TTSOpsMinimalTuple, but it is
> TTSOpsBufferHeapTuple.
>
> create table t (a int);
> insert into t values (1), (1);
>
> with recursive cte (a) as (select a from t union select a from cte)
> select a from cte;

That's a good find. I tested as far back as REL_13_STABLE and it's
failing the same Assert there too.

Maybe we need to backpatch passing NULL instead of &TTSOpsMinimalTuple
to ExecBuildGroupingEqual() in BuildTupleHashTableExt(). Something
like the attached patch.

As far as I see it, there's no downside to this as
ExecComputeSlotInfo() will return true anyway for the
EEOP_INNER_FETCHSOME step in ExecBuildGroupingEqual() due to minimal
tuples needing deformation anyway. For master, we could just do what
Tom proposed above so that any callers that always use virtual slots
can benefit from the elimination of the EEOP_INNER_FETCHSOME step.

David