MIME-Version: 1.0
References: <16807-9fe4e08fbaa5c7ce@postgresql.org>
 <CAPmGK17nuChuVAe0wr_51OxxLewbZdefwYNNF=wNVGjLcTwADQ@mail.gmail.com>
 <CAPmGK16Te7mLsxJ6yaPmJy_qFdmXEmqegxXm4sNQRprYsxtmfg@mail.gmail.com>
 <20210114.133024.160907178608761711.horikyota.ntt@gmail.com>
In-Reply-To: <20210114.133024.160907178608761711.horikyota.ntt@gmail.com>
From: Etsuro Fujita <etsuro.fujita@gmail.com>
Date: Thu, 4 Feb 2021 18:56:22 +0900
Message-ID: 
 <CAPmGK141OhAhUQu9MyvFWDGuWJUMTbDZGO0N-hLEFJsF7ZQ17A@mail.gmail.com>
Subject: Re: BUG #16807: Assert failed in postgres_fdw/estimate_path_cost_size
 with an empty foreign table
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: exclusion@gmail.com,
	PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Content-Type: multipart/mixed; boundary="000000000000dd67b205ba7fb78a"
Precedence: bulk

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

On Thu, Jan 14, 2021 at 1:30 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
> At Thu, 14 Jan 2021 11:56:08 +0900, Etsuro Fujita <etsuro.fujita@gmail.co=
m> wrote in
> > If foreignrel->tuples is set to zero, the retrieved_rows estimate will
> > also be set to zero, so the assertion pointed out upthread is not
> > correct.  I think I mistakenly assumed that foreignrel->tuples would
> > have been forced to be at least one row, like foreignrel->rows, before
> > we get to estimate_path_cost_size(), which is not correct.
> >
> > To fix, I think it would be better to adjust the assertion accordingly
> > than removing it, since it was added to make sure that the
> > retrieved_rows estimate is set to a sensible value.  Attached is a
> > patch for that.  I added a simpler test case as well.
>
> FWIW I drew the same conclusion.

Ok, thanks for reviewing!

I tweaked a regression test comment a little bit and added the commit
message.  Attached is an updated patch.  I=E2=80=99ll commit the patch.

Best regards,
Etsuro Fujita

--000000000000dd67b205ba7fb78a
Content-Type: application/octet-stream;
	name="0001-postgres_fdw-Fix-assertion-in-estimate_path_cost_siz.patch"
Content-Disposition: attachment;
	filename="0001-postgres_fdw-Fix-assertion-in-estimate_path_cost_siz.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_kkqops7t0>
X-Attachment-Id: f_kkqops7t0

RnJvbSBjMTBkM2FjNzg4Njc4NTlkNjIzYTM2Y2NlZWQyMmYyZWMyZmY5YjhjIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBFdHN1cm8gRnVqaXRhIDxldHN1cm8uZnVqaXRhQGdtYWlsLmNv
bT4KRGF0ZTogVGh1LCA0IEZlYiAyMDIxIDE4OjQ4OjM3ICswOTAwClN1YmplY3Q6IFtQQVRDSF0g
cG9zdGdyZXNfZmR3OiBGaXggYXNzZXJ0aW9uIGluIGVzdGltYXRlX3BhdGhfY29zdF9zaXplKCku
CgpJbiBjb21taXQgMDhkMmQ1OGEyLCBJIGFzc3VtZWQgdGhhdCB0aGUgcmV0cmlldmVkX3Jvd3Mg
ZXN0aW1hdGUgZm9yIGEKZm9yZWlnbiByZWxhdGlvbiwgd2hpY2ggaXMgcmUtdXNlZCB0byBjb3N0
IHByZS1zb3J0ZWQgZm9yZWlnbiBwYXRocyB3aXRoCmxvY2FsIHN0YXRzLCBpcyBzZXQgdG8gYXQg
bGVhc3Qgb25lIHJvdyBpbiBlc3RpbWF0ZV9wYXRoX2Nvc3Rfc2l6ZSgpLAp3aGljaCBpcyBub3Qg
Y29ycmVjdDogd2hlbiB0aGUgcmVsYXRpb24gaXMgYSBmb3JlaWduIHRhYmxlIHdpdGggdHVwbGVz
PTAsCnRoZSBlc3RpbWF0ZSB3b3VsZCBiZSBzZXQgdG8gMCB3aGVuIG5vdCB1c2luZyByZW1vdGUg
ZXN0aW1hdGVzLgoKQmFjay1wYXRjaCB0byB2MTMgd2hlcmUgdGhlIGFmb3JlbWVudGlvbmVkIGNv
bW1pdCB3ZW50IGluLgoKUGVyIGJ1ZyAjMTY4MDcgZnJvbSBBbGV4YW5kZXIgTGFraGluLgoKQXV0
aG9yOiBFdHN1cm8gRnVqaXRhClJldmlld2VkLWJ5OiBLeW90YXJvIEhvcmlndWNoaQpEaXNjdXNz
aW9uOiBodHRwczovL3Bvc3Rnci5lcy9tLzE2ODA3LTlmZTRlMDhmYmFhNWM3Y2UlNDBwb3N0Z3Jl
c3FsLm9yZwotLS0KIGNvbnRyaWIvcG9zdGdyZXNfZmR3L2V4cGVjdGVkL3Bvc3RncmVzX2Zkdy5v
dXQgfCAxOCArKysrKysrKysrKysrKysrKysKIGNvbnRyaWIvcG9zdGdyZXNfZmR3L3Bvc3RncmVz
X2Zkdy5jICAgICAgICAgICAgfCAgMiArLQogY29udHJpYi9wb3N0Z3Jlc19mZHcvc3FsL3Bvc3Rn
cmVzX2Zkdy5zcWwgICAgICB8IDEyICsrKysrKysrKysrKwogMyBmaWxlcyBjaGFuZ2VkLCAzMSBp
bnNlcnRpb25zKCspLCAxIGRlbGV0aW9uKC0pCgpkaWZmIC0tZ2l0IGEvY29udHJpYi9wb3N0Z3Jl
c19mZHcvZXhwZWN0ZWQvcG9zdGdyZXNfZmR3Lm91dCBiL2NvbnRyaWIvcG9zdGdyZXNfZmR3L2V4
cGVjdGVkL3Bvc3RncmVzX2Zkdy5vdXQKaW5kZXggYjA5ZGNlNjNmNS4uNjBjN2UxMTVkNiAxMDA2
NDQKLS0tIGEvY29udHJpYi9wb3N0Z3Jlc19mZHcvZXhwZWN0ZWQvcG9zdGdyZXNfZmR3Lm91dAor
KysgYi9jb250cmliL3Bvc3RncmVzX2Zkdy9leHBlY3RlZC9wb3N0Z3Jlc19mZHcub3V0CkBAIC02
MTMsNiArNjEzLDI0IEBAIFNFTEVDVCB0MS4iQyAxIiwgdDIuYzEsIHQzLmMxIEZST00gIlMgMSIu
IlQgMSIgdDEgZnVsbCBqb2luIGZ0MSB0MiBmdWxsIGpvaW4gZnQyCiAKIFJFU0VUIGVuYWJsZV9o
YXNoam9pbjsKIFJFU0VUIGVuYWJsZV9uZXN0bG9vcDsKKy0tIFRlc3QgZXhlY3V0aW5nIGFzc2Vy
dGlvbiBpbiBlc3RpbWF0ZV9wYXRoX2Nvc3Rfc2l6ZSgpIHRoYXQgbWFrZXMgc3VyZSB0aGF0Cist
LSByZXRyaWV2ZWRfcm93cyBmb3IgZm9yZWlnbiByZWwgcmUtdXNlZCB0byBjb3N0IHByZS1zb3J0
ZWQgZm9yZWlnbiBwYXRocyBpcworLS0gYSBzZW5zaWJsZSB2YWx1ZSBldmVuIHdoZW4gdGhlIHJl
bCBoYXMgdHVwbGVzPTAKK0NSRUFURSBUQUJMRSBsb2N0X2VtcHR5IChjMSBpbnQgTk9UIE5VTEws
IGMyIHRleHQpOworQ1JFQVRFIEZPUkVJR04gVEFCTEUgZnRfZW1wdHkgKGMxIGludCBOT1QgTlVM
TCwgYzIgdGV4dCkKKyAgU0VSVkVSIGxvb3BiYWNrIE9QVElPTlMgKHRhYmxlX25hbWUgJ2xvY3Rf
ZW1wdHknKTsKK0lOU0VSVCBJTlRPIGxvY3RfZW1wdHkKKyAgU0VMRUNUIGlkLCAnQUFBJyB8fCB0
b19jaGFyKGlkLCAnRk0wMDAnKSBGUk9NIGdlbmVyYXRlX3NlcmllcygxLCAxMDApIGlkOworREVM
RVRFIEZST00gbG9jdF9lbXB0eTsKK0FOQUxZWkUgZnRfZW1wdHk7CitFWFBMQUlOIChWRVJCT1NF
LCBDT1NUUyBPRkYpIFNFTEVDVCAqIEZST00gZnRfZW1wdHkgT1JERVIgQlkgYzE7CisgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgUVVFUlkgUExBTiAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgCistLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tCisgRm9yZWlnbiBTY2FuIG9u
IHB1YmxpYy5mdF9lbXB0eQorICAgT3V0cHV0OiBjMSwgYzIKKyAgIFJlbW90ZSBTUUw6IFNFTEVD
VCBjMSwgYzIgRlJPTSBwdWJsaWMubG9jdF9lbXB0eSBPUkRFUiBCWSBjMSBBU0MgTlVMTFMgTEFT
VAorKDMgcm93cykKKwogLS0gPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PQogLS0gV0hFUkUgd2l0aCByZW1vdGVseS1leGVj
dXRhYmxlIGNvbmRpdGlvbnMKIC0tID09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KZGlmZiAtLWdpdCBhL2NvbnRyaWIvcG9z
dGdyZXNfZmR3L3Bvc3RncmVzX2Zkdy5jIGIvY29udHJpYi9wb3N0Z3Jlc19mZHcvcG9zdGdyZXNf
ZmR3LmMKaW5kZXggMmNlNDJjZTNmMS4uMGU5NzcwNjZhOCAxMDA2NDQKLS0tIGEvY29udHJpYi9w
b3N0Z3Jlc19mZHcvcG9zdGdyZXNfZmR3LmMKKysrIGIvY29udHJpYi9wb3N0Z3Jlc19mZHcvcG9z
dGdyZXNfZmR3LmMKQEAgLTI5MjMsNyArMjkyMyw3IEBAIGVzdGltYXRlX3BhdGhfY29zdF9zaXpl
KFBsYW5uZXJJbmZvICpyb290LAogCQkgKi8KIAkJaWYgKGZwaW5mby0+cmVsX3N0YXJ0dXBfY29z
dCA+PSAwICYmIGZwaW5mby0+cmVsX3RvdGFsX2Nvc3QgPj0gMCkKIAkJewotCQkJQXNzZXJ0KGZw
aW5mby0+cmV0cmlldmVkX3Jvd3MgPj0gMSk7CisJCQlBc3NlcnQoZnBpbmZvLT5yZXRyaWV2ZWRf
cm93cyA+PSAwKTsKIAogCQkJcm93cyA9IGZwaW5mby0+cm93czsKIAkJCXJldHJpZXZlZF9yb3dz
ID0gZnBpbmZvLT5yZXRyaWV2ZWRfcm93czsKZGlmZiAtLWdpdCBhL2NvbnRyaWIvcG9zdGdyZXNf
ZmR3L3NxbC9wb3N0Z3Jlc19mZHcuc3FsIGIvY29udHJpYi9wb3N0Z3Jlc19mZHcvc3FsL3Bvc3Rn
cmVzX2Zkdy5zcWwKaW5kZXggMzE5YzE1ZDYzNS4uMTUxZjRmMTgzNCAxMDA2NDQKLS0tIGEvY29u
dHJpYi9wb3N0Z3Jlc19mZHcvc3FsL3Bvc3RncmVzX2Zkdy5zcWwKKysrIGIvY29udHJpYi9wb3N0
Z3Jlc19mZHcvc3FsL3Bvc3RncmVzX2Zkdy5zcWwKQEAgLTMwOSw2ICszMDksMTggQEAgU0VMRUNU
IHQxLiJDIDEiLCB0Mi5jMSwgdDMuYzEgRlJPTSAiUyAxIi4iVCAxIiB0MSBmdWxsIGpvaW4gZnQx
IHQyIGZ1bGwgam9pbiBmdDIKIFJFU0VUIGVuYWJsZV9oYXNoam9pbjsKIFJFU0VUIGVuYWJsZV9u
ZXN0bG9vcDsKIAorLS0gVGVzdCBleGVjdXRpbmcgYXNzZXJ0aW9uIGluIGVzdGltYXRlX3BhdGhf
Y29zdF9zaXplKCkgdGhhdCBtYWtlcyBzdXJlIHRoYXQKKy0tIHJldHJpZXZlZF9yb3dzIGZvciBm
b3JlaWduIHJlbCByZS11c2VkIHRvIGNvc3QgcHJlLXNvcnRlZCBmb3JlaWduIHBhdGhzIGlzCist
LSBhIHNlbnNpYmxlIHZhbHVlIGV2ZW4gd2hlbiB0aGUgcmVsIGhhcyB0dXBsZXM9MAorQ1JFQVRF
IFRBQkxFIGxvY3RfZW1wdHkgKGMxIGludCBOT1QgTlVMTCwgYzIgdGV4dCk7CitDUkVBVEUgRk9S
RUlHTiBUQUJMRSBmdF9lbXB0eSAoYzEgaW50IE5PVCBOVUxMLCBjMiB0ZXh0KQorICBTRVJWRVIg
bG9vcGJhY2sgT1BUSU9OUyAodGFibGVfbmFtZSAnbG9jdF9lbXB0eScpOworSU5TRVJUIElOVE8g
bG9jdF9lbXB0eQorICBTRUxFQ1QgaWQsICdBQUEnIHx8IHRvX2NoYXIoaWQsICdGTTAwMCcpIEZS
T00gZ2VuZXJhdGVfc2VyaWVzKDEsIDEwMCkgaWQ7CitERUxFVEUgRlJPTSBsb2N0X2VtcHR5Owor
QU5BTFlaRSBmdF9lbXB0eTsKK0VYUExBSU4gKFZFUkJPU0UsIENPU1RTIE9GRikgU0VMRUNUICog
RlJPTSBmdF9lbXB0eSBPUkRFUiBCWSBjMTsKKwogLS0gPT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQogLS0gV0hFUkUgd2l0
aCByZW1vdGVseS1leGVjdXRhYmxlIGNvbmRpdGlvbnMKIC0tID09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0gCjIuMTQu
MyAoQXBwbGUgR2l0LTk4KQoK
--000000000000dd67b205ba7fb78a--