Re: ddd

Hi,

On 2017-12-21 10:35:06 -0500, Robert Haas wrote:
> Great subject line!

Had started written the email before finishing testing, forgot to fill
in the blank. mutt annoyingly aborts with an empty subject, hence
smashing one key a couple times.

> On Thu, Dec 21, 2017 at 10:18 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> > If I run the regression tests with force_parallel_mode=on prior to the
> > parallel hash join patch, they pass. If I run them now, they fail
> > inside the parallel hash join tests here:
> >
> > create table wide as select generate_series(1, 2) as id, rpad('',
> > 320000, 'x') as t;
> >
> > I'm guessing that test case would have failed before, too, but we
> > didn't have it. I'll analyze this further in a bit.
>
> I think this is just a poorly-written assertion. currentCommandIdUsed
> is only used to skip redundant increments of the command counter, and
> CommandCounterIncrement() is categorically denied under parallelism
> anyway. Therefore, it's OK for this to happen in parallel mode; we
> just need to be in the leader, not the worker.
>
> Therefore, I proposed the attached patch, which fixes the regression
> test crash for me.

> diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
> index d430e662e6..b37510c24f 100644
> --- a/src/backend/access/transam/xact.c
> +++ b/src/backend/access/transam/xact.c
> @@ -683,12 +683,12 @@ GetCurrentCommandId(bool used)
> if (used)
> {
> /*
> - * Forbid setting currentCommandIdUsed in parallel mode, because we
> - * have no provision for communicating this back to the master. We
> + * Forbid setting currentCommandIdUsed in a parallel worker, because
> + * we have no provision for communicating this back to the master. We
> * could relax this restriction when currentCommandIdUsed was already
> * true at the start of the parallel operation.
> */
> - Assert(CurrentTransactionState->parallelModeLevel == 0);
> + Assert(!IsParallelWorker());
> currentCommandIdUsed = true;
> }
> return currentCommandId;

Yea, that looks reasonable to me.

Greetings,

Andres Freund