Re: SQLFunctionCache and generic plans

Alexander Pyhalov <a.pyhalov@postgrespro.ru>

From: Alexander Pyhalov <a.pyhalov@postgrespro.ru>
To: Pavel Stehule <pavel.stehule@gmail.com>
Cc: Alexander Korotkov <aekorotkov@gmail.com>, pgsql-hackers@lists.postgresql.org, Ronan Dunklau <ronan.dunklau@aiven.io>, Tom Lane <tgl@sss.pgh.pa.us>
Date: 2025-01-17T07:15:34Z
Lists: pgsql-hackers

Attachments

Pavel Stehule писал(а) 2024-12-31 18:39:
> Hi
> 
> út 31. 12. 2024 v 16:36 odesílatel Alexander Pyhalov
> <a.pyhalov@postgrespro.ru> napsal:
> 
>> Hi.
>> 
>>>> What should we do with "pre-parsed" SQL functions (when prosrc is
>>>> empty)? How should we create cached plans when we don't have raw
>>>> parsetrees?
>>>> Currently we can create cached plans without raw parsetrees, but
>> this
>>>> means that plan revalidation doesn't work, choose_custom_plan()
>>>> always returns false and we get generic plan. Perhaps, we need
>> some
>>>> form
>>>> of GetCachedPlan(), which ignores raw_parse_tree?
>>> 
>>> I don't think you need a new form of GetCachedPlan().  Instead, it
>>> seems that StmtPlanRequiresRevalidation() should be revised.  As I
>> got
>>> from comments and the d8b2fcc9d4 commit message, the primary goal
>> was
>>> to skip revalidation of utility statements.  Skipping revalidation
>> was
>>> a positive side effect, as long as we didn't support custom plans
>> for
>>> them anyway.  But as you're going to change this,
>>> StmtPlanRequiresRevalidation() needs to be revised.
>>> 
>> 
>> Thanks for feedback.
>> 
>> I've modifed StmtPlanRequiresRevalidation() so that it looks on
>> queries
>> command type.
>> Not sure if it's enough or I have to recreate something more similar
>> to
>> stmt_requires_parse_analysis()
>> logic. I was wondering if this can lead to triggering plan
>> revalidation
>> in RevalidateCachedQuery().
>> I suppose not - as we plan in executor (so shouldn't catch userid
>> change
>> or see some changes in
>> related objects. Revalidation would kill our plan, destroying
>> resultDesc.
>> 
>> Also while looking at this, fixed processing of instead of rules
>> (which
>> would lead to NULL execution_state).
>> --
> 
> there are lot of fails found by tester
> 
> Please, can you check it?

Hi. Sorry for late response - we had holidays here and later there was 
some urgent work from 2024 :)

Do you speak about failures on check-world?
It seems

commit a8ccf4e93a7eeaae66007bbf78cf9183ceb1b371
Author: Richard Guo <rguo@postgresql.org>
Date:   Tue Nov 26 09:25:18 2024 +0900

     Reordering DISTINCT keys to match input path's pathkeys

added new GUC enable_distinct_reordering and this caused test failures.
I've rebased patch on master. Tests pass here.
-- 
Best regards,
Alexander Pyhalov,
Postgres Professional