Push down join with PHVs (WAS: Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116)

2016-06-20 Thread Etsuro Fujita
On 2016/06/17 21:45, Robert Haas wrote: On Thu, Jun 16, 2016 at 10:44 PM, Etsuro Fujita wrote: On 2016/06/16 22:00, Robert Haas wrote: On Thu, Jun 16, 2016 at 7:05 AM, Etsuro Fujita wrote: ISTM that a robuster solution to this is to push down the ft1-ft2-ft3 join with the PHV by extending d

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-17 Thread Robert Haas
On Thu, Jun 16, 2016 at 10:44 PM, Etsuro Fujita wrote: > On 2016/06/16 22:00, Robert Haas wrote: >> On Thu, Jun 16, 2016 at 7:05 AM, Etsuro Fujita >> wrote: >>> >>> ISTM that a robuster solution to this is to push down the ft1-ft2-ft3 >>> join >>> with the PHV by extending deparseExplicitTargetLi

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-16 Thread Etsuro Fujita
On 2016/06/16 22:00, Robert Haas wrote: On Thu, Jun 16, 2016 at 7:05 AM, Etsuro Fujita wrote: ISTM that a robuster solution to this is to push down the ft1-ft2-ft3 join with the PHV by extending deparseExplicitTargetList() and/or something else so that we can send the remote query like: select

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-16 Thread Robert Haas
On Thu, Jun 16, 2016 at 7:05 AM, Etsuro Fujita wrote: > ISTM that a robuster solution to this is to push down the ft1-ft2-ft3 join > with the PHV by extending deparseExplicitTargetList() and/or something else > so that we can send the remote query like: > > select ft1.a, (ft3.a IS NOT NULL) from (

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-16 Thread Etsuro Fujita
On 2016/06/15 9:13, Amit Langote wrote: On 2016/06/15 0:50, Robert Haas wrote: On Tue, Jun 14, 2016 at 4:06 AM, Amit Langote wrote: Attached new version of the patch with following changes: OK, I committed this version with some cosmetic changes. Thank you all for working on this! While r

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-14 Thread Amit Langote
On 2016/06/15 0:50, Robert Haas wrote: > On Tue, Jun 14, 2016 at 4:06 AM, Amit Langote wrote: >> You're right. It indeed should be possible to push down ft1-ft2 join. >> However it could not be done without also modifying >> build_tlist_to_deparse() a little (as Ashutosh proposed [1] to do >> upth

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-14 Thread Robert Haas
On Tue, Jun 14, 2016 at 4:06 AM, Amit Langote wrote: > On 2016/06/14 6:51, Robert Haas wrote: >> On Fri, Jun 10, 2016 at 4:14 PM, Robert Haas wrote: >>> Although I have done a bit of review of this patch, it needs more >>> thought than I have so far had time to give it. I will update again >>> b

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-14 Thread Amit Langote
On 2016/06/14 6:51, Robert Haas wrote: > On Fri, Jun 10, 2016 at 4:14 PM, Robert Haas wrote: >> Although I have done a bit of review of this patch, it needs more >> thought than I have so far had time to give it. I will update again >> by Tuesday. > > I've reviewed this a bit further and have di

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-13 Thread Ashutosh Bapat
On Tue, Jun 14, 2016 at 4:10 AM, Robert Haas wrote: > On Mon, Jun 13, 2016 at 5:51 PM, Robert Haas > wrote: > > On Fri, Jun 10, 2016 at 4:14 PM, Robert Haas > wrote: > >> Although I have done a bit of review of this patch, it needs more > >> thought than I have so far had time to give it. I wi

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-13 Thread Robert Haas
On Mon, Jun 13, 2016 at 5:51 PM, Robert Haas wrote: > On Fri, Jun 10, 2016 at 4:14 PM, Robert Haas wrote: >> Although I have done a bit of review of this patch, it needs more >> thought than I have so far had time to give it. I will update again >> by Tuesday. > > I've reviewed this a bit furthe

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-13 Thread Robert Haas
On Fri, Jun 10, 2016 at 4:14 PM, Robert Haas wrote: > Although I have done a bit of review of this patch, it needs more > thought than I have so far had time to give it. I will update again > by Tuesday. I've reviewed this a bit further and have discovered an infelicity. The following is all wit

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-10 Thread Robert Haas
On Wed, Jun 8, 2016 at 8:27 AM, Robert Haas wrote: > On Tue, Jun 7, 2016 at 10:41 PM, Noah Misch wrote: >> [Action required within 72 hours. This is a generic notification.] >> >> The above-described topic is currently a PostgreSQL 9.6 open item. Robert, >> since you committed the patch believe

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-10 Thread Amit Langote
On 2016/06/10 2:07, Robert Haas wrote: > On Thu, Jun 9, 2016 at 5:50 AM, Amit Langote wrote: >> I adjusted some comments per off-list suggestion from Ashutosh. Please >> find attached the new version. > > Are PlaceHolderVars the only problem we need to worry about here? It seems so, as far as po

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-09 Thread Robert Haas
On Thu, Jun 9, 2016 at 5:50 AM, Amit Langote wrote: > On 2016/06/08 23:16, Amit Langote wrote: >> On Wed, Jun 8, 2016 at 9:27 PM, Robert Haas wrote: >>> On Tue, Jun 7, 2016 at 10:41 PM, Noah Misch wrote: [Action required within 72 hours. This is a generic notification.] The above

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-09 Thread Amit Langote
On 2016/06/08 23:16, Amit Langote wrote: > On Wed, Jun 8, 2016 at 9:27 PM, Robert Haas wrote: >> On Tue, Jun 7, 2016 at 10:41 PM, Noah Misch wrote: >>> [Action required within 72 hours. This is a generic notification.] >>> >>> The above-described topic is currently a PostgreSQL 9.6 open item. R

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-08 Thread Amit Langote
On Wed, Jun 8, 2016 at 9:27 PM, Robert Haas wrote: > On Tue, Jun 7, 2016 at 10:41 PM, Noah Misch wrote: >> [Action required within 72 hours. This is a generic notification.] >> >> The above-described topic is currently a PostgreSQL 9.6 open item. Robert, >> since you committed the patch believe

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-08 Thread Robert Haas
On Tue, Jun 7, 2016 at 10:41 PM, Noah Misch wrote: > [Action required within 72 hours. This is a generic notification.] > > The above-described topic is currently a PostgreSQL 9.6 open item. Robert, > since you committed the patch believed to have created it, you own this open > item. If some o

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-08 Thread Ashutosh Bapat
On Wed, Jun 8, 2016 at 12:25 PM, Amit Langote wrote: > On 2016/06/08 14:13, Ashutosh Bapat wrote: > > On Tue, Jun 7, 2016 at 6:19 PM, Amit Langote wrote: > >> On Tue, Jun 7, 2016 at 7:47 PM, Ashutosh Bapat wrote: > >>> Looks good to me. If we add a column from the outer relation, the > >> "NULL"n

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-07 Thread Amit Langote
On 2016/06/08 14:13, Ashutosh Bapat wrote: > On Tue, Jun 7, 2016 at 6:19 PM, Amit Langote wrote: >> On Tue, Jun 7, 2016 at 7:47 PM, Ashutosh Bapat wrote: >>> Looks good to me. If we add a column from the outer relation, the >> "NULL"ness >>> of inner column would be more clear. May be we should twe

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-07 Thread Ashutosh Bapat
On further thought, I think we need to restrict the join pushdown only for outer joins. Only those joins can produce NULL rows. If we go with that change, we will need my changes as well and a testcase with inner join. On Tue, Jun 7, 2016 at 6:19 PM, Amit Langote wrote: > On Tue, Jun 7, 2016 at

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-07 Thread Noah Misch
On Tue, Jun 07, 2016 at 09:49:23PM +0900, Amit Langote wrote: > On Tue, Jun 7, 2016 at 7:47 PM, Ashutosh Bapat wrote: > > On Tue, Jun 7, 2016 at 4:07 PM, Amit Langote wrote: > >> On 2016/06/07 19:13, Ashutosh Bapat wrote: > >> > So, your patch looks to be the correct approach (even after we support

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-07 Thread Amit Langote
On Tue, Jun 7, 2016 at 7:47 PM, Ashutosh Bapat wrote: > On Tue, Jun 7, 2016 at 4:07 PM, Amit Langote wrote: >> On 2016/06/07 19:13, Ashutosh Bapat wrote: >> > So, your patch looks to be the correct approach (even after we support >> > deparsing subqueries). Can you please include a test in regressi

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-07 Thread Ashutosh Bapat
On Tue, Jun 7, 2016 at 4:07 PM, Amit Langote wrote: > On 2016/06/07 19:13, Ashutosh Bapat wrote: > > I thought, columns of inner relation will be set to null during > projection > > from ForeignScan for joins. But I was wrong. If we want to push-down > joins > > in this case, we have two solution

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-07 Thread Amit Langote
On 2016/06/07 19:13, Ashutosh Bapat wrote: > I thought, columns of inner relation will be set to null during projection > from ForeignScan for joins. But I was wrong. If we want to push-down joins > in this case, we have two solutions > 1. Build queries with subqueries at the time of deparsing. Thu

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-07 Thread Ashutosh Bapat
> That's the patch I came up with initially but it seemed to me to produce > the wrong result. Correct me if that is not so: > > CREATE EXTENSION IF NOT EXISTS postgres_fdw WITH SCHEMA public; > > CREATE SERVER myserver FOREIGN DATA WRAPPER postgres_fdw OPTIONS (dbname > 'test'); > > CREATE USER M

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-07 Thread Amit Langote
Hi Ashutosh, On 2016/06/07 17:02, Ashutosh Bapat wrote: > On Tue, Jun 7, 2016 at 11:36 AM, Amit Langote wrote: >> On 2016/06/05 23:01, Andreas Seltenreich wrote: ... >>> --8<---cut here---start->8--- >>> create extension postgres_fdw; >>> create server myself

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-07 Thread Ashutosh Bapat
On Tue, Jun 7, 2016 at 11:36 AM, Amit Langote wrote: > 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

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-06 Thread Michael Paquier
On Sun, Jun 5, 2016 at 11:01 PM, 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", L

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

2016-06-06 Thread Amit Langote
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) >