Re: Assert failure of the cross-check for nullingrels

2023-06-06 Thread Richard Guo
On Wed, Jun 7, 2023 at 4:22 AM Tom Lane wrote: > Alvaro Herrera writes: > > So, is this done? I see that you made other commits fixing related code > > several days after this email, but none seems to match the changes you > > posted in this patch; and also it's not clear to me that there's

Re: Assert failure of the cross-check for nullingrels

2023-06-06 Thread Tom Lane
Alvaro Herrera writes: > So, is this done? I see that you made other commits fixing related code > several days after this email, but none seems to match the changes you > posted in this patch; and also it's not clear to me that there's any > test case where this patch is expected to change

Re: Assert failure of the cross-check for nullingrels

2023-06-06 Thread Alvaro Herrera
On 2023-May-21, Tom Lane wrote: > Since we're hard up against the beta1 wrap deadline, I went ahead > and pushed the v5 patch. I doubt that it's perfect yet, but it's > a small change and demonstrably fixes the cases we know about. > > As I said in the commit message, the main knock I'd lay on

Re: Assert failure of the cross-check for nullingrels

2023-05-21 Thread Tom Lane
I wrote: > If they have the same clause_relids, then clearly in its current > form clause_is_computable_at() cannot distinguish them. So what > I now think we should do is have clause_is_computable_at() examine > their required_relids instead. Those will be different, by > construction in

Re: Assert failure of the cross-check for nullingrels

2023-05-20 Thread Tom Lane
Richard Guo writes: > I tried with v4 patch and find that, as you predicted, it might reject > all the clones in some cases. Check the query below > ... > So the qual 't3.a = t4.a' is missing in this plan shape. I poked into that more closely and realized that the reason that

Re: Assert failure of the cross-check for nullingrels

2023-05-19 Thread Tom Lane
Richard Guo writes: > I keep thinking about my proposal in v2 patch. It seems more natural to > me to fix this issue, because an outer join's quals are always treated > as a whole when we check if identity 3 applies in make_outerjoininfo, as > well as when we adjust the outer join's quals for

Re: Assert failure of the cross-check for nullingrels

2023-05-18 Thread Richard Guo
On Fri, May 19, 2023 at 12:33 AM Tom Lane wrote: > Bleah. The other solution I'd been poking at involved adding an > extra check for clone clauses, as attached (note this requires > 8a2523ff3). This survives your example, but I wonder if it might > reject all the clones in some cases. It

Re: Assert failure of the cross-check for nullingrels

2023-05-18 Thread Tom Lane
Richard Guo writes: > On Thu, May 18, 2023 at 3:34 AM Tom Lane wrote: >> After some poking at it I hit on what seems like a really simple >> solution: we should be checking syn_righthand not min_righthand >> to see whether a Var should be considered nullable by a given OJ. > I thought about

Re: Assert failure of the cross-check for nullingrels

2023-05-18 Thread Tom Lane
Richard Guo writes: > On Thu, May 18, 2023 at 3:42 AM Tom Lane wrote: >> ... BTW, something I'd considered in an earlier attempt at fixing this >> was to change clause_is_computable_at's API to pass the clause's >> RestrictInfo not just the clause_relids, along the lines of > This change looks

Re: Assert failure of the cross-check for nullingrels

2023-05-18 Thread Richard Guo
On Thu, May 18, 2023 at 3:42 AM Tom Lane wrote: > ... BTW, something I'd considered in an earlier attempt at fixing this > was to change clause_is_computable_at's API to pass the clause's > RestrictInfo not just the clause_relids, along the lines of > > @@ -541,9 +547,10 @@

Re: Assert failure of the cross-check for nullingrels

2023-05-18 Thread Richard Guo
On Thu, May 18, 2023 at 3:34 AM Tom Lane wrote: > After some poking at it I hit on what seems like a really simple > solution: we should be checking syn_righthand not min_righthand > to see whether a Var should be considered nullable by a given OJ. > Maybe that's still not quite right, but it

Re: Assert failure of the cross-check for nullingrels

2023-05-17 Thread Tom Lane
... BTW, something I'd considered in an earlier attempt at fixing this was to change clause_is_computable_at's API to pass the clause's RestrictInfo not just the clause_relids, along the lines of @@ -541,9 +547,10 @@ extract_actual_join_clauses(List *restrictinfo_list, */ bool

Re: Assert failure of the cross-check for nullingrels

2023-05-17 Thread Tom Lane
Richard Guo writes: > Here is an updated patch with comments and test case. I also change the > code to store 'group_clause_relids' directly in RestrictInfo. Hmm ... I don't like this patch terribly much. It's not obvious why (or if) it works, and it's squirreling bits of semantic knowledge

Re: Assert failure of the cross-check for nullingrels

2023-05-16 Thread Jonathan S. Katz
On 5/16/23 9:49 AM, Tom Lane wrote: "Jonathan S. Katz" writes: Is there a specific commit targeted for v16 that introduced this issue? Does it only affect v16 or does it affect backbranches? It's part of the outer-join-aware-Vars stuff, so it's my fault ... and v16 only. *nods* thanks. I

Re: Assert failure of the cross-check for nullingrels

2023-05-16 Thread Tom Lane
"Jonathan S. Katz" writes: > Is there a specific commit targeted for v16 that introduced this issue? > Does it only affect v16 or does it affect backbranches? It's part of the outer-join-aware-Vars stuff, so it's my fault ... and v16 only. regards, tom lane

Re: Assert failure of the cross-check for nullingrels

2023-05-16 Thread Jonathan S. Katz
On 5/12/23 3:02 AM, Richard Guo wrote: On Fri, Mar 17, 2023 at 11:05 AM Richard Guo > wrote: Here is an updated patch with comments and test case.  I also change the code to store 'group_clause_relids' directly in RestrictInfo. BTW, I've added an open

Re: Assert failure of the cross-check for nullingrels

2023-05-12 Thread Richard Guo
On Fri, Mar 17, 2023 at 11:05 AM Richard Guo wrote: > Here is an updated patch with comments and test case. I also change the > code to store 'group_clause_relids' directly in RestrictInfo. > BTW, I've added an open item for this issue. Thanks Richard

Re: Assert failure of the cross-check for nullingrels

2023-03-16 Thread Richard Guo
On Mon, Mar 13, 2023 at 5:44 PM Richard Guo wrote: > On Mon, Mar 13, 2023 at 5:03 PM Richard Guo > wrote: > >> Back to the original issue, if a join has more than one quals, actually >> we treat them as a whole when we check if identity 3 applies as well as >> when we adjust them to be suitable

Re: Assert failure of the cross-check for nullingrels

2023-03-13 Thread Richard Guo
On Mon, Mar 13, 2023 at 5:03 PM Richard Guo wrote: > Back to the original issue, if a join has more than one quals, actually > we treat them as a whole when we check if identity 3 applies as well as > when we adjust them to be suitable for commutation according to identity > 3. So when we check

Re: Assert failure of the cross-check for nullingrels

2023-03-13 Thread Richard Guo
On Fri, Mar 10, 2023 at 4:13 PM Richard Guo wrote: > I wonder if we should consider syn_xxxhand rather than min_xxxhand in > clause_is_computable_at when we check if clause mentions any nullable > Vars. But I'm not sure about that. > No, considering syn_xxxhand is not right. After some join

Assert failure of the cross-check for nullingrels

2023-03-10 Thread Richard Guo
While looking into issue [1], I came across $subject on master. Below is how to reproduce it. DROP TABLE IF EXISTS t1,t2,t3,t4 CASCADE; CREATE TABLE t1 AS SELECT true AS x FROM generate_series(0,1) x; CREATE TABLE t2 AS SELECT true AS x FROM generate_series(0,1) x; CREATE TABLE t3 AS SELECT true