On Fri, Feb 10, 2017 at 2:54 PM, Kuntal Ghosh <kuntalghosh.2...@gmail.com> wrote: > On Tue, Feb 7, 2017 at 10:19 AM, Rafia Sabih > <rafia.sa...@enterprisedb.com> wrote: >> Thanks a lot Kuntal for the review, please find attached patch for the >> revised version. > Few comments on the patch: > > There are some spacing issues in the code. For example, > + estate->es_queryString = (char > *)palloc0(strlen(queryDesc->sourceText) + 1); > + /*Estimate space for query text. */ > pgindent might be helpful to track all such changes. > > +#define PARALLEL_KEY_QUERY_TEXT UINT64CONST(0xE000000000000010) > I'm uncomfortable with declaring the same macro in two > files(parallel.c, execParallel.c). My suggestion would be to move > pgstat_report_activity in ParallelQueryMain instead of > ParallelWorkerMain. Then, you can remove the macro definition from > parallel.c. Thoughts? >
Yes, I also don't see any need of defining it in parallel.c. I think she has kept to report it in pg_stat_activity, but I feel that code can also be moved to execParallel.c. Another question is don't we need to set debug_query_string in worker? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
