Re: [HACKERS] Re: [COMMITTERS] pgsql: Expand partitioned table RTEs level by level, without flattening

[Robert Haas]

On Mon, Sep 18, 2017 at 3:30 AM, Ashutosh Bapat
 wrote:
>> testing with sqlsmith shows that the following assertion in this commit
>> doesn't hold:
>>
>> TRAP: FailedAssertion("!(((brel)->reloptkind == RELOPT_BASEREL ||
>> (brel)->reloptkind == RELOPT_OTHER_MEMBER_REL))", File: "initsplan.c", Line:
>> 647)
>>
>> One of the simpler queries that triggers it for me:
>>
>>select from information_schema.user_mapping_options;
>
> Thanks Andreas for the report. Sorry for the assertion failure.
>
> PFA patch to fix the assertion failure.
>
> The assertion assumed that all relations in simple_rel_array[] were
> either "base" relations or "other" relations. This isn't true. The
> array can contain "dead" relations as well. I have removed the
> assertion and instead fixed the code to skip anything which is not
> "base" or "other member rel".
>
> I have also added a test to cover dead relations and lateral
> references in join.sql.

Committed.  Thanks.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Re: [COMMITTERS] pgsql: Expand partitioned table RTEs level by level, without flattening

[Ashutosh Bapat]

On Sun, Sep 17, 2017 at 7:24 AM, Robert Haas  wrote:
> Can you debug this on Monday?
>
> ...Robert
>
> Begin forwarded message:
>
> From: Andreas Seltenreich 
> Date: September 16, 2017 at 6:55:46 AM EDT
> To: Robert Haas 
> Cc: pgsql-committ...@postgresql.org
> Subject: Re: [COMMITTERS] pgsql: Expand partitioned table RTEs level by
> level, without flattening
>
> Robert Haas writes:
>
> Expand partitioned table RTEs level by level, without flattening.
>
>
> testing with sqlsmith shows that the following assertion in this commit
> doesn't hold:
>
> TRAP: FailedAssertion("!(((brel)->reloptkind == RELOPT_BASEREL ||
> (brel)->reloptkind == RELOPT_OTHER_MEMBER_REL))", File: "initsplan.c", Line:
> 647)
>
> One of the simpler queries that triggers it for me:
>
>select from information_schema.user_mapping_options;

Thanks Andreas for the report. Sorry for the assertion failure.

PFA patch to fix the assertion failure.

The assertion assumed that all relations in simple_rel_array[] were
either "base" relations or "other" relations. This isn't true. The
array can contain "dead" relations as well. I have removed the
assertion and instead fixed the code to skip anything which is not
"base" or "other member rel".

I have also added a test to cover dead relations and lateral
references in join.sql.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
From 0227d3357d0b988183af439bb278e696d516bc5f Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat 
Date: Mon, 18 Sep 2017 12:16:24 +0530
Subject: [PATCH] Skip "dead" relations in create_lateral_join_info()

Prior to commit 0a480502b092195a9b25a2f0f199a21d592a9c57, the loop to propagate
lateral relation information to the child rels was skipping "dead" relations by
skipping any relation with reloptkind other than RELOPT_BASEREL. This commit
enabled lateral relation information to be percolated through relations with
reloptkind RELOPT_OTHER_MEMBER_REL. But it assumed that simple_rel_array can
have only RELOPT_OTHER_MEMBER_REL or RELOPT_BASEREL. Obviously that's not true.
Fix the loop to skip "dead" relations.

Ashutosh Bapat, reported by Andreas Seltenreich.
---
 src/backend/optimizer/plan/initsplan.c |7 +--
 src/test/regress/expected/join.out |   12 
 src/test/regress/sql/join.sql  |5 +
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index ad81f0f..9931ddd 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -632,7 +632,11 @@ create_lateral_join_info(PlannerInfo *root)
 		RelOptInfo *brel = root->simple_rel_array[rti];
 		RangeTblEntry *brte = root->simple_rte_array[rti];
 
-		if (brel == NULL)
+		/*
+		 * Skip empty slots. Also skip non-simple relations i.e. dead
+		 * relations.
+		 */
+		if (brel == NULL || !IS_SIMPLE_REL(brel))
 			continue;
 
 		/*
@@ -644,7 +648,6 @@ create_lateral_join_info(PlannerInfo *root)
 		 * therefore be marked with the appropriate lateral info so that those
 		 * children eventually get marked also.
 		 */
-		Assert(IS_SIMPLE_REL(brel));
 		Assert(brte);
 		if (brel->reloptkind == RELOPT_OTHER_MEMBER_REL &&
 			(brte->rtekind != RTE_RELATION ||
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 06a84e8..f47449b 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -4060,6 +4060,18 @@ select i8.* from int8_tbl i8 left join (select f1 from int4_tbl group by f1) i4
  Seq Scan on int8_tbl i8
 (1 row)
 
+-- check join removal with lateral references
+explain (costs off)
+select 1 from (select a.id FROM a left join b on a.b_id = b.id) q,
+			  lateral generate_series(1, q.id) gs(i) where q.id = gs.i;
+QUERY PLAN 
+---
+ Nested Loop
+   ->  Seq Scan on a
+   ->  Function Scan on generate_series gs
+ Filter: (a.id = i)
+(4 rows)
+
 rollback;
 create temp table parent (k int primary key, pd int);
 create temp table child (k int unique, cd int);
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 8b21838..d847d53 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -1336,6 +1336,11 @@ explain (costs off)
 select i8.* from int8_tbl i8 left join (select f1 from int4_tbl group by f1) i4
   on i8.q1 = i4.f1;
 
+-- check join removal with lateral references
+explain (costs off)
+select 1 from (select a.id FROM a left join b on a.b_id = b.id) q,
+			  lateral generate_series(1, q.id) gs(i) where q.id = gs.i;
+
 rollback;
 
 create temp table parent (k int primary key, pd int);
-- 
1.7.9.5


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription: