MIME-Version: 1.0
References: <CAHewXNkPmtEXNfVQMou_7NqQmFABca9f4etjBtdbbm0ZKDmWvw@mail.gmail.com>
 <CAMbWs49LbQF_Z0iKPRPnTHfsRECT7M-4rF6ft5vpW1ARSpBkPA@mail.gmail.com>
 <CA+TgmobPBCvTHnBVn+a-=MS7pO_PUvtiLJkYo=BOrUjOxTss8g@mail.gmail.com>
 <CAMbWs49agQ1THL0XYv_4aeZXN2R1cRksevja05n-MFOzZtNsuA@mail.gmail.com>
 <CAHewXN=-XQwFY0n5EMUTzvK4G0aHWkNzdOPCfwnmQw3kDed2zQ@mail.gmail.com>
 <9FBD029B-3E71-4D47-AB54-938C73A4668F@yandex-team.ru> <CAHewXN=rP49ySfue9FA437jW4Me+FLBY-58zqTy+A_jxGGW=Xg@mail.gmail.com>
 <5641923793cef7706395a34e62538b75d05e498b.camel@post.pl> <CAHewXNmWbA9Oe6JCEc66dJrVyyBDv1uSx6z_zdz-BQRE0Jr9pQ@mail.gmail.com>
In-Reply-To: <CAHewXNmWbA9Oe6JCEc66dJrVyyBDv1uSx6z_zdz-BQRE0Jr9pQ@mail.gmail.com>
From: Richard Guo <guofenglinux@gmail.com>
Date: Tue, 18 Jun 2024 17:24:05 +0800
Message-ID: <CAMbWs48TaubitHF+JXqysbSHLCzTLOU9PD3zpLGFUtizsonAMA@mail.gmail.com>
Subject: Re: Should consider materializing the cheapest inner path in consider_parallel_nestloop()
To: Tender Wang <tndrwang@gmail.com>
Cc: Tomasz Rybak <tomasz.rybak@post.pl>, pgsql-hackers@lists.postgresql.org, 
	robertmhaas@gmail.com, David Rowley <dgrowleyml@gmail.com>
Content-Type: multipart/mixed; boundary="0000000000009bf1ed061b26a54d"
Archived-At: <https://www.postgresql.org/message-id/CAMbWs48TaubitHF%2BJXqysbSHLCzTLOU9PD3zpLGFUtizsonAMA%40mail.gmail.com>
Precedence: bulk

--0000000000009bf1ed061b26a54d
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Jun 4, 2024 at 6:51=E2=80=AFPM Tender Wang <tndrwang@gmail.com> wro=
te:
> Yeah, Richard commented the v1 patch about JOIN_UNIQUE_INNER in [1]
>
> * I think we should not consider materializing the cheapest inner path
> if we're doing JOIN_UNIQUE_INNER, because in this case we have to
> unique-ify the inner path.
>
> We don't consider material inner path if jointype is JOIN_UNIQUE_INNER in=
 match_unsorted_order().
> So here is as same logic as match_unsorted_order(). I added comments to e=
xplain why.

I looked through the v4 patch and found an issue.  For the plan diff:

+         ->  Nested Loop
+               ->  Parallel Seq Scan on prt1_p1 t1_1
+               ->  Materialize
+                     ->  Sample Scan on prt1_p1 t2_1
+                           Sampling: system (t1_1.a) REPEATABLE (t1_1.b)
+                           Filter: (t1_1.a =3D a)

This does not seem correct to me.  The inner path is parameterized by
the outer rel, in which case it does not make sense to add a Materialize
node on top of it.

I updated the patch to include a check in consider_parallel_nestloop
ensuring that inner_cheapest_total is not parameterized by outerrel
before materializing it.  I also tweaked the comments, test cases and
commit message.

Thanks
Richard

--0000000000009bf1ed061b26a54d
Content-Type: application/octet-stream; 
	name="v5-0001-Consider-materializing-the-cheapest-inner-path-in-parallel-nestloop.patch"
Content-Disposition: attachment; 
	filename="v5-0001-Consider-materializing-the-cheapest-inner-path-in-parallel-nestloop.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_lxk767q00>
X-Attachment-Id: f_lxk767q00

RnJvbSA0ZWE2MzYzZDIzMmJhMzkyOWZkN2VkZDE4ZTI3MTIxN2M4NTQ3NWJhIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBSaWNoYXJkIEd1byA8Z3VvZmVuZ2xpbnV4QGdtYWlsLmNvbT4K
RGF0ZTogVHVlLCAxOCBKdW4gMjAyNCAxNjoyNToxOSArMDkwMApTdWJqZWN0OiBbUEFUQ0ggdjVd
IENvbnNpZGVyIG1hdGVyaWFsaXppbmcgdGhlIGNoZWFwZXN0IGlubmVyIHBhdGggaW4gcGFyYWxs
ZWwKIG5lc3Rsb29wCgpXaGVuIGdlbmVyYXRpbmcgbm9uLXBhcmFsbGVsIG5lc3Rsb29wIHBhdGhz
IGZvciBlYWNoIGF2YWlsYWJsZSBvdXRlcgpwYXRoLCB3ZSBhbHdheXMgY29uc2lkZXIgbWF0ZXJp
YWxpemluZyB0aGUgY2hlYXBlc3QgaW5uZXIgcGF0aCBpZgpmZWFzaWJsZS4gIFNpbWlsYXJseSwg
aW4gdGhpcyBwYXRjaCwgd2UgYWxzbyBjb25zaWRlciBtYXRlcmlhbGl6aW5nIHRoZQpjaGVhcGVz
dCBpbm5lciBwYXRoIHdoZW4gYnVpbGRpbmcgcGFydGlhbCBuZXN0bG9vcCBwYXRocy4gIFRoaXMg
YXBwcm9hY2gKcG90ZW50aWFsbHkgcmVkdWNlcyB0aGUgbmVlZCB0byByZXNjYW4gdGhlIGlubmVy
IHNpZGUgb2YgYSBwYXJ0aWFsCm5lc3Rsb29wIHBhdGggZm9yIGVhY2ggb3V0ZXIgdHVwbGUuCi0t
LQogc3JjL2JhY2tlbmQvb3B0aW1pemVyL3BhdGgvam9pbnBhdGguYyAgICAgICAgIHwgMjYgKysr
KysrKysrKysrKysrKwogc3JjL3Rlc3QvcmVncmVzcy9leHBlY3RlZC9zZWxlY3RfcGFyYWxsZWwu
b3V0IHwgMzAgKysrKysrKysrKysrKysrKysrKwogc3JjL3Rlc3QvcmVncmVzcy9zcWwvc2VsZWN0
X3BhcmFsbGVsLnNxbCAgICAgIHwgMTAgKysrKysrKwogMyBmaWxlcyBjaGFuZ2VkLCA2NiBpbnNl
cnRpb25zKCspCgpkaWZmIC0tZ2l0IGEvc3JjL2JhY2tlbmQvb3B0aW1pemVyL3BhdGgvam9pbnBh
dGguYyBiL3NyYy9iYWNrZW5kL29wdGltaXplci9wYXRoL2pvaW5wYXRoLmMKaW5kZXggNWJlOGRh
OWUwOS4uMTFlMTI1M2JjZCAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvb3B0aW1pemVyL3BhdGgv
am9pbnBhdGguYworKysgYi9zcmMvYmFja2VuZC9vcHRpbWl6ZXIvcGF0aC9qb2lucGF0aC5jCkBA
IC0yMDE0LDExICsyMDE0LDMyIEBAIGNvbnNpZGVyX3BhcmFsbGVsX25lc3Rsb29wKFBsYW5uZXJJ
bmZvICpyb290LAogCQkJCQkJICAgSm9pblBhdGhFeHRyYURhdGEgKmV4dHJhKQogewogCUpvaW5U
eXBlCXNhdmVfam9pbnR5cGUgPSBqb2ludHlwZTsKKwlQYXRoCSAgICppbm5lcl9jaGVhcGVzdF90
b3RhbCA9IGlubmVycmVsLT5jaGVhcGVzdF90b3RhbF9wYXRoOworCVBhdGgJICAgKm1hdHBhdGgg
PSBOVUxMOwogCUxpc3RDZWxsICAgKmxjMTsKIAogCWlmIChqb2ludHlwZSA9PSBKT0lOX1VOSVFV
RV9JTk5FUikKIAkJam9pbnR5cGUgPSBKT0lOX0lOTkVSOwogCisJLyoKKwkgKiBDb25zaWRlciBt
YXRlcmlhbGl6aW5nIHRoZSBjaGVhcGVzdCBpbm5lciBwYXRoLCB1bmxlc3M6CisJICogMSkgd2Un
cmUgZG9pbmcgSk9JTl9VTklRVUVfSU5ORVIsIGJlY2F1c2UgaW4gdGhpcyBjYXNlIHdlIGhhdmUg
dG8KKwkgKiAgICB1bmlxdWUtaWZ5IHRoZSBjaGVhcGVzdCBpbm5lciBwYXRoLAorCSAqIDIpIGVu
YWJsZV9tYXRlcmlhbCBpcyBvZmYsCisJICogMykgdGhlIGNoZWFwZXN0IGlubmVyIHBhdGggaXMg
bm90IHBhcmFsbGVsLXNhZmUsCisJICogNCkgdGhlIGNoZWFwZXN0IGlubmVyIHBhdGggaXMgcGFy
YW1ldGVyaXplZCBieSB0aGUgb3V0ZXIgcmVsLCBvcgorCSAqIDUpIHRoZSBjaGVhcGVzdCBpbm5l
ciBwYXRoIG1hdGVyaWFsaXplcyBpdHMgb3V0cHV0IGFueXdheS4KKwkgKi8KKwlpZiAoc2F2ZV9q
b2ludHlwZSAhPSBKT0lOX1VOSVFVRV9JTk5FUiAmJgorCQllbmFibGVfbWF0ZXJpYWwgJiYgaW5u
ZXJfY2hlYXBlc3RfdG90YWwtPnBhcmFsbGVsX3NhZmUgJiYKKwkJIVBBVEhfUEFSQU1fQllfUkVM
KGlubmVyX2NoZWFwZXN0X3RvdGFsLCBvdXRlcnJlbCkgJiYKKwkJIUV4ZWNNYXRlcmlhbGl6ZXNP
dXRwdXQoaW5uZXJfY2hlYXBlc3RfdG90YWwtPnBhdGh0eXBlKSkKKwl7CisJCW1hdHBhdGggPSAo
UGF0aCAqKQorCQkJY3JlYXRlX21hdGVyaWFsX3BhdGgoaW5uZXJyZWwsIGlubmVyX2NoZWFwZXN0
X3RvdGFsKTsKKwkJQXNzZXJ0KG1hdHBhdGgtPnBhcmFsbGVsX3NhZmUpOworCX0KKwogCWZvcmVh
Y2gobGMxLCBvdXRlcnJlbC0+cGFydGlhbF9wYXRobGlzdCkKIAl7CiAJCVBhdGgJICAgKm91dGVy
cGF0aCA9IChQYXRoICopIGxmaXJzdChsYzEpOwpAQCAtMjA3NSw2ICsyMDk2LDExIEBAIGNvbnNp
ZGVyX3BhcmFsbGVsX25lc3Rsb29wKFBsYW5uZXJJbmZvICpyb290LAogCQkJCXRyeV9wYXJ0aWFs
X25lc3Rsb29wX3BhdGgocm9vdCwgam9pbnJlbCwgb3V0ZXJwYXRoLCBtcGF0aCwKIAkJCQkJCQkJ
CQkgIHBhdGhrZXlzLCBqb2ludHlwZSwgZXh0cmEpOwogCQl9CisKKwkJLyogQWxzbyBjb25zaWRl
ciBtYXRlcmlhbGl6ZWQgZm9ybSBvZiB0aGUgY2hlYXBlc3QgaW5uZXIgcGF0aCAqLworCQlpZiAo
bWF0cGF0aCAhPSBOVUxMKQorCQkJdHJ5X3BhcnRpYWxfbmVzdGxvb3BfcGF0aChyb290LCBqb2lu
cmVsLCBvdXRlcnBhdGgsIG1hdHBhdGgsCisJCQkJCQkJCQkgIHBhdGhrZXlzLCBqb2ludHlwZSwg
ZXh0cmEpOwogCX0KIH0KIApkaWZmIC0tZ2l0IGEvc3JjL3Rlc3QvcmVncmVzcy9leHBlY3RlZC9z
ZWxlY3RfcGFyYWxsZWwub3V0IGIvc3JjL3Rlc3QvcmVncmVzcy9leHBlY3RlZC9zZWxlY3RfcGFy
YWxsZWwub3V0CmluZGV4IDg3MjczZmE2MzUuLmQxM2Y4MTJmMzYgMTAwNjQ0Ci0tLSBhL3NyYy90
ZXN0L3JlZ3Jlc3MvZXhwZWN0ZWQvc2VsZWN0X3BhcmFsbGVsLm91dAorKysgYi9zcmMvdGVzdC9y
ZWdyZXNzL2V4cGVjdGVkL3NlbGVjdF9wYXJhbGxlbC5vdXQKQEAgLTY1Myw2ICs2NTMsMzYgQEAg
c2VsZWN0ICBjb3VudCgqKSBmcm9tIHRlbmsxLCB0ZW5rMiB3aGVyZSB0ZW5rMS51bmlxdWUxID0g
dGVuazIudW5pcXVlMTsKIAogcmVzZXQgZW5hYmxlX2hhc2hqb2luOwogcmVzZXQgZW5hYmxlX25l
c3Rsb29wOworLS0gdGVzdCBwYXJhbGxlbCBuZXN0bG9vcCBqb2luIHBhdGggd2l0aCBtYXRlcmlh
bGl6YXRpb24gb2YgdGhlIGlubmVyIHBhdGguCithbHRlciB0YWJsZSB0ZW5rMiBzZXQgKHBhcmFs
bGVsX3dvcmtlcnMgPSAwKTsKK2V4cGxhaW4gKGNvc3RzIG9mZikKKwlzZWxlY3QgKiBmcm9tIHRl
bmsxIHQxLCB0ZW5rMiB0MiB3aGVyZSB0MS50d28gPiB0Mi50d287CisgICAgICAgICAgICAgICAg
UVVFUlkgUExBTiAgICAgICAgICAgICAgICAgCistLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tCisgR2F0aGVyCisgICBXb3JrZXJzIFBsYW5uZWQ6IDQKKyAgIC0+ICBO
ZXN0ZWQgTG9vcAorICAgICAgICAgSm9pbiBGaWx0ZXI6ICh0MS50d28gPiB0Mi50d28pCisgICAg
ICAgICAtPiAgUGFyYWxsZWwgU2VxIFNjYW4gb24gdGVuazEgdDEKKyAgICAgICAgIC0+ICBNYXRl
cmlhbGl6ZQorICAgICAgICAgICAgICAgLT4gIFNlcSBTY2FuIG9uIHRlbmsyIHQyCisoNyByb3dz
KQorCistLSB0aGlzIGlzIG5vdCBwYXJhbGxlbC1zYWZlIGR1ZSB0byB0aGUgT0ZGU0VUIGNsYXVz
ZSBpbiB0aGUgc3VicXVlcnkKK2V4cGxhaW4gKGNvc3RzIG9mZikKKwlzZWxlY3QgKiBmcm9tIHRl
bmsxIHQxLCAoc2VsZWN0ICogZnJvbSB0ZW5rMiB0MiBvZmZzZXQgMCkgdDIgd2hlcmUgdDEudHdv
ID4gdDIudHdvOworICAgICAgICAgICAgICAgIFFVRVJZIFBMQU4gICAgICAgICAgICAgICAgIAor
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQorIE5lc3RlZCBMb29w
CisgICBKb2luIEZpbHRlcjogKHQxLnR3byA+IHQyLnR3bykKKyAgIC0+ICBHYXRoZXIKKyAgICAg
ICAgIFdvcmtlcnMgUGxhbm5lZDogNAorICAgICAgICAgLT4gIFBhcmFsbGVsIFNlcSBTY2FuIG9u
IHRlbmsxIHQxCisgICAtPiAgTWF0ZXJpYWxpemUKKyAgICAgICAgIC0+ICBTZXEgU2NhbiBvbiB0
ZW5rMiB0MgorKDcgcm93cykKKworYWx0ZXIgdGFibGUgdGVuazIgcmVzZXQgKHBhcmFsbGVsX3dv
cmtlcnMpOwogLS0gdGVzdCBnYXRoZXIgbWVyZ2UKIHNldCBlbmFibGVfaGFzaGFnZyA9IGZhbHNl
OwogZXhwbGFpbiAoY29zdHMgb2ZmKQpkaWZmIC0tZ2l0IGEvc3JjL3Rlc3QvcmVncmVzcy9zcWwv
c2VsZWN0X3BhcmFsbGVsLnNxbCBiL3NyYy90ZXN0L3JlZ3Jlc3Mvc3FsL3NlbGVjdF9wYXJhbGxl
bC5zcWwKaW5kZXggMjAzNzZjMDNmYS4uNjdkYWM3ZjYyYiAxMDA2NDQKLS0tIGEvc3JjL3Rlc3Qv
cmVncmVzcy9zcWwvc2VsZWN0X3BhcmFsbGVsLnNxbAorKysgYi9zcmMvdGVzdC9yZWdyZXNzL3Nx
bC9zZWxlY3RfcGFyYWxsZWwuc3FsCkBAIC0yNjYsNiArMjY2LDE2IEBAIHNlbGVjdCAgY291bnQo
KikgZnJvbSB0ZW5rMSwgdGVuazIgd2hlcmUgdGVuazEudW5pcXVlMSA9IHRlbmsyLnVuaXF1ZTE7
CiByZXNldCBlbmFibGVfaGFzaGpvaW47CiByZXNldCBlbmFibGVfbmVzdGxvb3A7CiAKKy0tIHRl
c3QgcGFyYWxsZWwgbmVzdGxvb3Agam9pbiBwYXRoIHdpdGggbWF0ZXJpYWxpemF0aW9uIG9mIHRo
ZSBpbm5lciBwYXRoLgorYWx0ZXIgdGFibGUgdGVuazIgc2V0IChwYXJhbGxlbF93b3JrZXJzID0g
MCk7CitleHBsYWluIChjb3N0cyBvZmYpCisJc2VsZWN0ICogZnJvbSB0ZW5rMSB0MSwgdGVuazIg
dDIgd2hlcmUgdDEudHdvID4gdDIudHdvOworCistLSB0aGlzIGlzIG5vdCBwYXJhbGxlbC1zYWZl
IGR1ZSB0byB0aGUgT0ZGU0VUIGNsYXVzZSBpbiB0aGUgc3VicXVlcnkKK2V4cGxhaW4gKGNvc3Rz
IG9mZikKKwlzZWxlY3QgKiBmcm9tIHRlbmsxIHQxLCAoc2VsZWN0ICogZnJvbSB0ZW5rMiB0MiBv
ZmZzZXQgMCkgdDIgd2hlcmUgdDEudHdvID4gdDIudHdvOworYWx0ZXIgdGFibGUgdGVuazIgcmVz
ZXQgKHBhcmFsbGVsX3dvcmtlcnMpOworCiAtLSB0ZXN0IGdhdGhlciBtZXJnZQogc2V0IGVuYWJs
ZV9oYXNoYWdnID0gZmFsc2U7CiAKLS0gCjIuNDMuMAoK
--0000000000009bf1ed061b26a54d--