Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1gheHi-000134-5k for pgsql-bugs@arkaria.postgresql.org; Thu, 10 Jan 2019 17:38:26 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1gheHg-0007sx-NK for pgsql-bugs@arkaria.postgresql.org; Thu, 10 Jan 2019 17:38:24 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1gheHg-0007qY-Gq for pgsql-bugs@lists.postgresql.org; Thu, 10 Jan 2019 17:38:24 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1gheHd-0007cT-Tm for pgsql-bugs@lists.postgresql.org; Thu, 10 Jan 2019 17:38:24 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.14.4/8.14.4) with ESMTP id x0AHcDiw002888; Thu, 10 Jan 2019 12:38:13 -0500 From: Tom Lane To: Robert Haas cc: Thomas Munro , David Rowley , Bartosz Polnik , PostgreSQL mailing lists Subject: Re: BUG #15577: Query returns different results when executed multiple times In-reply-to: <5725.1547090985@sss.pgh.pa.us> References: <15577-ca61ab18904af852@postgresql.org> <18883.1547071723@sss.pgh.pa.us> <5725.1547090985@sss.pgh.pa.us> Comments: In-reply-to Tom Lane message dated "Wed, 09 Jan 2019 22:29:45 -0500" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <2792.1547141744.0@sss.pgh.pa.us> Date: Thu, 10 Jan 2019 12:38:13 -0500 Message-ID: <2887.1547141893@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <2792.1547141744.1@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 ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="remove-NLP-slot-sharing-poc.patch"; charset="us-ascii" Content-ID: <2792.1547141744.2@sss.pgh.pa.us> Content-Description: remove-NLP-slot-sharing-poc.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimiz= er/plan/createplan.c index 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 =3D assign_nestloop_param_var(root, var); ! /* Is this param already listed in root->curOuterParams? */ foreach(lc, root->curOuterParams) { nlp =3D (NestLoopParam *) lfirst(lc); ! if (nlp->paramno =3D=3D param->paramid) { ! Assert(equal(var, nlp->paramval)); ! /* Present, so we can just return the Param */ return (Node *) param; } } ! /* No, so add it */ nlp =3D makeNode(NestLoopParam); nlp->paramno =3D param->paramid; nlp->paramval =3D 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 =3D (NestLoopParam *) lfirst(lc); ! if (equal(var, nlp->paramval)) { ! /* Yes, so just make a Param referencing this NLP's slot */ ! param =3D makeNode(Param); ! param->paramkind =3D PARAM_EXEC; ! param->paramid =3D nlp->paramno; ! param->paramtype =3D var->vartype; ! param->paramtypmod =3D var->vartypmod; ! param->paramcollid =3D var->varcollid; ! param->location =3D var->location; return (Node *) param; } } ! /* No, so assign a PARAM_EXEC slot for a new NLP */ ! param =3D SS_make_initplan_output_param(root, ! var->vartype, ! var->vartypmod, ! var->varcollid); ! param->location =3D var->location; ! /* Add it to the list of NLPs */ nlp =3D makeNode(NestLoopParam); nlp->paramno =3D param->paramid; nlp->paramval =3D var; *************** replace_nestloop_params_mutator(Node *no *** 4423,4442 **** root); return (Node *) newphv; } ! /* Create a Param representing the PlaceHolderVar */ ! param =3D assign_nestloop_param_placeholdervar(root, phv); ! /* Is this param already listed in root->curOuterParams? */ foreach(lc, root->curOuterParams) { nlp =3D (NestLoopParam *) lfirst(lc); ! if (nlp->paramno =3D=3D param->paramid) { ! Assert(equal(phv, nlp->paramval)); ! /* Present, so we can just return the Param */ return (Node *) param; } } ! /* No, so add it */ nlp =3D makeNode(NestLoopParam); nlp->paramno =3D param->paramid; nlp->paramval =3D (Var *) phv; --- 4433,4461 ---- root); return (Node *) newphv; } ! /* Is this PHV already listed in root->curOuterParams? */ foreach(lc, root->curOuterParams) { nlp =3D (NestLoopParam *) lfirst(lc); ! if (equal(phv, nlp->paramval)) { ! /* Yes, so just make a Param referencing this NLP's slot */ ! param =3D makeNode(Param); ! param->paramkind =3D PARAM_EXEC; ! param->paramid =3D nlp->paramno; ! param->paramtype =3D exprType((Node *) phv->phexpr); ! param->paramtypmod =3D exprTypmod((Node *) phv->phexpr); ! param->paramcollid =3D exprCollation((Node *) phv->phexpr); ! param->location =3D -1; return (Node *) param; } } ! /* No, so assign a PARAM_EXEC slot for a new NLP */ ! param =3D 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 =3D makeNode(NestLoopParam); nlp->paramno =3D param->paramid; nlp->paramval =3D (Var *) phv; ------- =_aaaaaaaaaa0--