From: Etsuro Fujita <etsuro.fujita@gmail.com>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: exclusion@gmail.com, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #16807: Assert failed in postgres_fdw/estimate_path_cost_size with an empty foreign table
Date: Thu, 4 Feb 2021 18:56:22 +0900
Message-ID: <CAPmGK141OhAhUQu9MyvFWDGuWJUMTbDZGO0N-hLEFJsF7ZQ17A@mail.gmail.com> (raw)
In-Reply-To: <20210114.133024.160907178608761711.horikyota.ntt@gmail.com>
References: <16807-9fe4e08fbaa5c7ce@postgresql.org>
<CAPmGK17nuChuVAe0wr_51OxxLewbZdefwYNNF=wNVGjLcTwADQ@mail.gmail.com>
<CAPmGK16Te7mLsxJ6yaPmJy_qFdmXEmqegxXm4sNQRprYsxtmfg@mail.gmail.com>
<20210114.133024.160907178608761711.horikyota.ntt@gmail.com>
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.com> 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’ll commit the patch.
Best regards,
Etsuro Fujita
Attachments:0001-postgres_fdw-Fix-assertion-in-estimate_path_cost_siz.patch (application/octet-stream, 4.1K) download
From c10d3ac78867859d623a36cceed22f2ec2ff9b8c Mon Sep 17 00:00:00 2001
From: Etsuro Fujita <etsuro.fujita@gmail.com>
Date: Thu, 4 Feb 2021 18:48:37 +0900
Subject: [PATCH] postgres_fdw: Fix assertion in estimate_path_cost_size().
In commit 08d2d58a2, I assumed that the retrieved_rows estimate for a
foreign relation, which is re-used to cost pre-sorted foreign paths with
local stats, is set to at least one row in estimate_path_cost_size(),
which is not correct: when the relation is a foreign table with tuples=0,
the estimate would be set to 0 when not using remote estimates.
Back-patch to v13 where the aforementioned commit went in.
Per bug #16807 from Alexander Lakhin.
Author: Etsuro Fujita
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/16807-9fe4e08fbaa5c7ce%40postgresql.org
---
contrib/postgres_fdw/expected/postgres_fdw.out | 18 ++++++++++++++++++
contrib/postgres_fdw/postgres_fdw.c | 2 +-
contrib/postgres_fdw/sql/postgres_fdw.sql | 12 ++++++++++++
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.outindex b09dce63f5..60c7e115d6 100644--- a/contrib/postgres_fdw/expected/postgres_fdw.out+++ b/contrib/postgres_fdw/expected/postgres_fdw.out@@ -613,6 +613,24 @@ SELECT t1."C 1", t2.c1, t3.c1 FROM "S 1"."T 1" t1 full join ft1 t2 full join ft2
RESET enable_hashjoin;
RESET enable_nestloop;
+-- Test executing assertion in estimate_path_cost_size() that makes sure that+-- retrieved_rows for foreign rel re-used to cost pre-sorted foreign paths is+-- a sensible value even when the rel has tuples=0+CREATE TABLE loct_empty (c1 int NOT NULL, c2 text);+CREATE FOREIGN TABLE ft_empty (c1 int NOT NULL, c2 text)+ SERVER loopback OPTIONS (table_name 'loct_empty');+INSERT INTO loct_empty+ SELECT id, 'AAA' || to_char(id, 'FM000') FROM generate_series(1, 100) id;+DELETE FROM loct_empty;+ANALYZE ft_empty;+EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft_empty ORDER BY c1;+ QUERY PLAN +-------------------------------------------------------------------------------+ Foreign Scan on public.ft_empty+ Output: c1, c2+ Remote SQL: SELECT c1, c2 FROM public.loct_empty ORDER BY c1 ASC NULLS LAST+(3 rows)+
-- ===================================================================
-- WHERE with remotely-executable conditions
-- ===================================================================
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.cindex 2ce42ce3f1..0e977066a8 100644--- a/contrib/postgres_fdw/postgres_fdw.c+++ b/contrib/postgres_fdw/postgres_fdw.c@@ -2923,7 +2923,7 @@ estimate_path_cost_size(PlannerInfo *root,
*/
if (fpinfo->rel_startup_cost >= 0 && fpinfo->rel_total_cost >= 0)
{
- Assert(fpinfo->retrieved_rows >= 1);+ Assert(fpinfo->retrieved_rows >= 0);
rows = fpinfo->rows;
retrieved_rows = fpinfo->retrieved_rows;
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sqlindex 319c15d635..151f4f1834 100644--- a/contrib/postgres_fdw/sql/postgres_fdw.sql+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql@@ -309,6 +309,18 @@ SELECT t1."C 1", t2.c1, t3.c1 FROM "S 1"."T 1" t1 full join ft1 t2 full join ft2
RESET enable_hashjoin;
RESET enable_nestloop;
+-- Test executing assertion in estimate_path_cost_size() that makes sure that+-- retrieved_rows for foreign rel re-used to cost pre-sorted foreign paths is+-- a sensible value even when the rel has tuples=0+CREATE TABLE loct_empty (c1 int NOT NULL, c2 text);+CREATE FOREIGN TABLE ft_empty (c1 int NOT NULL, c2 text)+ SERVER loopback OPTIONS (table_name 'loct_empty');+INSERT INTO loct_empty+ SELECT id, 'AAA' || to_char(id, 'FM000') FROM generate_series(1, 100) id;+DELETE FROM loct_empty;+ANALYZE ft_empty;+EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft_empty ORDER BY c1;+
-- ===================================================================
-- WHERE with remotely-executable conditions
-- ===================================================================
--
2.14.3 (Apple Git-98)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: etsuro.fujita@gmail.com, horikyota.ntt@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16807: Assert failed in postgres_fdw/estimate_path_cost_size with an empty foreign table
In-Reply-To: <CAPmGK141OhAhUQu9MyvFWDGuWJUMTbDZGO0N-hLEFJsF7ZQ17A@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox