From: | Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com> |
---|---|
To: | Rafia Sabih <rafia(dot)sabih(at)enterprisedb(dot)com> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Passing query string to workers |
Date: | 2017-02-10 09:24:48 |
Message-ID: | CAGz5QCJtL497O4Ls+6BEWGVhRU4BRZw--MYpB3+=RoiLkFJu+w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Feb 7, 2017 at 10:19 AM, Rafia Sabih
<rafia(dot)sabih(at)enterprisedb(dot)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?
And, the value of the macro seems pretty random to me. IMO, it should
be UINT64CONST(0xE000000000000007).
--
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Simon Riggs | 2017-02-10 10:19:58 | Re: Documentation improvements for partitioning |
Previous Message | Simon Riggs | 2017-02-10 08:38:26 | log_autovacuum_min_duration doesn't log VACUUMs |