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

From: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Amit Langote <amitlangote09(at)gmail(dot)com>, Noah Misch <noah(at)leadboat(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, Andreas Seltenreich <seltenreich(at)gmx(dot)de>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116
Date: 2016-06-10 08:55:19
Message-ID: 31b32da2-8cfa-8d4c-a43e-caf9964cae36@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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 postgres_fdw join push-down logic is concerned.

> What about other expressions that creep into the target list during
> subquery pull-up which are not safe to push down? See comments in
> set_append_rel_size(), recent discussion on the thread "Failed
> assertion in parallel worker (ExecInitSubPlan)", and commit
> b12fd41c695b43c76b0a9a4d19ba43b05536440c.

I went through the discussion on that thread. I see at least some
difference between how those considerations affect parallel-safety and
postgres_fdw join push-down safety. While parallelism is considered for
append child rels requiring guards discussed on that thread, the same does
not seem to hold for the join push-down case. The latter would fail the
safety check much earlier on the grounds of one of the component rels not
being pushdown_safe. That's because the failing component rel would be
append parent rel (not in its role as append child) so would not have any
foreign path to begin with. Any hazards introduced by subquery pull-up
then become irrelevant. That's the case when we have an inheritance tree
of foreign tables (headed by a foreign table). The other case (union all
with subquery pull-up) does not even get that far.

So the only case this fix should account for is where we have a single
foreign table entering a potential foreign join after being pulled up from
a subquery with unsafe PHVs being created that are referenced above the
join. If a given push-down attempt reaches as far as the check introduced
by the proposed patch, we can be sure that there are no other unsafe
expressions to worry about (accounted for by is_foreign_expr() checks up
to that point).

Am I missing something?

Thanks,
Amit

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Sridhar N Bamandlapally 2016-06-10 09:18:22 Re: [HACKERS] Online DW
Previous Message Craig Ringer 2016-06-10 08:52:58 Re: Online DW