Thread: Fwd: pgsql: Remove over-optimistic Assert.
Resend this email to -hackers. Sorry for the noise.
Thanks
Richard
Thanks
Richard
---------- Forwarded message ---------
From: Richard Guo <guofenglinux@gmail.com>
Date: Thu, Feb 2, 2023 at 9:51 AM
Subject: Re: pgsql: Remove over-optimistic Assert.
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: <pgsql-committers@lists.postgresql.org>, PostgreSQL-development <pgsql-hackers@postgresql.org>
From: Richard Guo <guofenglinux@gmail.com>
Date: Thu, Feb 2, 2023 at 9:51 AM
Subject: Re: pgsql: Remove over-optimistic Assert.
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: <pgsql-committers@lists.postgresql.org>, PostgreSQL-development <pgsql-hackers@postgresql.org>
On Thu, Feb 2, 2023 at 8:40 AM Tom Lane <tgl@sss.pgh.pa.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
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