Hi,
For v3_0003-reloption-parallel_dml-src.patch :
+ * Check if parallel_dml_enabled is enabled for the target table,
+ * if not, skip the safety checks and return PARALLEL_UNSAFE.
Looks like the return value is true / false. So the above comment should be adjusted.
+ if (!RelationGetParallelDML(rel, true))
+ {
+ table_close(rel, NoLock);
+ return false;
+ }
+
+ table_close(rel, NoLock);
Since the rel would always be closed, it seems the return value from RelationGetParallelDML() can be assigned to a variable, followed by call to table_close(), then the return statement.
Cheers
On Mon, Feb 1, 2021 at 4:02 PM Hou, Zhijie <houzj.fnst@cn.fujitsu.com> wrote:
>
> Attatching v2 patch which addressed the comments above.
>
> Some further refactor:
>
> Introducing a new function is_parallel_possible_for_modify() which decide whether to do safety check.
>
> IMO, It seems more readable to extract all the check that we can do before the safety-check and put them
> in the new function.
>
> Please consider it for further review.
>
I've updated your v2 patches and altered some comments and
documentation changes (but made no code changes) - please compare
against your v2 patches, and see whether you agree with the changes to
the wording.
In the documentation, you will also notice that in your V2 patch, it
says that the "parallel_dml_enabled" table option defaults to false.
As it actually defaults to true, I changed that in the documentation
too.
Regards,
Greg Nancarrow
Fujitsu Australia