Re: Making Vars outer-join aware

2023-07-23 Thread Anton A. Melnikov
On 08.06.2023 19:58, Tom Lane wrote: I think the right thing here is not either of your patches, but to tweak adjust_relid_set() to not fail on negative oldrelid. I'll go make it so. Thanks! This fully solves the problem with ChangeVarNodes() that i wrote above. Hmm. That implies that

Re: Making Vars outer-join aware

2023-06-08 Thread Tom Lane
[ back from PGCon ... ] "Anton A. Melnikov" writes: > On 04.05.2023 15:22, Tom Lane wrote: >> Under what circumstances would you be trying to inject INDEX_VAR >> into a nullingrel set? Only outer-join relids should ever appear there. > The thing is that i don't try to push INDEX_VAR into a

Re: Making Vars outer-join aware

2023-05-29 Thread Anton A. Melnikov
Hello and sorry for the big delay in reply! On 04.05.2023 15:22, Tom Lane wrote: AFAICS the change you propose would serve only to mask bugs. Yes, it's a bad decision. Under what circumstances would you be trying to inject INDEX_VAR into a nullingrel set? Only outer-join relids should ever

Re: Making Vars outer-join aware

2023-05-04 Thread Tom Lane
"Anton A. Melnikov" writes: > The thing is that for custom scan nodes as readme says: > "INDEX_VAR is abused to signify references to columns of a custom scan tuple > type" > But INDEX_VAR has a negative value, so it can not be used in varnullingrels > bitmapset. > And therefore this

Re: Making Vars outer-join aware

2023-05-04 Thread Anton A. Melnikov
Hello! I'm having doubts about this fix but most likely i don't understand something. Could you help me to figure it out, please. The thing is that for custom scan nodes as readme says: "INDEX_VAR is abused to signify references to columns of a custom scan tuple type" But INDEX_VAR has a

Re: Making Vars outer-join aware

2023-02-13 Thread Justin Pryzby
On Mon, Feb 13, 2023 at 03:33:15PM +0800, Richard Guo wrote: > On Mon, Feb 13, 2023 at 7:58 AM Justin Pryzby wrote: > > The patch broke this query: > > > > select from pg_inherits inner join information_schema.element_types > > right join (select from pg_constraint as sample_2) on true > > on

Re: Making Vars outer-join aware

2023-02-13 Thread Tom Lane
Richard Guo 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

Re: Making Vars outer-join aware

2023-02-12 Thread Richard Guo
On Mon, Feb 13, 2023 at 7:58 AM Justin Pryzby wrote: > The patch broke this query: > > select from pg_inherits inner join information_schema.element_types > right join (select from pg_constraint as sample_2) on true > on false, lateral (select scope_catalog, inhdetachpending from >

Re: Making Vars outer-join aware

2023-02-12 Thread Justin Pryzby
On Mon, Jan 23, 2023 at 03:38:06PM -0500, Tom Lane wrote: > Richard, are you planning to review this any more? I'm getting > a little antsy to get it committed. For such a large patch, > it's surprising it's had so few conflicts to date. The patch broke this query: select from pg_inherits

Re: Making Vars outer-join aware

2023-01-30 Thread Tom Lane
Richard Guo writes: > Sorry for the delayed reply. I don't have any more review comments at > the moment, except a nitpicking one. > In optimizer/README at line 729 there is a query as > SELECT * FROM a > LEFT JOIN (SELECT * FROM b WHERE b.z = 1) ss ON (a.x = b.y) > WHERE a.x =

Re: Making Vars outer-join aware

2023-01-30 Thread Richard Guo
On Tue, Jan 24, 2023 at 4:38 AM Tom Lane wrote: > Richard, are you planning to review this any more? I'm getting > a little antsy to get it committed. For such a large patch, > it's surprising it's had so few conflicts to date. Sorry for the delayed reply. I don't have any more review

Re: Making Vars outer-join aware

2023-01-24 Thread David G. Johnston
On Tue, Jan 24, 2023 at 1:25 PM Tom Lane wrote: > "David G. Johnston" writes: > > On Tue, Jan 24, 2023 at 12:31 PM Tom Lane wrote: > >> select ... from t1 left join t2 on (t1.x = t2.y and t1.x = 1); > >> > >> If we turn the generic equivclass.c logic loose on these clauses, > >> it will deduce

Re: Making Vars outer-join aware

2023-01-24 Thread Tom Lane
"David G. Johnston" writes: > On Tue, Jan 24, 2023 at 12:31 PM Tom Lane wrote: >> select ... from t1 left join t2 on (t1.x = t2.y and t1.x = 1); >> >> If we turn the generic equivclass.c logic loose on these clauses, >> it will deduce t2.y = 1, which is good, and then apply t2.y = 1 at >> the

Re: Making Vars outer-join aware

2023-01-24 Thread David G. Johnston
On Tue, Jan 24, 2023 at 12:31 PM Tom Lane wrote: > I wrote: > > Hans Buschmann writes: > >> I just noticed your new efforts in this area. > >> I wanted to recurr to my old thread [1] considering constant > propagation of quals. > >> [1] >

Re: Making Vars outer-join aware

2023-01-24 Thread Tom Lane
I wrote: > Hans Buschmann writes: >> I just noticed your new efforts in this area. >> I wanted to recurr to my old thread [1] considering constant propagation of >> quals. >> [1] https://www.postgresql.org/message-id/1571413123735.26...@nidsa.net > Yeah, this patch series is not yet quite up

Re: Making Vars outer-join aware

2023-01-24 Thread Tom Lane
Hans Buschmann writes: > I just noticed your new efforts in this area. > I wanted to recurr to my old thread [1] considering constant propagation of > quals. > [1] https://www.postgresql.org/message-id/1571413123735.26...@nidsa.net Yeah, this patch series is not yet quite up to the point of

Re: Making Vars outer-join aware

2023-01-24 Thread Hans Buschmann
Hello Tom I just noticed your new efforts in this area. I wanted to recurr to my old thread [1] considering constant propagation of quals. You gave an elaborated explanation at that time, but my knowledge was/is not yet sufficient to reveil the technical details. In our application the

Re: Making Vars outer-join aware

2022-12-28 Thread Tom Lane
Richard Guo writes: > I think I see where the problem is. And I can see currently in > get_eclass_for_sort_expr we always use the top JoinDomain. So although > the equality clause 'a.x = b.y' belongs to JoinDomain {B}, we set up ECs > for 'a.x' and 'b.y' that belong to the top JoinDomain {A, B,

Re: Making Vars outer-join aware

2022-12-28 Thread Richard Guo
On Tue, Dec 27, 2022 at 11:31 PM Tom Lane wrote: > The thing that I couldn't get around before is that if you have, > say, a mergejoinable equality clause in an outer join: > > select ... from a left join b on a.x = b.y; > > that equality clause can only be associated with the join domain >

Re: Making Vars outer-join aware

2022-12-27 Thread Tom Lane
Richard Guo writes: > For 0012, I'm still trying to understand JoinDomain. AFAIU all EC > members of the same EC should have the same JoinDomain, because for > constants we match EC members only within the same JoinDomain, and for > Vars if they come from different join domains they will have

Re: Making Vars outer-join aware

2022-12-27 Thread Richard Guo
On Sat, Dec 24, 2022 at 2:20 AM Tom Lane wrote: > I shoved some preliminary refactoring into the 0001 patch, > notably splitting deconstruct_jointree into two passes. > 0002-0009 cover the same ground as they did before, though > with some differences in detail. 0010-0012 are new work > mostly

Re: Making Vars outer-join aware

2022-12-23 Thread Tom Lane
Ted Yu writes: > For v8-0012-invent-join-domains.patch, in `distribute_qual_to_rels`, it > seems that `pseudoconstant` and `root->hasPseudoConstantQuals` carry the > same value. > Can the variable `pseudoconstant` be omitted ? Surely not. 'pseudoconstant' tells whether the current qual clause

Re: Making Vars outer-join aware

2022-12-23 Thread Ted Yu
On Fri, Dec 23, 2022 at 10:21 AM Tom Lane wrote: > Here's a new edition of this patch series. > > I shoved some preliminary refactoring into the 0001 patch, > notably splitting deconstruct_jointree into two passes. > 0002-0009 cover the same ground as they did before, though > with some

Re: Making Vars outer-join aware

2022-11-17 Thread Richard Guo
On Thu, Nov 17, 2022 at 4:46 AM Tom Lane wrote: > So we've marked the 4 and 7 joins as possibly commuting, but they > cannot commute according to 7's min_lefthand set. I don't think > the extra clone condition is terribly harmful --- it's useless > but shouldn't cause any problems. However, if

Re: Making Vars outer-join aware

2022-11-16 Thread Tom Lane
Richard Guo writes: > I'm reviewing the part about multiple version clauses, and I find a case > that may not work as expected. I tried with some query as below > (A leftjoin (B leftjoin C on (Pbc)) on (Pab)) left join D on (Pcd) > Assume Pbc is strict for B and Pcd is strict for C. > According

Re: Making Vars outer-join aware

2022-11-15 Thread Richard Guo
On Sun, Nov 6, 2022 at 5:53 AM Tom Lane wrote: > I wrote: > > I've been working away at this patch series, and here is an up-to-date > > version. > > This needs a rebase after ff8fa0bf7 and b0b72c64a. I also re-ordered > the patches so that the commit messages' claims about when regression >

Re: Making Vars outer-join aware

2022-11-10 Thread Richard Guo
On Thu, Aug 25, 2022 at 6:27 PM Richard Guo wrote: > I'm not sure if I understand it correctly. If we are given the first > order from the parser, the SpecialJoinInfo for the B/C join would have > min_lefthand as containing both B and the A/B join. And this > SpecialJoinInfo would make the B/C

Re: Making Vars outer-join aware

2022-08-29 Thread Richard Guo
On Mon, Aug 29, 2022 at 2:30 PM Richard Guo wrote: > > On Fri, Aug 19, 2022 at 2:45 AM Tom Lane wrote: > >> Here's a rebase up to HEAD, mainly to get the cfbot back in sync >> as to what's the live patch. > > > Noticed another different behavior from previous. When we try to reduce > JOIN_LEFT

Re: Making Vars outer-join aware

2022-08-29 Thread Richard Guo
On Fri, Aug 19, 2022 at 2:45 AM Tom Lane wrote: > Here's a rebase up to HEAD, mainly to get the cfbot back in sync > as to what's the live patch. Noticed another different behavior from previous. When we try to reduce JOIN_LEFT to JOIN_ANTI, we want to know if the join's own quals are strict

Re: Making Vars outer-join aware

2022-08-25 Thread Richard Guo
On Thu, Aug 25, 2022 at 5:18 AM Tom Lane wrote: > Richard Guo writes: > > Do we need to also > > generate two SpecialJoinInfos for the B/C join in the first order, with > > and without the A/B join in its min_lefthand? > > No, the SpecialJoinInfos would stay as they are now. It's already the >

Re: Making Vars outer-join aware

2022-08-24 Thread Tom Lane
Richard Guo writes: > On Sun, Aug 21, 2022 at 6:52 AM Tom Lane wrote: >> What I'm thinking we should do about this, once we detect that >> this identity is applicable, is to generate *both* forms of Pbc, >> either adding or removing the varnullingrels bits depending on >> which form we got from

Re: Making Vars outer-join aware

2022-08-24 Thread Richard Guo
On Sun, Aug 21, 2022 at 6:52 AM Tom Lane wrote: > What I'm thinking we should do about this, once we detect that > this identity is applicable, is to generate *both* forms of Pbc, > either adding or removing the varnullingrels bits depending on > which form we got from the parser. Then, when we

Re: Making Vars outer-join aware

2022-08-20 Thread Tom Lane
Progress report on this ... I've been trying to remove some of the cruftier aspects of EquivalenceClasses (which really is the main point of this whole effort), and gotten repeatedly blocked by the fact that the semantics are still a bit crufty thanks to the hacking associated with outer join

Re: Making Vars outer-join aware

2022-08-17 Thread Tom Lane
Richard Guo writes: > BTW, the comment just above the two calls to build_joinrel_tlist says: > * Create a new tlist containing just the vars that need to be output from > Here by 'vars' it means both plain Vars and PlaceHolderVars, right? If > not we may need to adjust this comment to also

Re: Making Vars outer-join aware

2022-08-17 Thread Richard Guo
On Wed, Aug 17, 2022 at 4:57 AM Tom Lane wrote: > On further thought, it seems better to maintain the index array > from the start, allowing complete replacement of the linear list > searches. We can add a separate bool flag to denote frozen-ness. +1 for 0001 patch. Now we process plain Vars

Re: Making Vars outer-join aware

2022-08-16 Thread Tom Lane
I wrote: > ... We can fix > that by adding an index array to go straight from phid to the > PlaceHolderInfo. While thinking about where to construct such > an index array, I decided it'd be a good idea to have an explicit > step to "freeze" the set of PlaceHolderInfos, at the start of >

Re: Making Vars outer-join aware

2022-08-16 Thread Tom Lane
I wrote: > Richard Guo writes: >> If the two issues are indeed something we need to fix, maybe we can >> change add_placeholders_to_joinrel() to search the PHVs from >> outer_rel/inner_rel's targetlist, and add the ojrelid to phnullingrels >> if needed, just like what we do in

Re: Making Vars outer-join aware

2022-08-16 Thread Tom Lane
Richard Guo writes: > When we add required PlaceHolderVars to a join rel's targetlist, if the > PHV can be computed in the nullable-side of the join, we would add the > join's RT index to phnullingrels. This is correct as we know the PHV's > value can be nulled at this join. But I'm wondering if

Re: Making Vars outer-join aware

2022-08-15 Thread Richard Guo
On Tue, Aug 2, 2022 at 3:51 AM Tom Lane wrote: > Here's a rebase up to HEAD, mostly to placate the cfbot. > I accounted for d8e34fa7a (s/all_baserels/all_query_rels/ > in those places) and made one tiny bug-fix change. > Nothing substantive as yet. When we add required PlaceHolderVars to a

Re: Making Vars outer-join aware

2022-08-01 Thread Tom Lane
Zhihong Yu writes: > For v3-0003-label-Var-nullability-in-parser.patch : > + if (rtindex > 0 && rtindex <= list_length(pstate->p_nullingrels)) > + relids = (Bitmapset *) list_nth(pstate->p_nullingrels, rtindex - 1); > + else > + relids = NULL; > + > + /* > +* Merge with any

Re: Making Vars outer-join aware

2022-08-01 Thread Zhihong Yu
On Mon, Aug 1, 2022 at 12:51 PM Tom Lane wrote: > Here's a rebase up to HEAD, mostly to placate the cfbot. > I accounted for d8e34fa7a (s/all_baserels/all_query_rels/ > in those places) and made one tiny bug-fix change. > Nothing substantive as yet. > > regards, tom lane

Re: Making Vars outer-join aware

2022-07-13 Thread Tom Lane
Richard Guo writes: > But I'm not sure which is better, to evaluate the expression below or > above the outer join. It seems to me that if the size of base rel is > large and somehow the size of the joinrel is small, evaluation above the > outer join would win. And in the opposite case evaluation

Re: Making Vars outer-join aware

2022-07-13 Thread Richard Guo
On Tue, Jul 12, 2022 at 9:37 PM Tom Lane wrote: > Richard Guo writes: > > Note that the evaluation of expression 'b.j + 1' now occurs below the > > outer join. Is this something we need to be concerned about? > > It seems more formally correct to me, but perhaps somebody would > complain about

Re: Making Vars outer-join aware

2022-07-12 Thread Tom Lane
Richard Guo writes: > Note that the evaluation of expression 'b.j + 1' now occurs below the > outer join. Is this something we need to be concerned about? It seems more formally correct to me, but perhaps somebody would complain about possibly-useless expression evals. We could likely

Re: Making Vars outer-join aware

2022-07-12 Thread Richard Guo
On Mon, Jul 11, 2022 at 3:38 AM Tom Lane wrote: > Here's v2 of this patch series. It's functionally identical to v1, > but I've rebased it over the recent auto-node-support-generation > changes, and also extracted a few separable bits in hopes of making > the main planner patch smaller. (It's

Re: Making Vars outer-join aware

2022-07-10 Thread Tom Lane
Zhihong Yu writes: > In remove_unneeded_nulling_relids(): > + if (removable_relids == NULL) > Why is bms_is_empty() not used in the above check ? We initialized that to NULL just a few lines above, and then did nothing to it other than perhaps bms_add_member, so it's impossible for it to be

Re: Making Vars outer-join aware

2022-07-10 Thread Zhihong Yu
On Sun, Jul 10, 2022 at 12:39 PM Tom Lane wrote: > Here's v2 of this patch series. It's functionally identical to v1, > but I've rebased it over the recent auto-node-support-generation > changes, and also extracted a few separable bits in hopes of making > the main planner patch smaller. (It's

Re: Making Vars outer-join aware

2022-07-05 Thread Tom Lane
Richard Guo writes: > For the query in the example > SELECT * FROM t1 LEFT JOIN t2 ON (t1.x = t2.y) WHERE foo(t2.z) > (foo() is not strict.) We want to avoid pushing foo(t2.z) down to the t2 > scan level. Previously we do that with check_outerjoin_delay() by > scanning all the outer joins

Re: Making Vars outer-join aware

2022-07-05 Thread Richard Guo
On Sat, Jul 2, 2022 at 12:42 AM Tom Lane wrote: > > Anyway, even though this is far from done, I'm pretty pleased with > the results so far, so I thought I'd put it out for review by > anyone who cares to take a look. I'll add it to the September CF > in hopes that it might be more or less

Re: Making Vars outer-join aware

2022-07-01 Thread Tom Lane
"Finnerty, Jim" writes: > Given that views are represented in a parsed representation, does > anything need to happen to the Vars inside a view when that view is > outer-joined to? No. The markings only refer to what is in the same Query tree as the Var itself. Subquery flattening

Re: Making Vars outer-join aware

2022-07-01 Thread Finnerty, Jim
Tom, two quick questions before attempting to read the patch: Given that views are represented in a parsed representation, does anything need to happen to the Vars inside a view when that view is outer-joined to? If an outer join is converted to an inner join, must this information