From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: David Rowley <david.rowley@2ndquadrant.com>
Cc: Bartosz Polnik <bartoszpolnik@gmail.com>
Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #15577: Query returns different results when executed multiple times
Date: Thu, 10 Jan 2019 12:38:13 -0500
Message-ID: <2887.1547141893@sss.pgh.pa.us> (raw)
In-Reply-To: <5725.1547090985@sss.pgh.pa.us>
References: <15577-ca61ab18904af852@postgresql.org>
<CAKJS1f8kLijpf-EKHLL7yJ2KJy9-FWNQfXBuNtToVTPkW_Ue+w@mail.gmail.com>
<CAM37Zev2xUezKc0XZFPfi=BtH+of+fqTAYUCJJuf+4Pc6u4G7g@mail.gmail.com>
<CAEepm=0yv2qNDM_JfiakqQO3EqYNhQZjNCAX=fK8Cuhz6_RO4Q@mail.gmail.com>
<CAM37ZetJ-R5De+aTfZV8+BE_KzqbajF3gjv_t1DGsS853aDc0Q@mail.gmail.com>
<CAEepm=15_MWhhp-Pg6rW1whuXrBTJ-gM=w+2t-Qk-VQHgniG0w@mail.gmail.com>
<CAM37ZevSHt0pdKhZ_Bu7nDoMMWSDsJaaPUPR_ApowoFida4fcA@mail.gmail.com>
<CAEepm=1QU7HpJSGhBkfFwKC_wW6fCTADeHymA7ngzszJRM39bw@mail.gmail.com>
<CAM37Zeu1gv=PswA-C4WeCN13KrrXcD9s1mTdaTuVTKjV=aLoiA@mail.gmail.com>
<CAEepm=34FwV5W3qoz29_bS1uodKkN1wS-P_9V429JUjjGKwopg@mail.gmail.com>
<CAM37ZeuWGV_58O8MeH0TfqBiK0JUONPhHs2=YM2MR61K2E=v9Q@mail.gmail.com>
<CAKJS1f8OZteLVbmKyoTuC7wY=fzZ+jW=gFV=0NoXS0AvfbOZ2g@mail.gmail.com>
<CAEepm=2MYvOvsA7YY-9kU0JT-2Bn51N-YjGq1mDi4GOeR=0nJA@mail.gmail.com>
<CAEepm=3s70u7-E9aSdsSq+_S_cm2GnT4Tk3_vC81PDxe=hCFRw@mail.gmail.com>
<CAEepm=3q=22Stv6QTR+ijWorWYMFRPC=T=hM9i! JPMxgpTOE55A@mail.gmail.com>
<CAM37Zeshup0OkzJ2z=8_jBPrv-=dQZvs+v0HDmKnFf4wLn7jzQ@mail.gmail.com>
<CAEepm=0eviDVN2n=Dr7FegnJ69w3Z+EaB=CBV5Y9HXLN_HwXKQ@mail.gmail.com>
<CAEepm=2UuWkK-tUZMK=SqJ8Qn3jX0jTpE=wk5KKSSJB0gYzSXQ@mail.gmail.com>
<CAKJS1f8zFL8539hpy_gBV3JOcNRqhb1wVVhuVkp_H_E_qJU_iw@mail.gmail.com>
<CAEepm=0ZqJp2nXRZf8e9ys6Utyu96QVy0=L8Q=-2UoDxwY=smA@mail.gmail.com>
<18883.1547071723@sss.pgh.pa.us>
<CA+TgmoZEeHpKYdbksgDXTpE2o=AXOfX08NudPcz_fHYiu84gUg@mail.gmail.com>
<5725.1547090985@sss.pgh.pa.us>
I wrote:
> Well, if we posit that the problems are limited to false Param-sharing,
> my inclination is to fix it by simplifying rather than adding complexity.
> To wit, let's just assign a new PARAM_EXEC slot for every NestLoopParam
> and never try to share them. The argument that sharing them would work
> for NLPs representing the same Var hinged on the assumption that nested
> NestLoops would necessarily execute "in sync", which we now see is false
> at least when Gathers are in between.
Here's a POC patch for that. It's seriously ugly, but it passes
check-world and I can no longer reproduce the problem using
mockup_bug15577_v2.sql.
At this point assign_nestloop_param_var and
assign_nestloop_param_placeholdervar are dead code, and there's a bunch
of incorrect comments in subselect.c, and I really need to refactor
the division of labor between createplan.c and subselect.c (for one
thing, this is an abuse of the documented purpose of
SS_make_initplan_output_param). But functionally I think it does the
right thing. Please test and verify that you no longer see the race
condition.
regards, tom lane
Attachments:remove-NLP-slot-sharing-poc.patch (text/x-diff, 3.8K) downloaddiff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.cindex 066685c..6ce5938 100644
*** a/src/backend/optimizer/plan/createplan.c
--- b/src/backend/optimizer/plan/createplan.c
*************** replace_nestloop_params_mutator(Node *no
*** 4360,4379 ****
/* If not to be replaced, we can just return the Var unmodified */
if (!bms_is_member(var->varno, root->curOuterRels))
return node;
! /* Create a Param representing the Var */
! param = assign_nestloop_param_var(root, var);
! /* Is this param already listed in root->curOuterParams? */
foreach(lc, root->curOuterParams)
{
nlp = (NestLoopParam *) lfirst(lc);
! if (nlp->paramno == param->paramid)
{
! Assert(equal(var, nlp->paramval));
! /* Present, so we can just return the Param */
return (Node *) param;
}
}
! /* No, so add it */
nlp = makeNode(NestLoopParam);
nlp->paramno = param->paramid;
nlp->paramval = var;
--- 4360,4389 ----
/* If not to be replaced, we can just return the Var unmodified */
if (!bms_is_member(var->varno, root->curOuterRels))
return node;
! /* Is this Var already listed in root->curOuterParams? */
foreach(lc, root->curOuterParams)
{
nlp = (NestLoopParam *) lfirst(lc);
! if (equal(var, nlp->paramval))
{
! /* Yes, so just make a Param referencing this NLP's slot */
! param = makeNode(Param);
! param->paramkind = PARAM_EXEC;
! param->paramid = nlp->paramno;
! param->paramtype = var->vartype;
! param->paramtypmod = var->vartypmod;
! param->paramcollid = var->varcollid;
! param->location = var->location;
return (Node *) param;
}
}
! /* No, so assign a PARAM_EXEC slot for a new NLP */
! param = SS_make_initplan_output_param(root,
! var->vartype,
! var->vartypmod,
! var->varcollid);
! param->location = var->location;
! /* Add it to the list of NLPs */
nlp = makeNode(NestLoopParam);
nlp->paramno = param->paramid;
nlp->paramval = var;
*************** replace_nestloop_params_mutator(Node *no
*** 4423,4442 ****
root);
return (Node *) newphv;
}
! /* Create a Param representing the PlaceHolderVar */
! param = assign_nestloop_param_placeholdervar(root, phv);
! /* Is this param already listed in root->curOuterParams? */
foreach(lc, root->curOuterParams)
{
nlp = (NestLoopParam *) lfirst(lc);
! if (nlp->paramno == param->paramid)
{
! Assert(equal(phv, nlp->paramval));
! /* Present, so we can just return the Param */
return (Node *) param;
}
}
! /* No, so add it */
nlp = makeNode(NestLoopParam);
nlp->paramno = param->paramid;
nlp->paramval = (Var *) phv;
--- 4433,4461 ----
root);
return (Node *) newphv;
}
! /* Is this PHV already listed in root->curOuterParams? */
foreach(lc, root->curOuterParams)
{
nlp = (NestLoopParam *) lfirst(lc);
! if (equal(phv, nlp->paramval))
{
! /* Yes, so just make a Param referencing this NLP's slot */
! param = makeNode(Param);
! param->paramkind = PARAM_EXEC;
! param->paramid = nlp->paramno;
! param->paramtype = exprType((Node *) phv->phexpr);
! param->paramtypmod = exprTypmod((Node *) phv->phexpr);
! param->paramcollid = exprCollation((Node *) phv->phexpr);
! param->location = -1;
return (Node *) param;
}
}
! /* No, so assign a PARAM_EXEC slot for a new NLP */
! param = SS_make_initplan_output_param(root,
! exprType((Node *) phv->phexpr),
! exprTypmod((Node *) phv->phexpr),
! exprCollation((Node *) phv->phexpr));
! /* Add it to the list of NLPs */
nlp = makeNode(NestLoopParam);
nlp->paramno = param->paramid;
nlp->paramval = (Var *) phv;
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, robertmhaas@gmail.com, thomas.munro@enterprisedb.com, david.rowley@2ndquadrant.com, bartoszpolnik@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15577: Query returns different results when executed multiple times
In-Reply-To: <2887.1547141893@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox