Hello, On Sun, Sep 8, 2019 at 11:45 AM Imai Yoshikazu <yoshikazu_i...@live.jp> wrote: > > I also saw the codes and have one comment.
Thanks for looking at this patch! > [0002 patch] > In pgss_planner_hook: > > + /* calc differences of buffer counters. */ > + bufusage = compute_buffer_counters(bufusage_start, > pgBufferUsage); > + > + /* > + * we only store planning duration, query text has been > initialized > + * during previous pgss_post_parse_analyze as it not > available inside > + * pgss_planner_hook. > + */ > + pgss_store(query_text, > > Do we need to calculate bufusage in here? > We only store planning duration in the following pgss_store. Good point! Postgres can definitely access some buffers while planning a query (the most obvious example would be get_actual_variable_range()), but as far as I can tell those were previously not accounted for with pg_stat_statements as queryDesc->totaltime->bufusage is only accumulating buffer usage in the executor, and indeed current patch also ignore such computed counters. I think it would be better to keep this bufusage calculation during planning and fix pgss_store() to process them, but this would add slightly more overhead. > We only store planning duration in the following pgss_store. > > -- > Yoshikazu Imai
