Re: BUG #17540: Prepared statement: PG switches to a generic query plan which is consistently much slower

On Tue, Sep 26, 2023 at 9:42 PM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:

> I ended up moving the function that looks for the NullTest quals in
> the joinlist out so it's done after the quals have been distributed to
> the relations.

It seems that the RestrictInfos for the "same" qual condition still may
get different serial numbers even if transform_join_clauses() is called
after we've distributed all the quals. For example,

select * from t1
left join t2 on t1.a = t2.c
left join t3 on t2.c = t3.e and t2.c is null;

There are two versions for qual 't2.c is null': with and without being
marked nullable by t1/t2 join. Let's write them as 'c* is null' and 'c
is null'. They are supposed to have identical serial number. But after
we've transformed 'c is null' to 'false', they do not have identical
serial number any more. This may cause problems where the logic of
serial numbers is relied on?

> I'm not really that happy with this as if we ever
> found some way to optimise quals that could be made part of an
> EquivalenceClass then those quals would have already have been
> processed to become EquivalenceClasses. I just don't see how to do it
> earlier as deconstruct_distribute_oj_quals() calls
> remove_nulling_relids() which changes the Var's varnullingrels causing
> them to be empty during the processing of the NullTest qual.

Hmm, I don't think it's a problem that deconstruct_distribute_oj_quals
changes the nullingrels. It will compute the correct nullingrels at
last for different clones of the same qual condition. We can just check
the nullingrels whatever it computes.

> It's also not so great that the RestrictInfo gets duplicated in:
>
> Adjusting the code to build a new false clause and setting that in the
> existing RestrictInfo rather than building a new RestrictInfo seems to
> fix that. I wondered if the duplication was a result of the
> rinfo_serial number changing.

The RestrictInfo nodes in different joinlists are multiply-linked rather
than copied, so when building restrictlist for a joinrel we use pointer
equality to remove any duplication. In your patch new RestrictInfo
nodes are created in transform_join_clauses(), so pointer equality no
longer works and we see duplication in the plan.

> Checking back to the original MinMaxAgg I'm not sure if this is all
> getting more complex than it's worth or not.

It seems that optimizing IS NULL quals is more complex than optimizing
IS NOT NULL quals. I also wonder if it's worth the trouble to optimize
IS NULL quals.

BTW, there is an Assert failure running regression tests with your
patch. I haven't looked into it.

Thanks
Richard