Hi, For v12-0001-Enable-parallel-SELECT-for-INSERT-INTO-.-SELECT.patch : is found from the additional parallel-safety checks, or from the existing parallel-safety checks for SELECT that it currently performs.
existing and 'it currently performs' are redundant. You can omit 'that it currently performs'. Minor. For index_expr_max_parallel_hazard_for_modify(), + if (keycol == 0) + { + /* Found an index expression */ You can check if keycol != 0, continue with the loop. This would save some indent. + if (index_expr_item == NULL) /* shouldn't happen */ + { + elog(WARNING, "too few entries in indexprs list"); I think the warning should be an error since there is assertion ahead of the if statement. + Assert(!isnull); + if (isnull) + { + /* + * This shouldn't ever happen, but if it does, log a WARNING + * and return UNSAFE, rather than erroring out. + */ + elog(WARNING, "null conbin for constraint %u", con->oid); The above should be error as well. Cheers On Wed, Jan 20, 2021 at 5:06 PM Greg Nancarrow <gregn4...@gmail.com> wrote: > Thanks for the feedback. > Posting an updated set of patches. Changes are based on feedback, as > detailed below: > > There's a couple of potential issues currently being looked at: > - locking issues in additional parallel-safety checks? > - apparent uneven work distribution across the parallel workers, for > large insert data > > > [Antonin] > - Fixed bad Assert in PrepareParallelMode() > - Added missing comment to explain use of GetCurrentCommandId() in > PrepareParallelMode() > - Some variable name shortening in a few places > - Created common function for creation of non-parallel and parallel > ModifyTable paths > - Path count variable changed to bool > - Added FIXME comment to dubious code for creating Gather target-list > from ModifyTable subplan > - Fixed check on returningLists to use NIL instead of NULL > > [Amit] > - Moved additional parallel-safety checks (for modify case) into > max_parallel_hazard() > - Removed redundant calls to max_parallel_hazard_test() > - Added Asserts to "should never happen" null-attribute cases (and > added WARNING log missing from one case) > - Added comment for use of NoLock in max_parallel_hazard_for_modify() > > [Vignesh] > - Fixed a couple of typos > - Added a couple of test cases for testing that the same transaction > is used by all parallel workers > > > Regards, > Greg Nancarrow > Fujitsu Australia >