pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Richard Guo <guofenglinux@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>
Cc: pgsql-hackers@lists.postgresql.org, "Finnerty, Jim" <jfinnert@amazon.com>
Subject: Re: Making Vars outer-join aware
Date: Mon, 13 Feb 2023 11:50:17 -0500
Message-ID: <1223373.1676307017@sss.pgh.pa.us> (raw)
In-Reply-To: <CAMbWs4-K8_yqARnrmRB5=xAWTJGgTpimQinvaa-k6c8f6Hideg@mail.gmail.com>
References: <CAMbWs4-mvPPCJ1W6iK6dD5HiNwoJdi6mZp=-7mE8N9Sh+cd0tQ@mail.gmail.com>
	<CAMbWs4_WNcvQGiFMLJanj3h6WMzFi6P-=bifmnuXUcg4PSXQgA@mail.gmail.com>
	<1035990.1667261942@sss.pgh.pa.us>
	<2901865.1667685211@sss.pgh.pa.us>
	<CAMbWs4_pH7w7voY0AejUM4RiKq-Xuf+sXmoF8PcQwzNrsagEXA@mail.gmail.com>
	<CAMbWs4_gXgGdkkOraUPEoZD_Q3xTW7FTxfomW+fxPN2Kr-YSXg@mail.gmail.com>
	<4019771.1672155080@sss.pgh.pa.us>
	<CAMbWs4_JDDdXvEBWtTao9ZTwOGdDCU7X-=bF7yi9G1u_-b0Dfw@mail.gmail.com>
	<323812.1672241806@sss.pgh.pa.us>
	<2043562.1674506286@sss.pgh.pa.us>
	<20230212235823.GW1653@telsasoft.com>
	<CAMbWs4-K8_yqARnrmRB5=xAWTJGgTpimQinvaa-k6c8f6Hideg@mail.gmail.com>

Richard Guo <guofenglinux@gmail.com> writes:
> Thanks for the report!  I've looked at it a little bit and traced down
> to function have_unsafe_outer_join_ref().  The comment there says
>  * In practice, this test never finds a problem ...
> This seems not correct as showed by the counterexample.

Right.  I'd noticed that the inner loop of that function was not
reached in our regression tests, and incorrectly concluded that it
was not reachable --- but I failed to consider cases where the
inner rel's parameterization depends on Vars from multiple places.

> The NOT_USED part of code is doing this check.  But I think we need a
> little tweak.  We should check the nullable side of related outer joins
> against the satisfied part, rather than inner_paramrels.  Maybe
> something like attached.

Agreed.

> However, this test seems to cost some cycles after the change.  So I
> wonder if it's worthwhile to perform it, considering that join order
> restrictions should be able to guarantee there is no problem here.

Yeah, I think we should reduce it to an Assert check.  It shouldn't be
worth the cycles to run in production, and that will also make it easier
to notice in sqlsmith testing if anyone happens across another
counterexample.

Pushed that way.  Thanks for the report and the patch!

			regards, tom lane





view thread (60+ messages)  latest in thread

Message-ID: <1223373.1676307017@sss.pgh.pa.us>
Permalink:  ../1223373.1676307017@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/1223373.1676307017@sss.pgh.pa.us

 · 

reply
Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, guofenglinux@gmail.com, pryzby@telsasoft.com, jfinnert@amazon.com
  Subject: Re: Making Vars outer-join aware
  In-Reply-To: <1223373.1676307017@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox