On Thu, Mar 12, 2020 at 10:19 AM imai.yoshikazu@fujitsu.com
<imai.yoshikazu@fujitsu.com> wrote:
>
> I'll summary code review of this thread.
Thanks for the summary! I just have some minor comments
> [Performance]
>
> If track_planning is not enabled, performance will drop 0.2-0.6% which can be
> ignored. If track_planning is enabled, performance will drop 0-2.2%. 2.2% is a
> bit large but I think it is still acceptable because people using this feature
> might take account that some overhead will happen for additional calling of a
> gettime function.
>
>
https://www.postgresql.org/message-id/CY4PR20MB12227E5CE199FFBB90C68A13BCB40%40CY4PR20MB1222.namprd20.prod.outlook.com
>
> [Values in each row]
>
> * bufusage still only counts the buffer usage during executor.
>
> Now we have the ability to count the buffer usage during planner but we keep
> the bufusage count the buffer usage during executor for now.
The bufusage should reflect the sum of planning and execution usage if
track_planning is enabled. Did I miss something there?
> [Coding]
>
> * We add Assert in pg_plan_query so that query_text will not be NULL when
> executing planner.
>
> There's no case query_text will be NULL with current sources. It is not
> ensured there will be any case query_text will be possibly NULL in the
> future though. Some considerations are needed by other people about this.
There's at least the current version of IVM patchset that lacks the
querytext. Looking at various extensions, I see that pg_background
and pglogical call pg_plan_query internally but shouldn't have any
issue providing the query text. But there's also citus extension,
which don't keep around the query string at least when distributing
plans, which makes sense since it's of no use and they're heavily
modifying the original Query. I think that citus folks opinion on the
subject would be useful, so I'm Cc-ing Marco.
