On 2016/06/05 23:01, Andreas Seltenreich wrote:
> Creating some foreign tables via postgres_fdw in the regression db of
> master as of de33af8, sqlsmith triggers the following assertion:
> 
>     TRAP: FailedAssertion("!(((((const Node*)(var))->type) == T_Var))", File: 
> "deparse.c", Line: 1116)
> 
> gdb says var is holding a T_PlaceHolderVar instead.  In a build without
> assertions, it leads to an error later:
> 
>     ERROR:  cache lookup failed for type 0
> 
> Recipe:
> 
> --8<---------------cut here---------------start------------->8---
> create extension postgres_fdw;
> create server myself foreign data wrapper postgres_fdw;
> create schema fdw_postgres;
> create user mapping for public server myself options (user :'USER');
> import foreign schema public from server myself into fdw_postgres;
> select subq_0.c0 as c0 from
>        (select 31 as c0 from fdw_postgres.a as ref_0
>             where 93 >= ref_0.aa) as subq_0
>        right join fdw_postgres.rtest_vview5 as ref_1
>        on (subq_0.c0 = ref_1.a )
>        where 92 = subq_0.c0;
> --8<---------------cut here---------------end--------------->8---

Thanks for the example.  It seems that postgres_fdw join-pushdown logic
(within foreign_join_ok()?) should reject a join if any PlaceHolderVars in
its targetlist are required above it.  Tried to do that with the attached
patch which trivially fixes the reported assertion failure.

A guess from my reading of the patch's thread [1] is that to support such
a case, join deparse code should be able to construct subqueries which it
currently doesn't support.  I may be missing something though.

Thanks,
Amit

[1]
https://www.postgresql.org/message-id/CAFjFpRdHgeNOhM0AB6Gxz1eVx_yOqkYwuKddZeB5vPzfBaeCnQ%40mail.gmail.com
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 4d17272..ec86b9a 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -4036,6 +4036,20 @@ foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype,
 			return false;
 	}
 
+	/*
+	 * Cannot push down if any PlaceHolderVars in its result are needed above
+	 * the join.
+	 */
+	foreach(lc, root->placeholder_list)
+	{
+		PlaceHolderInfo	   *phinfo = lfirst(lc);
+		Relids				relids = joinrel->relids;
+
+		if (bms_nonempty_difference(phinfo->ph_needed, relids) &&
+			bms_is_subset(phinfo->ph_eval_at, relids))
+			return false;
+	}
+
 	/* Save the join clauses, for later use. */
 	fpinfo->joinclauses = joinclauses;
 
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to