Content-Type: multipart/mixed; boundary="------------QOpnW2kjNJVbc2Rdw0SWTif4"
Message-ID: <3c83c594-6beb-414b-8059-f57e9a9eb0d7@gmail.com>
Date: Wed, 7 May 2025 20:42:16 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Incorrect calculation of path fraction value in MergeAppend
To: Alvaro Herrera <alvherre@alvh.no-ip.org>, Junwang Zhao <zhjwpku@gmail.com>
Cc: "pgsql-bugs@lists.postgresql.org" <pgsql-bugs@lists.postgresql.org>,
 Tomas Vondra <tomas@vondra.me>, Tom Lane <tgl@sss.pgh.pa.us>
References: <3ca271fa-ca5c-458c-8934-eb148622b270@gmail.com>
 <CAEG8a3K3CgS_cK6jQHjmxsJpMwyTx3r3eLEC+8b2FACFWv_fjg@mail.gmail.com>
Content-Language: en-US
From: Andrei Lepikhov <lepihov@gmail.com>
In-Reply-To: 
 <CAEG8a3K3CgS_cK6jQHjmxsJpMwyTx3r3eLEC+8b2FACFWv_fjg@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/3c83c594-6beb-414b-8059-f57e9a9eb0d7%40gmail.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------QOpnW2kjNJVbc2Rdw0SWTif4
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 7/5/2025 12:45, Álvaro Herrera wrote:
 > On 2025-May-07, Andrei Lepikhov wrote:
 >> The magic is how it finds a link to the thread. This time it doesn't
 > Weird, that doesn't seem to work very well if you try to search for
 > stuff.  But if you just specify the Message-Id without clicking
 > "search" then it works.  So here you go:
 > https://commitfest.postgresql.org/patch/5742/
Thanks! I think this memo should be pinned at the top of the page.

On 7/5/2025 17:35, Junwang Zhao wrote:
> + /* Convert absolute limit to a path fraction */
> + if (path_fraction >= 1.)
> + path_fraction /= childrel->rows;
> 
> maybe add `&& childrel->rows > 0.` to the if condition to
> avoid potential crash. I'm not sure if `childrel->rows` will
> be 0 but I see get_cheapest_fractional_path did this.
You may look at the 76281aa for the reason. Also, looking into the code, 
I see that it is impossible now to find more places with rows == 0 
except for the Dummy result node. Moreover, it just doesn't make sense 
in the case of append nodes: why should the optimiser spend resources 
and consider paths if it is impossible to pull any tuples from the subplan?

So, I think checking assertion instead would be the better option. See 
new version in the attachment.

-- 
regards, Andrei Lepikhov
--------------QOpnW2kjNJVbc2Rdw0SWTif4
Content-Type: text/plain; charset=UTF-8;
 name="v1-0001-Fix-tuple_fraction-calculation-for-merge-append-f.patch"
Content-Disposition: attachment;
 filename*0="v1-0001-Fix-tuple_fraction-calculation-for-merge-append-f.pa";
 filename*1="tch"
Content-Transfer-Encoding: base64

RnJvbSAyYmM1ZjUzNDgyODNjNmYyNTM1ZDBkYTlkMmJiZmQ0OTQyZjg3NzM2IE1vbiBTZXAg
MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiAiQW5kcmVpIFYuIExlcGlraG92IiA8bGVwaWhvdkBn
bWFpbC5jb20+CkRhdGU6IFR1ZSwgMjkgQXByIDIwMjUgMTI6NTQ6NTkgKzAyMDAKU3ViamVj
dDogW1BBVENIIHYxXSBGaXggdHVwbGVfZnJhY3Rpb24gY2FsY3VsYXRpb24gZm9yIG1lcmdl
IGFwcGVuZCBmcmFjdGlvbmFsCiBwYXRoCgotLS0KIHNyYy9iYWNrZW5kL29wdGltaXplci9w
YXRoL2FsbHBhdGhzLmMgICAgICAgIHwgIDkgKysrKysrKystCiBzcmMvdGVzdC9yZWdyZXNz
L2V4cGVjdGVkL3BhcnRpdGlvbl9qb2luLm91dCB8IDE4ICsrKysrKysrKysrKysrKysrKwog
c3JjL3Rlc3QvcmVncmVzcy9zcWwvcGFydGl0aW9uX2pvaW4uc3FsICAgICAgfCAgMyArKysK
IDMgZmlsZXMgY2hhbmdlZCwgMjkgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQoKZGlm
ZiAtLWdpdCBhL3NyYy9iYWNrZW5kL29wdGltaXplci9wYXRoL2FsbHBhdGhzLmMgYi9zcmMv
YmFja2VuZC9vcHRpbWl6ZXIvcGF0aC9hbGxwYXRocy5jCmluZGV4IDkwNTI1MGIzMzI1Li42
MzczMDY5NTM3NSAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvb3B0aW1pemVyL3BhdGgvYWxs
cGF0aHMuYworKysgYi9zcmMvYmFja2VuZC9vcHRpbWl6ZXIvcGF0aC9hbGxwYXRocy5jCkBA
IC0xODkxLDcgKzE4OTEsMTQgQEAgZ2VuZXJhdGVfb3JkZXJlZGFwcGVuZF9wYXRocyhQbGFu
bmVySW5mbyAqcm9vdCwgUmVsT3B0SW5mbyAqcmVsLAogCQkJICovCiAJCQlpZiAocm9vdC0+
dHVwbGVfZnJhY3Rpb24gPiAwKQogCQkJewotCQkJCWRvdWJsZQkJcGF0aF9mcmFjdGlvbiA9
ICgxLjAgLyByb290LT50dXBsZV9mcmFjdGlvbik7CisJCQkJZG91YmxlCQlwYXRoX2ZyYWN0
aW9uID0gcm9vdC0+dHVwbGVfZnJhY3Rpb247CisKKwkJCQkvKiBNZXJnZSBBcHBlbmQgY29u
c2lkZXJzIG9ubHkgbGl2ZSBjaGlsZHJlbHMgKi8KKwkJCQlBc3NlcnQoY2hpbGRyZWwtPnJv
d3MgPiAwKTsKKworCQkJCS8qIENvbnZlcnQgYWJzb2x1dGUgbGltaXQgdG8gYSBwYXRoIGZy
YWN0aW9uICovCisJCQkJaWYgKHBhdGhfZnJhY3Rpb24gPj0gMS4wKQorCQkJCQlwYXRoX2Zy
YWN0aW9uIC89IGNoaWxkcmVsLT5yb3dzOwogCiAJCQkJY2hlYXBlc3RfZnJhY3Rpb25hbCA9
CiAJCQkJCWdldF9jaGVhcGVzdF9mcmFjdGlvbmFsX3BhdGhfZm9yX3BhdGhrZXlzKGNoaWxk
cmVsLT5wYXRobGlzdCwKZGlmZiAtLWdpdCBhL3NyYy90ZXN0L3JlZ3Jlc3MvZXhwZWN0ZWQv
cGFydGl0aW9uX2pvaW4ub3V0IGIvc3JjL3Rlc3QvcmVncmVzcy9leHBlY3RlZC9wYXJ0aXRp
b25fam9pbi5vdXQKaW5kZXggNjEwMWM4YzdjZjEuLmQ1MzY4MTg2Y2FhIDEwMDY0NAotLS0g
YS9zcmMvdGVzdC9yZWdyZXNzL2V4cGVjdGVkL3BhcnRpdGlvbl9qb2luLm91dAorKysgYi9z
cmMvdGVzdC9yZWdyZXNzL2V4cGVjdGVkL3BhcnRpdGlvbl9qb2luLm91dApAQCAtNTI2MCw2
ICs1MjYwLDI0IEBAIFNFTEVDVCB4LmlkLCB5LmlkIEZST00gZnJhY3RfdCB4IExFRlQgSk9J
TiBmcmFjdF90IHkgVVNJTkcgKGlkKSBPUkRFUiBCWSB4LmlkIERFCiAgICAgICAgICAgICAg
ICAgICAgICBJbmRleCBDb25kOiAoaWQgPSB4XzIuaWQpCiAoMTEgcm93cykKIAorRVhQTEFJ
TiAoQ09TVFMgT0ZGKSAtLSBTaG91bGQgdXNlIE5lc3RMb29wIHdpdGggcGFyYW1ldGVyaXNl
ZCBpbm5lciBzY2FuCitTRUxFQ1QgeC5pZCwgeS5pZCBGUk9NIGZyYWN0X3QgeCBMRUZUIEpP
SU4gZnJhY3RfdCB5IFVTSU5HIChpZCkKK09SREVSIEJZIHguaWQgREVTQyBMSU1JVCAyOwor
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBRVUVSWSBQTEFOICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAKKy0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
CisgTGltaXQKKyAgIC0+ICBNZXJnZSBBcHBlbmQKKyAgICAgICAgIFNvcnQgS2V5OiB4Lmlk
IERFU0MKKyAgICAgICAgIC0+ICBOZXN0ZWQgTG9vcCBMZWZ0IEpvaW4KKyAgICAgICAgICAg
ICAgIC0+ICBJbmRleCBPbmx5IFNjYW4gQmFja3dhcmQgdXNpbmcgZnJhY3RfdDBfcGtleSBv
biBmcmFjdF90MCB4XzEKKyAgICAgICAgICAgICAgIC0+ICBJbmRleCBPbmx5IFNjYW4gdXNp
bmcgZnJhY3RfdDBfcGtleSBvbiBmcmFjdF90MCB5XzEKKyAgICAgICAgICAgICAgICAgICAg
IEluZGV4IENvbmQ6IChpZCA9IHhfMS5pZCkKKyAgICAgICAgIC0+ICBOZXN0ZWQgTG9vcCBM
ZWZ0IEpvaW4KKyAgICAgICAgICAgICAgIC0+ICBJbmRleCBPbmx5IFNjYW4gQmFja3dhcmQg
dXNpbmcgZnJhY3RfdDFfcGtleSBvbiBmcmFjdF90MSB4XzIKKyAgICAgICAgICAgICAgIC0+
ICBJbmRleCBPbmx5IFNjYW4gdXNpbmcgZnJhY3RfdDFfcGtleSBvbiBmcmFjdF90MSB5XzIK
KyAgICAgICAgICAgICAgICAgICAgIEluZGV4IENvbmQ6IChpZCA9IHhfMi5pZCkKKygxMSBy
b3dzKQorCiAtLQogLS0gVGVzdCBBcHBlbmQncyBmcmFjdGlvbmFsIHBhdGhzCiAtLQpkaWZm
IC0tZ2l0IGEvc3JjL3Rlc3QvcmVncmVzcy9zcWwvcGFydGl0aW9uX2pvaW4uc3FsIGIvc3Jj
L3Rlc3QvcmVncmVzcy9zcWwvcGFydGl0aW9uX2pvaW4uc3FsCmluZGV4IGI3NmM1NDUxMDAx
Li4zMGYxNWVlOWFjYiAxMDA2NDQKLS0tIGEvc3JjL3Rlc3QvcmVncmVzcy9zcWwvcGFydGl0
aW9uX2pvaW4uc3FsCisrKyBiL3NyYy90ZXN0L3JlZ3Jlc3Mvc3FsL3BhcnRpdGlvbl9qb2lu
LnNxbApAQCAtMTIyNCw2ICsxMjI0LDkgQEAgU0VMRUNUIHguaWQsIHkuaWQgRlJPTSBmcmFj
dF90IHggTEVGVCBKT0lOIGZyYWN0X3QgeSBVU0lORyAoaWQpIE9SREVSIEJZIHguaWQgQVMK
IAogRVhQTEFJTiAoQ09TVFMgT0ZGKQogU0VMRUNUIHguaWQsIHkuaWQgRlJPTSBmcmFjdF90
IHggTEVGVCBKT0lOIGZyYWN0X3QgeSBVU0lORyAoaWQpIE9SREVSIEJZIHguaWQgREVTQyBM
SU1JVCAxMDsKK0VYUExBSU4gKENPU1RTIE9GRikgLS0gU2hvdWxkIHVzZSBOZXN0TG9vcCB3
aXRoIHBhcmFtZXRlcmlzZWQgaW5uZXIgc2NhbgorU0VMRUNUIHguaWQsIHkuaWQgRlJPTSBm
cmFjdF90IHggTEVGVCBKT0lOIGZyYWN0X3QgeSBVU0lORyAoaWQpCitPUkRFUiBCWSB4Lmlk
IERFU0MgTElNSVQgMjsKIAogLS0KIC0tIFRlc3QgQXBwZW5kJ3MgZnJhY3Rpb25hbCBwYXRo
cwotLSAKMi40OS4wCgo=

--------------QOpnW2kjNJVbc2Rdw0SWTif4--