Re: pgsql: Remove over-optimistic Assert.

From: Richard Guo <guofenglinux(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-committers(at)lists(dot)postgresql(dot)org, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pgsql: Remove over-optimistic Assert.
Date: 2023-02-02 01:51:58
Message-ID: CAMbWs4_dOrnBgOQ0WLU4axRDVae8+YMr895JeHEXmd755ZwH+w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On Thu, Feb 2, 2023 at 8:40 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Remove over-optimistic Assert.
>
> In commit 2489d76c4, I'd thought it'd be safe to assert that a
> PlaceHolderVar appearing in a scan-level expression has empty
> nullingrels. However this is not so, as when we determine that a
> join relation is certainly empty we'll put its targetlist into a
> Result-with-constant-false-qual node, and nothing is done to adjust
> the nullingrels of the Vars or PHVs therein. (Arguably, a Result
> used in this way isn't really a scan-level node, but it certainly
> isn't an upper node either ...)

It seems this is the only case we can have PlaceHolderVar with non-empty
nullingrels at scan level. So I wonder if we can manually adjust the
nullingrels of PHVs in this special case, and keep the assertion about
phnullingrels being NULL in fix_scan_expr. I think that assertion is
asserting the right thing in most cases. It's a pity to lose it.

Currently for the tlist of a childless Result, we special-case ROWID_VAR
Vars in set_plan_refs and thus keep assertions about varno != ROWID_VAR
in fix_scan_expr. Do you think we can special-case PHVs at the same
place by setting its phnullingrels to NULL? I'm imagining something
like attached.

Thanks
Richard

Attachment Content-Type Size
v1-0001-Adjust-phnullingrels-for-childless-Result.patch application/octet-stream 2.3 KB

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2023-02-02 01:53:17 Re: pgsql: Do assorted mop-up in the planner.
Previous Message Robins Tharakan 2023-02-02 01:50:09 Re: pgsql: Do assorted mop-up in the planner.

Browse pgsql-hackers by date

  From Date Subject
Next Message Richard Guo 2023-02-02 02:01:49 Fwd: pgsql: Remove over-optimistic Assert.
Previous Message Michael Paquier 2023-02-02 01:23:21 Re: Weird failure with latches in curculio on v15