Re: Removing unneeded self joins

On Mon, May 6, 2024 at 3:27 PM Alexander Korotkov wrote: > I agree it was a hurry to put the plan into commit message. I think > Tom already gave valuable feedback [1] and probably we will get more. > So, plan is to be decided. One way or the other I'm not going to > re-commit this without

Re: Removing unneeded self joins

On Mon, May 6, 2024 at 5:44 PM Robert Haas wrote: > I want to go on record right now as disagreeing with the plan proposed > in the commit message for the revert commit, namely, committing this > again early in the v18 cycle. I don't think Tom would have proposed > reverting this feature unless

Re: Removing unneeded self joins

On Mon, May 6, 2024 at 6:54 PM Tom Lane wrote: > Robert Haas writes: > > I want to go on record right now as disagreeing with the plan proposed > > in the commit message for the revert commit, namely, committing this > > again early in the v18 cycle. I don't think Tom would have proposed > >

Re: Removing unneeded self joins

On Mon, May 6, 2024 at 12:24:41PM -0400, Robert Haas wrote: > Now that being said, I do also agree that the planner code is quite > hard to understand, for various reasons. I don't think the structure > of that code and the assumptions underlying it are as well-documented > as they could be, and

Re: Removing unneeded self joins

On Mon, May 6, 2024 at 12:01 PM Andrei Lepikhov wrote: > Right now, it evolves extensively - new structures, variables, > alternative copies of the same node trees with slightly changed > properties ... This way allows us to quickly introduce some planning > features (a lot of changes in planner

Re: Removing unneeded self joins

On 6/5/2024 21:44, Robert Haas wrote: On Sat, May 4, 2024 at 10:46 PM Andrei Lepikhov wrote: Having no objective negative feedback, we have no reason to change anything in the design or any part of the code. It looks regrettable and unusual. To me, this sounds like you think it's someone

Re: Removing unneeded self joins

Robert Haas writes: > I want to go on record right now as disagreeing with the plan proposed > in the commit message for the revert commit, namely, committing this > again early in the v18 cycle. I don't think Tom would have proposed > reverting this feature unless he believed that it had more

Re: Removing unneeded self joins

On Mon, May 6, 2024 at 10:44:33AM -0400, Robert Haas wrote: > I want to go on record right now as disagreeing with the plan proposed > in the commit message for the revert commit, namely, committing this > again early in the v18 cycle. I don't think Tom would have proposed > reverting this

Re: Removing unneeded self joins

On Sat, May 4, 2024 at 10:46 PM Andrei Lepikhov wrote: > Having no objective negative feedback, we have no reason to change > anything in the design or any part of the code. It looks regrettable and > unusual. To me, this sounds like you think it's someone else's job to tell you what is wrong

Re: Removing unneeded self joins

On 3/5/2024 20:55, Robert Haas wrote: One of my most embarrassing gaffes in this area personally was a448e49bcbe40fb72e1ed85af910dd216d45bad8. I don't know how I managed to commit the original patch without realizing it was going to cause an increase in the WAL size, but I can tell you that when

Re: Removing unneeded self joins

On Fri, May 3, 2024 at 4:57 AM Alexander Korotkov wrote: > I agree to revert it for v17, but I'm not exactly sure the issue is > design (nevertheless design review is very welcome as any other type > of review). The experience of the bugs arising with the SJE doesn't > show me a particular weak

Re: Removing unneeded self joins

Hi, Tom! On Fri, May 3, 2024 at 2:19 AM Tom Lane wrote: > Alexander Korotkov writes: > > I would appreciate your review of this patchset, and review from Andrei as > > well. > > I hate to say this ... but if we're still finding bugs this > basic in SJE, isn't it time to give up on it for v17?

Re: Removing unneeded self joins

On 5/3/24 06:19, Tom Lane wrote: Alexander Korotkov writes: I would appreciate your review of this patchset, and review from Andrei as well. I hate to say this ... but if we're still finding bugs this basic in SJE, isn't it time to give up on it for v17? I might feel better about it if

Re: Removing unneeded self joins

Alexander Korotkov writes: > I would appreciate your review of this patchset, and review from Andrei as > well. I hate to say this ... but if we're still finding bugs this basic in SJE, isn't it time to give up on it for v17? I might feel better about it if there were any reason to think these

Re: Removing unneeded self joins

Hi, Richard! On Thu, May 2, 2024 at 4:14 PM Richard Guo wrote: > On Thu, May 2, 2024 at 6:08 PM Alexander Korotkov > wrote: >> On Thu, May 2, 2024 at 12:45 PM Andrei Lepikhov >> wrote: >> > One question for me is: Do we anticipate other lateral self-references >> > except the TABLESAMPLE

Re: Removing unneeded self joins

On Thu, May 2, 2024 at 6:08 PM Alexander Korotkov wrote: > On Thu, May 2, 2024 at 12:45 PM Andrei Lepikhov > wrote: > > One question for me is: Do we anticipate other lateral self-references > > except the TABLESAMPLE case? Looking into the extract_lateral_references > > implementation, I see

Re: Removing unneeded self joins

On Thu, May 2, 2024 at 12:45 PM Andrei Lepikhov wrote: > > On 5/1/24 18:59, Alexander Korotkov wrote: > > I think we probably could forbid SJE for the tables with TABLESAMPLE > > altogether. Please, check the attached patch. > Your patch looks good to me. I added some comments and test case into

Re: Removing unneeded self joins

On 5/1/24 18:59, Alexander Korotkov wrote: I think we probably could forbid SJE for the tables with TABLESAMPLE altogether. Please, check the attached patch. Your patch looks good to me. I added some comments and test case into the join.sql. One question for me is: Do we anticipate other

Re: Removing unneeded self joins

On Wed, May 1, 2024 at 2:00 PM Alexander Lakhin wrote: > 30.04.2024 13:20, Alexander Korotkov wrote: > > On Tue, Apr 30, 2024 at 9:00 AM Alexander Lakhin > > wrote: > >> I've discovered another failure, introduced by d3d55ce57. > >> Please try the following: > >> CREATE TABLE t (a int unique, b

Re: Removing unneeded self joins

30.04.2024 13:20, Alexander Korotkov wrote: On Tue, Apr 30, 2024 at 9:00 AM Alexander Lakhin wrote: I've discovered another failure, introduced by d3d55ce57. Please try the following: CREATE TABLE t (a int unique, b float); SELECT * FROM t NATURAL JOIN LATERAL (SELECT * FROM t t2

Re: Removing unneeded self joins

On Tue, Apr 30, 2024 at 9:00 AM Alexander Lakhin wrote: > 23.10.2023 12:47, Alexander Korotkov wrote: > > I think this patch makes substantial improvement to query planning. > > It has received plenty of reviews. The code is currently in quite > > good shape. I didn't manage to find the cases

Re: Removing unneeded self joins

Hi, Alexander! On Tue, Apr 30, 2024 at 9:00 AM Alexander Lakhin wrote: > 23.10.2023 12:47, Alexander Korotkov wrote: > > I think this patch makes substantial improvement to query planning. > > It has received plenty of reviews. The code is currently in quite > > good shape. I didn't manage to

Re: Removing unneeded self joins

Hello Alexander, 23.10.2023 12:47, Alexander Korotkov wrote: I think this patch makes substantial improvement to query planning. It has received plenty of reviews. The code is currently in quite good shape. I didn't manage to find the cases when this optimization causes significant overhead

Re: Removing unneeded self joins

Hello, On Sat, Feb 24, 2024 at 01:02:01PM +0200, Alexander Korotkov wrote: > On Sat, Feb 24, 2024 at 7:12 AM Noah Misch wrote: > > On Sat, Feb 24, 2024 at 12:36:59AM +0200, Alexander Korotkov wrote: > > > On Thu, Feb 22, 2024 at 10:51 AM Andrei Lepikhov > > > wrote: > > > > On 21/2/2024 14:26,

Re: Removing unneeded self joins

Hi, Noah! On Sat, Feb 24, 2024 at 7:12 AM Noah Misch wrote: > On Sat, Feb 24, 2024 at 12:36:59AM +0200, Alexander Korotkov wrote: > > On Thu, Feb 22, 2024 at 10:51 AM Andrei Lepikhov > > wrote: > > > On 21/2/2024 14:26, Richard Guo wrote: > > > > I think the right fix for these issues is to

Re: Removing unneeded self joins

On Sat, Feb 24, 2024 at 12:36:59AM +0200, Alexander Korotkov wrote: > On Thu, Feb 22, 2024 at 10:51 AM Andrei Lepikhov > wrote: > > On 21/2/2024 14:26, Richard Guo wrote: > > > I think the right fix for these issues is to introduce a new element > > > 'sublevels_up' in ReplaceVarnoContext, and

Re: Removing unneeded self joins

On Thu, Feb 22, 2024 at 10:51 AM Andrei Lepikhov wrote: > On 21/2/2024 14:26, Richard Guo wrote: > > I think the right fix for these issues is to introduce a new element > > 'sublevels_up' in ReplaceVarnoContext, and enhance replace_varno_walker > > to 1) recurse into subselects with sublevels_up

Re: Removing unneeded self joins

On 21/2/2024 14:26, Richard Guo wrote: I think the right fix for these issues is to introduce a new element 'sublevels_up' in ReplaceVarnoContext, and enhance replace_varno_walker to 1) recurse into subselects with sublevels_up increased, and 2) perform the replacement only when varlevelsup is

Re: Removing unneeded self joins

On 21/2/2024 14:26, Richard Guo wrote: This patch also includes some cosmetic tweaks for SJE test cases.  It does not change the test cases using catalog tables though.  I think that should be a seperate patch. Thanks for this catch, it is really messy thing, keeping aside the question why we

Re: Removing unneeded self joins

On Mon, Feb 19, 2024 at 1:24 PM Andrei Lepikhov wrote: > On 18/2/2024 23:18, Alexander Korotkov wrote: > > On Sun, Feb 18, 2024 at 5:04 PM Alexander Korotkov > wrote: > >> On Sun, Feb 18, 2024 at 3:00 PM Alexander Lakhin > wrote: > >>> Please look at the following query which fails with an

Re: Removing unneeded self joins

On 18/2/2024 23:18, Alexander Korotkov wrote: On Sun, Feb 18, 2024 at 5:04 PM Alexander Korotkov wrote: On Sun, Feb 18, 2024 at 3:00 PM Alexander Lakhin wrote: 09.01.2024 01:09, Alexander Korotkov wrote: Fixed in 30b4955a46. Please look at the following query which fails with an error

Re: Removing unneeded self joins

18.02.2024 19:18, Alexander Korotkov wrote: Attached is a draft patch fixing this query. Could you, please, recheck? Yes, this patch fixes the behavior for that query (I've also tried several similar queries). Though I don't know the code well enough to judge the code change. Best regards,

Re: Removing unneeded self joins

On Sun, Feb 18, 2024 at 5:04 PM Alexander Korotkov wrote: > On Sun, Feb 18, 2024 at 3:00 PM Alexander Lakhin wrote: > > 09.01.2024 01:09, Alexander Korotkov wrote: > > > > Fixed in 30b4955a46. > > > > > > Please look at the following query which fails with an error since > > d3d55ce57: > > > >

Re: Removing unneeded self joins

On Sun, Feb 18, 2024 at 3:00 PM Alexander Lakhin wrote: > 09.01.2024 01:09, Alexander Korotkov wrote: > > Fixed in 30b4955a46. > > > Please look at the following query which fails with an error since > d3d55ce57: > > create table t (i int primary key); > > select t3.i from t t1 > join t t2 on

Re: Removing unneeded self joins

Hi Alexander, 09.01.2024 01:09, Alexander Korotkov wrote: Fixed in 30b4955a46. Please look at the following query which fails with an error since d3d55ce57: create table t (i int primary key); select t3.i from t t1  join t t2 on t1.i = t2.i,  lateral (select t1.i limit 1) t3; ERROR: 

Re: Removing unneeded self joins

On Tue, Jan 9, 2024 at 8:08 AM Alexander Korotkov wrote: > On Tue, Jan 9, 2024 at 6:00 AM Alexander Lakhin wrote: > > 09.01.2024 01:09, Alexander Korotkov wrote: > > > Fixed in 30b4955a46. > > > > Thank you for fixing that! > > > > I've found another anomaly coined with d3d55ce57. This query: >

Re: Removing unneeded self joins

On Tue, Jan 9, 2024 at 6:00 AM Alexander Lakhin wrote: > 09.01.2024 01:09, Alexander Korotkov wrote: > > Fixed in 30b4955a46. > > Thank you for fixing that! > > I've found another anomaly coined with d3d55ce57. This query: > CREATE TABLE t(a int PRIMARY KEY, b int); > INSERT INTO t VALUES (1,

Re: Removing unneeded self joins

09.01.2024 01:09, Alexander Korotkov wrote: Fixed in 30b4955a46. Thank you for fixing that! I've found another anomaly coined with d3d55ce57. This query: CREATE TABLE t(a int PRIMARY KEY, b int); INSERT INTO t VALUES  (1, 1), (2, 1); WITH t1 AS (SELECT * FROM t) UPDATE t SET b = t1.b + 1

Re: Removing unneeded self joins

On Mon, Jan 8, 2024 at 10:20 PM Alexander Korotkov wrote: > On Mon, Jan 8, 2024 at 10:00 PM Alexander Lakhin wrote: > > Please look at the following query which produces an incorrect result since > > d3d55ce57: > > CREATE TABLE t(a int PRIMARY KEY, b int); > > INSERT INTO t VALUES (1, 1), (2,

Re: Removing unneeded self joins

On Mon, Jan 8, 2024 at 10:00 PM Alexander Lakhin wrote: > Please look at the following query which produces an incorrect result since > d3d55ce57: > CREATE TABLE t(a int PRIMARY KEY, b int); > INSERT INTO t VALUES (1, 1), (2, 1); > SELECT * FROM t WHERE EXISTS (SELECT * FROM t t2 WHERE t2.a =

Re: Removing unneeded self joins

Hello Andrei and Alexander, Please look at the following query which produces an incorrect result since d3d55ce57: CREATE TABLE t(a int PRIMARY KEY, b int); INSERT INTO t VALUES  (1, 1), (2, 1); SELECT * FROM t WHERE EXISTS (SELECT * FROM t t2 WHERE t2.a = t.b AND t2.b > 0);  a | b ---+---  1 |

Re: Removing unneeded self joins

Hi Andrei, 29.12.2023 12:58, Andrei Lepikhov wrote: Thanks for the report! The problem is with the resultRelation field. We forget to replace the relid here. Could you check your issue with the patch in the attachment? Does it resolve this case? Yes, with the patch applied I see no error.

Re: Removing unneeded self joins

On 29/12/2023 12:00, Alexander Lakhin wrote: Hi Alexander, 23.10.2023 14:29, Alexander Korotkov wrote: Fixed all of the above. Thank you for catching this! I've discovered that starting from d3d55ce57 the following query: CREATE TABLE t(a int PRIMARY KEY); WITH tt AS (SELECT * FROM t)

Re: Removing unneeded self joins

Hi Alexander, 23.10.2023 14:29, Alexander Korotkov wrote: Fixed all of the above. Thank you for catching this! I've discovered that starting from d3d55ce57 the following query: CREATE TABLE t(a int PRIMARY KEY); WITH tt AS (SELECT * FROM t) UPDATE t SET a = tt.a + 1 FROM tt WHERE tt.a = t.a

Re: Removing unneeded self joins

On Mon, Oct 23, 2023 at 2:00 PM Alexander Lakhin wrote: > 23.10.2023 12:47, Alexander Korotkov wrote: > > I think this patch makes substantial improvement to query planning. > > It has received plenty of reviews. The code is currently in quite > > good shape. I didn't manage to find the cases

Re: Removing unneeded self joins

Hi Alexander, 23.10.2023 12:47, Alexander Korotkov wrote: I think this patch makes substantial improvement to query planning. It has received plenty of reviews. The code is currently in quite good shape. I didn't manage to find the cases when this optimization causes significant overhead to

Re: Removing unneeded self joins

On Mon, Oct 23, 2023 at 6:43 AM Andrei Lepikhov wrote: > On 22/10/2023 05:01, Alexander Korotkov wrote: > > On Thu, Oct 19, 2023 at 6:16 AM Andrei Lepikhov > > wrote: > >> On 19/10/2023 01:50, Alexander Korotkov wrote: > >>> This query took 3778.432 ms with self-join removal disabled, and > >>>

Re: Removing unneeded self joins

On 22/10/2023 05:01, Alexander Korotkov wrote: On Thu, Oct 19, 2023 at 6:16 AM Andrei Lepikhov wrote: On 19/10/2023 01:50, Alexander Korotkov wrote: This query took 3778.432 ms with self-join removal disabled, and 3756.009 ms with self-join removal enabled. So, no measurable overhead.

Re: Removing unneeded self joins

On Thu, Oct 19, 2023 at 6:16 AM Andrei Lepikhov wrote: > On 19/10/2023 01:50, Alexander Korotkov wrote: > > This query took 3778.432 ms with self-join removal disabled, and > > 3756.009 ms with self-join removal enabled. So, no measurable > > overhead. Similar to the higher number of joins.

Re: Removing unneeded self joins

On 19/10/2023 01:50, Alexander Korotkov wrote: On Mon, Oct 16, 2023 at 11:28 AM Andrei Lepikhov wrote: On 12/10/2023 18:32, Alexander Korotkov wrote: On Thu, Oct 5, 2023 at 12:17 PM Andrei Lepikhov wrote: On 4/10/2023 14:34, Alexander Korotkov wrote: > Relid replacement machinery is the

Re: Removing unneeded self joins

On Mon, Oct 16, 2023 at 11:28 AM Andrei Lepikhov wrote: > On 12/10/2023 18:32, Alexander Korotkov wrote: > > On Thu, Oct 5, 2023 at 12:17 PM Andrei Lepikhov > > wrote: > >> On 4/10/2023 14:34, Alexander Korotkov wrote: > >>> > Relid replacement machinery is the most contradictory code here. We

Re: Removing unneeded self joins

On 12/10/2023 18:32, Alexander Korotkov wrote: On Thu, Oct 5, 2023 at 12:17 PM Andrei Lepikhov wrote: On 4/10/2023 14:34, Alexander Korotkov wrote: > Relid replacement machinery is the most contradictory code here. We used > a utilitarian approach and implemented a simplistic variant.

Re: Removing unneeded self joins

On 13.10.2023 12:03, Andrei Lepikhov wrote: On 13/10/2023 15:56, a.rybakina wrote: Also I've incorporated improvements from Alena Rybakina except one for skipping SJ removal when no SJ quals is found.  It's not yet clear for me if this check fix some cases. But at least optimization got

Re: Removing unneeded self joins

On 13/10/2023 15:56, a.rybakina wrote: Also I've incorporated improvements from Alena Rybakina except one for skipping SJ removal when no SJ quals is found.  It's not yet clear for me if this check fix some cases. But at least optimization got skipped in some useful cases (as you can see in

Re: Removing unneeded self joins

Also I've incorporated improvements from Alena Rybakina except one for skipping SJ removal when no SJ quals is found.  It's not yet clear for me if this check fix some cases. But at least optimization got skipped in some useful cases (as you can see in regression tests). Agree. I wouldn't say

Re: Removing unneeded self joins

On 12/10/2023 18:32, Alexander Korotkov wrote: On Thu, Oct 5, 2023 at 12:17 PM Andrei Lepikhov We have almost the results we wanted to have. But in the last explain you can see that nothing happened with the OR clause. We should use the expression mutator instead of walker to handle such

Re: Removing unneeded self joins

On Thu, Oct 5, 2023 at 12:17 PM Andrei Lepikhov wrote: > On 4/10/2023 14:34, Alexander Korotkov wrote: > > > Relid replacement machinery is the most contradictory code here. We used > > > a utilitarian approach and implemented a simplistic variant. > > > > > > 2) It would be nice to skip the

Re: Removing unneeded self joins

On 11/10/2023 02:29, Alena Rybakina wrote: I have reviewed your patch and I noticed a few things. Thanks for your efforts, I have looked at the latest version of the code, I assume that the error lies in the replace_varno_walker function, especially in the place where we check the node by

Re: Removing unneeded self joins

Hi! I have reviewed your patch and I noticed a few things. First of all, I think I found a bug in your latest patch version, and this query shows it: EXPLAIN (COSTS OFF) SELECT c.oid, e.oid FROM pg_class c FULL JOIN (   SELECT e1.oid FROM pg_extension e1, pg_extension e2   WHERE

Re: Removing unneeded self joins

On 4/10/2023 14:34, Alexander Korotkov wrote: > Relid replacement machinery is the most contradictory code here. We used > a utilitarian approach and implemented a simplistic variant. > > 2) It would be nice to skip the insertion of IS NOT NULL checks when > > they are not necessary.  [1]

Re: Removing unneeded self joins

On 4/10/2023 14:34, Alexander Korotkov wrote: Hi! On Wed, Oct 4, 2023 at 9:56 AM Andrei Lepikhov mailto:a.lepik...@postgrespro.ru>> wrote: > On 4/10/2023 07:12, Alexander Korotkov wrote: > > On Tue, Sep 12, 2023 at 4:58 PM Andrey Lepikhov > > mailto:a.lepik...@postgrespro.ru>> wrote: > >>

Re: Removing unneeded self joins

Hi! On Wed, Oct 4, 2023 at 9:56 AM Andrei Lepikhov wrote: > On 4/10/2023 07:12, Alexander Korotkov wrote: > > On Tue, Sep 12, 2023 at 4:58 PM Andrey Lepikhov > > wrote: > >> On 5/7/2023 21:28, Andrey Lepikhov wrote: > >>> During the significant code revision in v.41 I lost some replacement >

Re: Removing unneeded self joins

On 4/10/2023 07:12, Alexander Korotkov wrote: Hi! Thanks for the review! I think this is a neat optimization. On Tue, Sep 12, 2023 at 4:58 PM Andrey Lepikhov wrote: On 5/7/2023 21:28, Andrey Lepikhov wrote: During the significant code revision in v.41 I lost some replacement operations.

Re: Removing unneeded self joins

Hi! I think this is a neat optimization. On Tue, Sep 12, 2023 at 4:58 PM Andrey Lepikhov wrote: > On 5/7/2023 21:28, Andrey Lepikhov wrote: > > During the significant code revision in v.41 I lost some replacement > > operations. Here is the fix and extra tests to check this in the future. > >

Re: Removing unneeded self joins

On 5/7/2023 21:28, Andrey Lepikhov wrote: Hi, During the significant code revision in v.41 I lost some replacement operations. Here is the fix and extra tests to check this in the future. Also, Tom added the JoinDomain structure five months ago, and I added code to replace relids for that

Re: Removing unneeded self joins

Hi, During the significant code revision in v.41 I lost some replacement operations. Here is the fix and extra tests to check this in the future. Also, Tom added the JoinDomain structure five months ago, and I added code to replace relids for that place too. One more thing, I found out that we

Re: Removing unneeded self joins

On 4/4/2023 02:30, Gregory Stark (as CFM) wrote: On Mon, 6 Mar 2023 at 00:30, Michał Kłeczek wrote: Hi All, I just wanted to ask about the status and plans for this patch. I can see it being stuck at “Waiting for Author” status in several commit tests. Sadly it seems to now be badly in

Re: Removing unneeded self joins

On 3/6/23 10:30, Michał Kłeczek wrote: > Hi All, > > I just wanted to ask about the status and plans for this patch. > I can see it being stuck at “Waiting for Author” status in several > commit tests. > > I think this patch would be really beneficial for us as we heavily use > views to structure

Re: Removing unneeded self joins

On Mon, 6 Mar 2023 at 00:30, Michał Kłeczek wrote: > > Hi All, > > I just wanted to ask about the status and plans for this patch. > I can see it being stuck at “Waiting for Author” status in several commit > tests. Sadly it seems to now be badly in need of a rebase. There are large hunks

Re: Removing unneeded self joins

Hi All, I just wanted to ask about the status and plans for this patch. I can see it being stuck at “Waiting for Author” status in several commit tests. I think this patch would be really beneficial for us as we heavily use views to structure out code. Each view is responsible for providing

Re: Removing unneeded self joins

On 12/6/22 23:46, Andres Freund wrote: This doesn't pass the main regression tests due to a plan difference. https://cirrus-ci.com/task/5537518245380096 https://api.cirrus-ci.com/v1/artifact/task/5537518245380096/testrun/build/testrun/regress/regress/regression.diffs diff -U3

Re: Removing unneeded self joins

Hi, On 2022-10-05 17:25:18 +0500, Andrey Lepikhov wrote: > New version, rebased onto current master. > Nothing special, just rebase. This doesn't pass the main regression tests due to a plan difference. https://cirrus-ci.com/task/5537518245380096

Re: Removing unneeded self joins

New version, rebased onto current master. Nothing special, just rebase. -- regards, Andrey Lepikhov Postgres Professional From 03aab7a2431032166c9ea5f52fbcccaf7168abec Mon Sep 17 00:00:00 2001 From: "Andrey V. Lepikhov" Date: Wed, 5 Oct 2022 16:58:34 +0500 Subject: [PATCH] Remove self-joins. A

Re: Removing unneeded self joins

On 8/29/22 04:39, Zhihong Yu wrote: On Fri, Aug 26, 2022 at 3:02 PM Zhihong Yu > wrote: Hi, For v36-0001-Remove-self-joins.patch : bq removes inner join of plane table to itself plane table -> plain table For relation_has_unique_index_ext(),

Re: Removing unneeded self joins

On Fri, Aug 26, 2022 at 3:02 PM Zhihong Yu wrote: > Hi, > For v36-0001-Remove-self-joins.patch : > > bq removes inner join of plane table to itself > > plane table -> plain table > > For relation_has_unique_index_ext(), it seems when extra_clauses is NULL, > there is no need to compute `exprs`.

Re: Removing unneeded self joins

Hi, For v36-0001-Remove-self-joins.patch : bq removes inner join of plane table to itself plane table -> plain table For relation_has_unique_index_ext(), it seems when extra_clauses is NULL, there is no need to compute `exprs`. Cheers >

Re: Removing unneeded self joins

On 30/6/2022 17:11, Andrey Lepikhov wrote: On 19/5/2022 16:47, Ronan Dunklau wrote: I'll take a look at that one. New version of the patch, rebased on current master: 1. pgindent over the patch have passed. 2. number of changed files is reduced. 3. Some documentation and comments is added.

Re: Removing unneeded self joins

On Mon, Jul 4, 2022 at 6:52 AM Ronan Dunklau wrote: > Le jeudi 30 juin 2022, 16:11:51 CEST Andrey Lepikhov a écrit : > > On 19/5/2022 16:47, Ronan Dunklau wrote: > > > I'll take a look at that one. > > > > New version of the patch, rebased on current master: > > 1. pgindent over the patch have

Re: Removing unneeded self joins

Le jeudi 30 juin 2022, 16:11:51 CEST Andrey Lepikhov a écrit : > On 19/5/2022 16:47, Ronan Dunklau wrote: > > I'll take a look at that one. > > New version of the patch, rebased on current master: > 1. pgindent over the patch have passed. > 2. number of changed files is reduced. > 3. Some

Re: Removing unneeded self joins

On 19/5/2022 16:47, Ronan Dunklau wrote: I'll take a look at that one. New version of the patch, rebased on current master: 1. pgindent over the patch have passed. 2. number of changed files is reduced. 3. Some documentation and comments is added. -- regards, Andrey Lepikhov Postgres

Re: Removing unneeded self joins

Le jeudi 19 mai 2022, 12:48:18 CEST Andrey Lepikhov a écrit : > On 5/17/22 19:14, Ronan Dunklau wrote: > > Le vendredi 13 mai 2022, 07:07:47 CEST Andrey Lepikhov a écrit : > >> New version of the feature. > >> Here a minor bug with RowMarks is fixed. A degenerated case is fixed, > >> when

Re: Removing unneeded self joins

On 5/17/22 19:14, Ronan Dunklau wrote: Le vendredi 13 mai 2022, 07:07:47 CEST Andrey Lepikhov a écrit : New version of the feature. Here a minor bug with RowMarks is fixed. A degenerated case is fixed, when uniqueness of an inner deduced not from join quals, but from a baserestrictinfo clauses

Re: Removing unneeded self joins

Le vendredi 13 mai 2022, 07:07:47 CEST Andrey Lepikhov a écrit : > New version of the feature. > Here a minor bug with RowMarks is fixed. A degenerated case is fixed, > when uniqueness of an inner deduced not from join quals, but from a > baserestrictinfo clauses 'x=const', where x - unique field.

Re: Removing unneeded self joins

Hi, w.r.t. v33-0001-Remove-self-joins.patch : removes inner join of plane table -> removes inner join of plain table in an query plan -> in a query plan + * Used as the relation_has_unique_index_for, Since relation_has_unique_index_for() becomes a wrapper of relation_has_unique_index_ext, the

Re: Removing unneeded self joins

On 4/1/22 20:27, Greg Stark wrote: Sigh. And now there's a patch conflict in a regression test expected output: sysviews.out Please rebase. Incidentally, make sure to check the expected output is actually correct. It's easy to "fix" an expected output to accidentally just memorialize an

Re: Removing unneeded self joins

Sigh. And now there's a patch conflict in a regression test expected output: sysviews.out Please rebase. Incidentally, make sure to check the expected output is actually correct. It's easy to "fix" an expected output to accidentally just memorialize an incorrect output. Btw, it's the last week

Re: Removing unneeded self joins

On 3/22/22 05:58, Andres Freund wrote: Hi, On 2022-03-04 15:47:47 +0500, Andrey Lepikhov wrote: Also, in new version of the patch I fixed one stupid bug: checking a self-join candidate expression operator - we can remove only expressions like F(arg1) = G(arg2). This CF entry currently fails

Re: Removing unneeded self joins

Hi, On 2022-03-04 15:47:47 +0500, Andrey Lepikhov wrote: > Also, in new version of the patch I fixed one stupid bug: checking a > self-join candidate expression operator - we can remove only expressions > like F(arg1) = G(arg2). This CF entry currently fails tests:

Re: Removing unneeded self joins

On 1/3/2022 03:03, Greg Stark wrote: On Thu, 1 Jul 2021 at 02:38, Ronan Dunklau wrote: Well in some cases they can't, when the query is not emitting redundant predicates by itself but they are added by something else like a view or a RLS policy. Maybe it would be worth it to allow spending a

Re: Removing unneeded self joins

I don't think the benchmarking that's needed is to check whether pruning unnecessary joins is helpful. Obviously it's going to be hard to measure on simple queries and small tables. But the resulting plan is unambiguously superior and in more complex cases could extra i/o. The benchmarking people

Re: Removing unneeded self joins

On Thu, Jul 15, 2021 at 05:49:11PM +0300, Andrey Lepikhov wrote: > On 6/7/21 13:49, Hywel Carver wrote: > > On Mon, Jul 5, 2021 at 2:20 PM Andrey Lepikhov > > mailto:a.lepik...@postgrespro.ru>> wrote: > > Looking through the email chain, a previous version of this patch added > > ~0.6% to planning

Re: Removing unneeded self joins

On Thu, 1 Jul 2021 at 02:38, Ronan Dunklau wrote: > > Well in some cases they can't, when the query is not emitting redundant > predicates by itself but they are added by something else like a view or a RLS > policy. > Maybe it would be worth it to allow spending a bit more time planning for >

Re: Removing unneeded self joins

On 7/16/21 12:28 AM, Zhihong Yu wrote: On Thu, Jul 15, 2021 at 8:25 AM Zhihong Yu > wrote: bq. We can proof the uniqueness proof -> prove Fixed 1. Collect all mergejoinable join quals looks like a.x = b.x  quals looks like -> quals which look

Re: Removing unneeded self joins

On Thu, Jul 15, 2021 at 8:25 AM Zhihong Yu wrote: > > > On Thu, Jul 15, 2021 at 7:49 AM Andrey Lepikhov > wrote: > >> On 6/7/21 13:49, Hywel Carver wrote: >> > On Mon, Jul 5, 2021 at 2:20 PM Andrey Lepikhov >> > mailto:a.lepik...@postgrespro.ru>> wrote: >> > Looking through the email chain, a

Re: Removing unneeded self joins

On Thu, Jul 15, 2021 at 7:49 AM Andrey Lepikhov wrote: > On 6/7/21 13:49, Hywel Carver wrote: > > On Mon, Jul 5, 2021 at 2:20 PM Andrey Lepikhov > > mailto:a.lepik...@postgrespro.ru>> wrote: > > Looking through the email chain, a previous version of this patch added > > ~0.6% to planning time in

Re: Removing unneeded self joins

On 6/7/21 13:49, Hywel Carver wrote: On Mon, Jul 5, 2021 at 2:20 PM Andrey Lepikhov mailto:a.lepik...@postgrespro.ru>> wrote: Looking through the email chain, a previous version of this patch added ~0.6% to planning time in the worst case tested - does that meet the "essentially free"

Re: Removing unneeded self joins

On Thu, May 27, 2021 at 12:21 PM Andrey V. Lepikhov wrote: > > On 5/8/21 2:00 AM, Hywel Carver wrote: > > On Fri, May 7, 2021 at 8:23 AM Andrey Lepikhov > > mailto:a.lepik...@postgrespro.ru>> wrote: > > Here I didn't work on 'unnecessary IS NOT NULL filter'. > > > > I've tested the new patch,

Re: Removing unneeded self joins

On Mon, Jul 5, 2021 at 2:20 PM Andrey Lepikhov wrote: > On 2/7/21 01:56, Hywel Carver wrote: > > On Wed, Jun 30, 2021 at 12:21 PM Andrey Lepikhov > > mailto:a.lepik...@postgrespro.ru>> wrote: > > I think, here we could ask more general question: do we want to > > remove a > > 'IS NOT

Re: Removing unneeded self joins

On 2/7/21 01:56, Hywel Carver wrote: On Wed, Jun 30, 2021 at 12:21 PM Andrey Lepikhov mailto:a.lepik...@postgrespro.ru>> wrote: I think, here we could ask more general question: do we want to remove a 'IS NOT NULL' clause from the clause list if the rest of the list implicitly

Re: Removing unneeded self joins

On Wed, Jun 30, 2021 at 12:21 PM Andrey Lepikhov wrote: > On 12/3/21 12:05, Hywel Carver wrote: > > I've built and tested this, and it seems to function correctly to me. > One question I have is whether the added "IS NOT NULL" filters can be > omitted when they're unnecessary. Some of the

  1   2   >