Hi, + * EXPLAIN ANALYZE CREATE TABLE AS or REFRESH MATERIALIZED VIEW + * WITH NO DATA is weird.
Maybe it is clearer to spell out WITH NO DATA for both statements, instead of sharing it. - if (!stmt->skipData) + if (!stmt->skipData && !explainInfo) ... + else if (explainInfo) It would be cleaner to put the 'if (explainInfo)' as the first check. That way, the check for skipData can be simplified. Cheers On Sun, Mar 7, 2021 at 1:34 AM Bharath Rupireddy < bharath.rupireddyforpostg...@gmail.com> wrote: > On Sun, Mar 7, 2021 at 12:13 PM Japin Li <japi...@hotmail.com> wrote: > > > > On Sun, 07 Mar 2021 at 14:25, Bharath Rupireddy < > bharath.rupireddyforpostg...@gmail.com> wrote: > > > On Sun, Mar 7, 2021 at 11:49 AM Japin Li <japi...@hotmail.com> wrote: > > >> > > >> On Fri, 05 Mar 2021 at 19:48, Bharath Rupireddy < > bharath.rupireddyforpostg...@gmail.com> wrote: > > >> > Attaching v5 patch set for further review. > > >> > > > >> > > >> The v5 patch looks good to me, if there is no objection, I'll change > the > > >> cf status to "Ready for Committer" in few days. > > > > > > Thanks for the review. > > > > > > As I mentioned upthread, I have 2 open points: > > > 1) In the patch I have added a new mat view info parameter to > > > ExplainOneQuery(), do we also need to add it to > > > ExplainOneQuery_hook_type? IMO, we should not (for now), because this > > > would create a backward compatibility issue. > > > > Sorry, I do not know how PostgreSQL handle the backward compatibility > issue. > > Is there a guideline? > > I'm not aware of any guidelines as such, but we usually avoid any > changes to existing API, adding/making changes to system catalogs and > so on. > > > > 2) Do we document (under respective command pages or somewhere else) > > > that we allow explain/explain analyze for a command? > > > > > > > IMO, we can add a new page to list the commands that can be > explain/explain analyze, > > since it's clear for users. > > We are listing all the supported commands in explain.sgml, so added > the CREATE MATERIALIZED VIEW(it's missing even though it's supported > prior to this patch) and REFRESH MATERIALIZED VIEW there. > > Attaching v6 patch set. Please have a look. > > With Regards, > Bharath Rupireddy. > EnterpriseDB: http://www.enterprisedb.com >
