Hi Imai-san,
On Thu, Mar 12, 2020 at 05:28:38AM +0000, imai.yoshikazu@fujitsu.com wrote:
> Hi Julien,
>
> On Mon, Mar 9, 2020 at 10:32 AM, Julien Rouhaud wrote:
> > On Thu, Mar 05, 2020 at 01:26:19PM -0700, legrand legrand wrote:
> > > Please consider PG13 shortest path ;o)
> > >
> > > My one is parse->queryId != UINT64CONST(0) in pgss_planner_hook().
> > > It fixes IVM problem (verified),
> > > and keep CTAS equal to pgss without planning counters (verified too).
> >
> > I still disagree that hiding this problem is the right fix, but since no one
> > objected here's a v5 with that behavior. Hopefully this will be fixed in v14.
>
> Is there any case that query_text will be NULL when executing pg_plan_query?
With current sources, there are no cases where the query text isn't provided
AFAICS.
> If query_text will be NULL, we need to add codes to avoid errors in
> pgss_store like as current patch. If query_text will not be NULL, we should
> add Assert in pg_plan_query so that other developers can notice that they
> would not mistakenly set query_text as NULL even without using pgss_planning
> counter.
I totally agree. I already had such assert locally, and regression tests pass
without any problem. I'm attaching a v6 with that extra assert. If the
first patch is committed, it'll now be a requirement to provide it. Or if
people think it's not, it'll make sure that we'll discuss it.