Re: Oversight in reparameterize_path_by_child leading to executor crash

On Thu, Aug 24, 2023 at 8:17 AM Richard Guo <guofenglinux@gmail.com> wrote:
>
>
> On Thu, Aug 24, 2023 at 1:44 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>
>> Richard Guo <guofenglinux@gmail.com> writes:
>> > If we go with the "tablesample scans can't be reparameterized" approach
>> > in the back branches, I'm a little concerned that what if we find more
>> > cases in the futrue where we need modify RTEs for reparameterization.
>> > So I spent some time seeking and have managed to find one: there might
>> > be lateral references in a scan path's restriction clauses, and
>> > currently reparameterize_path_by_child fails to adjust them.
>>
>> Hmm, this seems completely wrong to me.  By definition, such clauses
>> ought to be join clauses not restriction clauses, so how are we getting
>> into this state?  IOW, I agree this is clearly buggy but I think the
>> bug is someplace else.
>
>
> If the clause contains PHVs that syntactically belong to a rel and
> meanwhile have lateral references to other rels, then it may become a
> restriction clause with lateral references.  Take the query shown
> upthread as an example,
>
> select count(*) from prt1 t1 left join lateral
>     (select t1.b as t1b, t2.* from prt2 t2) s
>     on t1.a = s.b where s.t1b = s.a;
>
> The clause 's.t1b = s.a' would become 'PHV(t1.b) = t2.a' after we have
> pulled up the subquery.  The PHV in it syntactically belongs to 't2' and
> laterally refers to 't1'.  So this clause is actually a restriction
> clause for rel 't2', and will be put into the baserestrictinfo of t2
> rel.  But it also has lateral reference to rel 't1', which we need to
> adjust in reparameterize_path_by_child for partitionwise join.

When the clause s.t1b = s.a is presented to distribute_qual_to_rels()
it has form PHV(t1.b) = t2.b. The PHV's ph_eval_at is 4, which is what
is returned as varno to pull_varnos(). The other Var in the caluse has
varno = 4 already so  pull_varnos() returns a SINGLETON relids (b 4).
The clause is an equality clause, so it is used to create an
Equivalence class.
generate_base_implied_equalities_no_const() then constructs the same
RestrictInfo again and adds to baserestrictinfo of Rel with relid = 4
i.e. t2's baserestrictinfo. I don't know whether that's the right
thing to do. After the subquery has been pulled up, t1 and t2 can be
joined at the same level and thus the clause makes more sense as  a
joininfo in both t1 as well as t2. So I tend to agree with Tom. That's
how it will move up the join tree and be evaluated at appropriate
level. But then why the query returns the right results is a mystery.

I am not sure where we are taking the original bug fix with this
investigation. Is it required to fix this problem in order to fix the
original problem OR we should commit the fix for the original problem
and then investigate this further?

--
Best Wishes,
Ashutosh Bapat