Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1qr8Kp-002ii3-Dp for pgsql-hackers@arkaria.postgresql.org; Fri, 13 Oct 2023 02:55:31 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1qr8Km-00DYZs-2H for pgsql-hackers@arkaria.postgresql.org; Fri, 13 Oct 2023 02:55:28 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1qr8Kl-00DYZk-G2 for pgsql-hackers@lists.postgresql.org; Fri, 13 Oct 2023 02:55:28 +0000 Received: from mail.postgrespro.ru ([93.174.131.139]) by makus.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1qr8Ke-000E1Y-EA for pgsql-hackers@lists.postgresql.org; Fri, 13 Oct 2023 02:55:26 +0000 Received: from [10.10.20.54] (node-9fc.pool-101-109.dynamic.totinternet.net [101.109.47.184]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) (Authenticated sender: a.lepikhov@postgrespro.ru) by mail.postgrespro.ru (Postfix/587) with ESMTPSA id 5D554E20FD2; Fri, 13 Oct 2023 05:55:16 +0300 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=postgrespro.ru; s=mx2023; t=1697165717; bh=/Ql/AVtyUvPvoBg9OclU6w61FHoM5gZ2c9OE1kTZXOE=; h=Message-ID:Date:User-Agent:Subject:To:Cc:References:From: In-Reply-To:From; b=tmSHLToRV3zfDCT9xE2lfB3xRbZtKFpaOlwslsN5IrriU2dFwagIxHbpQdfNQ0SZN 3ya4cPaivVdZF0+btATKPqRIxPjMjqzFA88EZ31RHeCtkTXDRJrs/9yaFcyvJlPwLl hC5b41kwKGeuzf5DsuH9xfkt4D4sgaX2KcYx7gXYTxN0DuMIGJTqicOnlxA3G396u2 g28V6x4Zi9oQZe5W+PjRDygl8FgnUBbRmuXXM5QDzjMpLurU0f1c0FPNinvsjkl2Ek TpIxG5n+xsGNOfyzcGJBNsutZ7ZOjEkkN37a76G7l2oWyCCEtBU6xPxtIhaxzXbplC xSHirhyAH00Sg== Content-Type: multipart/mixed; boundary="------------Lb5az1arfDHVJ0KzwA6aYdKi" Message-ID: <9eb86ca9-da3c-417f-8ce1-29c0ef5a4f50@postgrespro.ru> Date: Fri, 13 Oct 2023 09:55:13 +0700 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: Removing unneeded self joins Content-Language: en-US To: Alexander Korotkov Cc: "Gregory Stark (as CFM)" , =?UTF-8?Q?Micha=C5=82_K=C5=82eczek?= , PostgreSQL Hackers , "a.rybakina" References: <64486b0b-0404-e39e-322d-0801154901f3@postgrespro.ru> <0fd15c10-eaa5-51bd-b5c7-e635fe03a94d@postgrespro.ru> <20221206184643.zya44h5auv6f45zp@awork3.anarazel.de> <36579124-f9d9-a51e-38b3-af3511921c7e@postgrespro.ru> <88079CBD-B5BB-4C61-A68B-A8C67599E1C7@kleczek.org> <05469e80-454b-41d7-d04f-421f42bbd4a9@postgrespro.ru> <48c11b23-9c23-440e-9a3c-020e05196449@postgrespro.ru> From: Andrei Lepikhov Organization: Postgres Professional In-Reply-To: List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk This is a multi-part message in MIME format. --------------Lb5az1arfDHVJ0KzwA6aYdKi Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 clauses. But It >> doesn't process the RestrictInfo node ... I'm inclined to put a solution >> of this issue off for a while. > > OK. I think it doesn't worth to eliminate IS NULL quals with this > complexity (at least at this stage of work). Yeah. I think It would be meaningful in the case of replacing also nested x IS NOT NULL with nothing. But it requires using a mutator instead of the walker and may be done more accurately next time. > I made improvements over the code. Mostly new comments, grammar > corrections of existing comments and small refactoring. Great! > Also, I found that the suggestion from David Rowley [1] to qsort > array of relations to faster find duplicates is still unaddressed. > I've implemented it. That helps to evade quadratic complexity with > large number of relations. I see. The thread is too long so far, thanks for the catch. > 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 I like it too. But also, I suggest skipping some unnecessary assertions proposed in that patch: Assert(toKeep->relid != -1); - quite strange. Why -1? Why not all the negative numbers, at least? Assert(is_opclause(orinfo->clause)); - above we skip clauses with rinfo->mergeopfamilies == NIL. Each mergejoinable clause is already checked as is_opclause. All these changes (see in the attachment) are optional. -- regards, Andrey Lepikhov Postgres Professional --------------Lb5az1arfDHVJ0KzwA6aYdKi Content-Type: text/plain; charset=UTF-8; name="minor-changes-v44.diff" Content-Disposition: attachment; filename="minor-changes-v44.diff" Content-Transfer-Encoding: base64 ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL29wdGltaXplci9wbGFuL2FuYWx5emVqb2lucy5j IGIvc3JjL2JhY2tlbmQvb3B0aW1pemVyL3BsYW4vYW5hbHl6ZWpvaW5zLmMKaW5kZXggZjA3 NDZmMzVhMy4uN2I4ZGM3YTJiNyAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvb3B0aW1pemVy L3BsYW4vYW5hbHl6ZWpvaW5zLmMKKysrIGIvc3JjL2JhY2tlbmQvb3B0aW1pemVyL3BsYW4v YW5hbHl6ZWpvaW5zLmMKQEAgLTE3MTAsOCArMTcxMCw2IEBAIHJlbW92ZV9zZWxmX2pvaW5f cmVsKFBsYW5uZXJJbmZvICpyb290LCBQbGFuUm93TWFyayAqa21hcmssIFBsYW5Sb3dNYXJr ICpybWFyaywKIAlMaXN0CSAgICpiaW5mb19jYW5kaWRhdGVzID0gTklMOwogCVJlcGxhY2VW YXJub0NvbnRleHQgY3R4ID0gey5mcm9tID0gdG9SZW1vdmUtPnJlbGlkLC50byA9IHRvS2Vl cC0+cmVsaWR9OwogCi0JQXNzZXJ0KHRvS2VlcC0+cmVsaWQgIT0gLTEpOwotCiAJLyoKIAkg KiBSZXBsYWNlIGluZGV4IG9mIHJlbW92aW5nIHRhYmxlIHdpdGggdGhlIGtlZXBpbmcgb25l LiBUaGUgdGVjaG5pcXVlIG9mCiAJICogcmVtb3ZpbmcvZGlzdHJpYnV0aW5nIHJlc3RyaWN0 aW5mbyBpcyB1c2VkIGhlcmUgdG8gYXR0YWNoIGp1c3QgYXBwZWFyZWQKQEAgLTIwMTcsOCAr MjAxNSw2IEBAIG1hdGNoX3VuaXF1ZV9jbGF1c2VzKFBsYW5uZXJJbmZvICpyb290LCBSZWxP cHRJbmZvICpvdXRlciwgTGlzdCAqdWNsYXVzZXMsCiAJCQkJLyogRG9uJ3QgY29uc2lkZXIg Y2xhdXNlcyB3aGljaCBhcmVuJ3Qgc2ltaWxhciB0byAnRihYKT1HKFkpJyAqLwogCQkJCWNv bnRpbnVlOwogCi0JCQlBc3NlcnQoaXNfb3BjbGF1c2Uob3JpbmZvLT5jbGF1c2UpKTsKLQog CQkJb2NsYXVzZSA9IGJtc19pc19lbXB0eShvcmluZm8tPmxlZnRfcmVsaWRzKSA/CiAJCQkJ Z2V0X3JpZ2h0b3Aob3JpbmZvLT5jbGF1c2UpIDogZ2V0X2xlZnRvcChvcmluZm8tPmNsYXVz ZSk7CiAJCQljMiA9IChibXNfaXNfZW1wdHkob3JpbmZvLT5sZWZ0X3JlbGlkcykgPwpkaWZm IC0tZ2l0IGEvc3JjL3Rvb2xzL3BnaW5kZW50L3R5cGVkZWZzLmxpc3QgYi9zcmMvdG9vbHMv cGdpbmRlbnQvdHlwZWRlZnMubGlzdAppbmRleCAyZmY0ODgxZmRmLi45NmViZDZlZWQzIDEw MDY0NAotLS0gYS9zcmMvdG9vbHMvcGdpbmRlbnQvdHlwZWRlZnMubGlzdAorKysgYi9zcmMv dG9vbHMvcGdpbmRlbnQvdHlwZWRlZnMubGlzdApAQCAtMzY3LDcgKzM2Nyw2IEBAIENhdGFs b2dJZAogQ2F0YWxvZ0lkTWFwRW50cnkKIENhdGFsb2dJbmRleFN0YXRlCiBDaGFuZ2VWYXJO b2Rlc19jb250ZXh0Ci1SZXBsYWNlVmFybm9Db250ZXh0CiBDaGVja1BvaW50CiBDaGVja1Bv aW50U3RtdAogQ2hlY2twb2ludFN0YXRzRGF0YQpAQCAtMjM0MSw2ICsyMzQwLDcgQEAgUmVv cmRlckJ1ZmZlclVwZGF0ZVByb2dyZXNzVHhuQ0IKIFJlb3JkZXJUdXBsZQogUmVwT3JpZ2lu SWQKIFJlcGFyYW1ldGVyaXplRm9yZWlnblBhdGhCeUNoaWxkX2Z1bmN0aW9uCitSZXBsYWNl VmFybm9Db250ZXh0CiBSZXBsYWNlVmFyc0Zyb21UYXJnZXRMaXN0X2NvbnRleHQKIFJlcGxh Y2VWYXJzTm9NYXRjaE9wdGlvbgogUmVwbGljYUlkZW50aXR5U3RtdApAQCAtMjQ3NCw2ICsy NDc0LDcgQEAgU2VlblJlbHNFbnRyeQogU2VsZWN0TGltaXQKIFNlbGVjdFN0bXQKIFNlbGVj dGl2aXR5CitTZWxmSm9pbkNhbmRpZGF0ZQogU2VtVFBhZGRlZAogU2VtaUFudGlKb2luRmFj dG9ycwogU2VxU2Nhbgo= --------------Lb5az1arfDHVJ0KzwA6aYdKi--