On 05/04/2016 06:19, Amit Kapila wrote:
>
> Few more comments:
>
> 1.
> @@ -909,6 +909,17 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } |
> UNLOGGED ] TABLE [ IF NOT EXI
>
> </varlistentry>
>
> <varlistentry>
> + <term><literal>parallel_degree</> (<type>integer</>)</term>
> + <listitem>
> +
> <para>
> + Sets the degree of parallelism for an individual relation. The
> requested
> + number of workers will be
> limited by <xref
> + linkend="guc-max-parallel-degree">.
> + </para>
> + </listitem>
> + </varlistentry>
>
> All other parameters in this category are supportted by Alter table
> command as well, so I think this parameter should also be supported by
> Alter Table command (for both SET and RESET variants).
>
I don't quite understand. With the patch you can use parallel_degree in
either CREATE or ALTER table (SET and RESET) statements. Considering
documentation, the list of storage parameters only appears in
create_table.sgml, alter_table.sgml pointing to it.
In alter_table.sgml, I didn't comment the lock level needed to modify
parallel_degree since it requires an access exclusive lock for now.
While thinking about it, I think it's safe to use a share update
exclusive lock but I may be wrong. What do you think?
> 2.
> +"Number of parallel processes per executor node wanted for this relation.",
>
> How about
> Number of parallel processes that can be used for this relation per
> executor node.
>
I just rephrased what was used for the max_parallel_degree GUC, which is:
"Sets the maximum number of parallel processes per executor node."
I find your version better once again, but should we keep some
consistency between them or it's not important?
> 3.
> -if (rel->pages < parallel_threshold && rel->reloptkind == RELOPT_BASEREL)
> +if (rel->pages <
> parallel_threshold && rel->rel_parallel_degree == -1 &&
> +rel->reloptkind == RELOPT_BASEREL)
>
> A. Second line should be indented with the begin of first line after
> bracket '(' which means with rel->pages. Refer multiline condition in
> near by code. Or you can run pgindent.
I ran pgindent, fixed.
> B. The comment above this condition needs slight adjustment as per new
> condition.
>
Also fixed.
> 4.
> +intparallel_degree; /* max number of parallel worker */
> } StdRdOptions;
>
> Typo in comments
> /worker/workers
>
fixed.
I'll send an updated patch when I'll know what to do about the first two
points.
--
Julien Rouhaud
http://dalibo.com - http://dalibo.org
