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 1rMFM9-003ZoI-Hq for pgsql-hackers@arkaria.postgresql.org; Sat, 06 Jan 2024 22:41:30 +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 1rMFM6-00C7aN-Od for pgsql-hackers@arkaria.postgresql.org; Sat, 06 Jan 2024 22:41:26 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1rMFM6-00C7aD-Ez for pgsql-hackers@lists.postgresql.org; Sat, 06 Jan 2024 22:41:26 +0000 Received: from sss.pgh.pa.us ([68.162.161.243]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1rMFLz-000DU6-Gr for pgsql-hackers@lists.postgresql.org; Sat, 06 Jan 2024 22:41:25 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 406MfEZe1450798; Sat, 6 Jan 2024 17:41:14 -0500 From: Tom Lane To: Alexander Lakhin cc: Richard Guo , Jian Guo , Tomas Vondra , Hans Buschmann , "pgsql-hackers@lists.postgresql.org" , Zhenghua Lyu Subject: Re: Wrong rows estimations with joins of CTEs slows queries by more than factor 500 In-reply-to: <608a4886-6c60-0f9e-97d5-591256bd4150@gmail.com> References: <4504e67078d648cdac3651b2960da6e7@nidsa.net> <46a7d950-7ca7-8ab9-76dd-f0bc7611c609@enterprisedb.com> <891440.1699483495@sss.pgh.pa.us> <414317.1700158568@sss.pgh.pa.us> <482957.1700192299@sss.pgh.pa.us> <754093.1700250120@sss.pgh.pa.us> <608a4886-6c60-0f9e-97d5-591256bd4150@gmail.com> Comments: In-reply-to Alexander Lakhin message dated "Sat, 06 Jan 2024 12:00:01 +0300" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <1450684.1704580829.0@sss.pgh.pa.us> Content-Transfer-Encoding: 8bit Date: Sat, 06 Jan 2024 17:41:14 -0500 Message-ID: <1450797.1704580874@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="UTF-8" Content-ID: <1450684.1704580829.1@sss.pgh.pa.us> Content-Transfer-Encoding: 8bit Alexander Lakhin writes: > Please look at the following query: > CREATE TABLE t(i int); > INSERT INTO t VALUES (1); > VACUUM ANALYZE t; > WITH ir AS (INSERT INTO t VALUES (2) RETURNING i) > SELECT * FROM ir WHERE i = 2; > which produces ERROR:  no relation entry for relid 1 > starting from f7816aec2. Thanks for the report! I guess we need something like the attached. I'm surprised that this hasn't been noticed before; was the case really unreachable before? regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="fix-examine-variable-for-INSERT-RETURNING.patch"; charset="us-ascii" Content-ID: <1450684.1704580829.2@sss.pgh.pa.us> Content-Description: fix-examine-variable-for-INSERT-RETURNING.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/= util/relnode.c index 5a9ce44532..22d01cef5b 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -420,6 +420,19 @@ find_base_rel(PlannerInfo *root, int relid) return NULL; /* keep compiler quiet */ } = +/* + * find_base_rel_noerr + * Find a base or otherrel relation entry, returning NULL if there's no= ne + */ +RelOptInfo * +find_base_rel_noerr(PlannerInfo *root, int relid) +{ + /* use an unsigned comparison to prevent negative array element access *= / + if ((uint32) relid < (uint32) root->simple_rel_array_size) + return root->simple_rel_array[relid]; + return NULL; +} + /* * find_base_rel_ignore_join * Find a base or otherrel relation entry, which must already exist. diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/self= uncs.c index dbcd98d985..cea777e9d4 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -119,6 +119,7 @@ #include "optimizer/paths.h" #include "optimizer/plancat.h" #include "parser/parse_clause.h" +#include "parser/parse_relation.h" #include "parser/parsetree.h" #include "statistics/statistics.h" #include "storage/bufmgr.h" @@ -5434,17 +5435,30 @@ examine_simple_variable(PlannerInfo *root, Var *va= r, = if (HeapTupleIsValid(vardata->statsTuple)) { - RelOptInfo *onerel =3D find_base_rel(root, var->varno); + RelOptInfo *onerel =3D find_base_rel_noerr(root, var->varno); Oid userid; = /* * Check if user has permission to read this column. We require * all rows to be accessible, so there must be no securityQuals - * from security barrier views or RLS policies. Use - * onerel->userid if it's set, in case we're accessing the table - * via a view. + * from security barrier views or RLS policies. + * + * Normally the Var will have an associated RelOptInfo from which + * we can find out which userid to do the check as; but it might + * not if it's a RETURNING Var for an INSERT target relation. In + * that case use the RTEPermissionInfo associated with the RTE. */ - userid =3D OidIsValid(onerel->userid) ? onerel->userid : GetUserId(); + if (onerel) + userid =3D onerel->userid; + else + { + RTEPermissionInfo *perminfo; + + perminfo =3D getRTEPermissionInfo(root->parse->rteperminfos, rte); + userid =3D perminfo->checkAsUser; + } + if (!OidIsValid(userid)) + userid =3D GetUserId(); = vardata->acl_ok =3D rte->securityQuals =3D=3D NIL && diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/path= node.h index 03a0e46e70..c43d97b48a 100644 --- a/src/include/optimizer/pathnode.h +++ b/src/include/optimizer/pathnode.h @@ -307,6 +307,7 @@ extern void expand_planner_arrays(PlannerInfo *root, i= nt add_size); extern RelOptInfo *build_simple_rel(PlannerInfo *root, int relid, RelOptInfo *parent); extern RelOptInfo *find_base_rel(PlannerInfo *root, int relid); +extern RelOptInfo *find_base_rel_noerr(PlannerInfo *root, int relid); extern RelOptInfo *find_base_rel_ignore_join(PlannerInfo *root, int relid= ); extern RelOptInfo *find_join_rel(PlannerInfo *root, Relids relids); extern RelOptInfo *build_join_rel(PlannerInfo *root, diff --git a/src/test/regress/expected/with.out b/src/test/regress/expecte= d/with.out index 69c56ce207..3cf969d938 100644 --- a/src/test/regress/expected/with.out +++ b/src/test/regress/expected/with.out @@ -654,6 +654,24 @@ select count(*) from tenk1 a -> CTE Scan on x (8 rows) = +explain (costs off) +with x as materialized (insert into tenk1 default values returning unique= 1) +select count(*) from tenk1 a + where unique1 in (select * from x); + QUERY PLAN = +------------------------------------------------------------ + Aggregate + CTE x + -> Insert on tenk1 + -> Result + -> Nested Loop + -> HashAggregate + Group Key: x.unique1 + -> CTE Scan on x + -> Index Only Scan using tenk1_unique1 on tenk1 a + Index Cond: (unique1 =3D x.unique1) +(10 rows) + -- SEARCH clause create temp table graph0( f int, t int, label text ); insert into graph0 values diff --git a/src/test/regress/sql/with.sql b/src/test/regress/sql/with.sql index 3ef9898866..ff68e21e2e 100644 --- a/src/test/regress/sql/with.sql +++ b/src/test/regress/sql/with.sql @@ -354,6 +354,11 @@ with x as materialized (select unique1 from tenk1 b) select count(*) from tenk1 a where unique1 in (select * from x); = +explain (costs off) +with x as materialized (insert into tenk1 default values returning unique= 1) +select count(*) from tenk1 a + where unique1 in (select * from x); + -- SEARCH clause = create temp table graph0( f int, t int, label text ); ------- =_aaaaaaaaaa0--