Re: BUG #15577: Query returns different results when executed multiple times

pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed

From: Tom Lane <tgl@sss.pgh.pa.us>
To: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: Robert Haas <robertmhaas@gmail.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 14:51:33 -0500
Message-ID: <13255.1547149893@sss.pgh.pa.us> (raw)
In-Reply-To: <CAEepm=1gpbnPQdWEA5TSKwnnAQ0gySTr0+XN+L14dmw2zLSjpA@mail.gmail.com>
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>
	<CAM37Zeshup0OkzJ2z=8_jBPrv-=dQZvs+v0HDm! KnFf4wLn7jzQ@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>
	<2887.1547141893@sss.pgh.pa.us>
	<CAEepm=1gpbnPQdWEA5TSKwnnAQ0gySTr0+XN+L14dmw2zLSjpA@mail.gmail.com>

Thomas Munro <thomas.munro@enterprisedb.com> writes:
> On Fri, Jan 11, 2019 at 6:38 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 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.

> I no longer see it here.

Thanks for checking!

I'm having some difficulty choosing what to do refactoring-wise.
There are a couple of conflicting considerations:

* Currently, subselect.c is in charge of assigning PARAM_EXEC slots;
in particular, nothing else touches root->glob->paramExecTypes.
I'm kind of loath to give that up.

* On the other hand, root->curOuterParams is currently only manipulated
by createplan.c, and if we could keep that as a local data structure,
that'd be nice too.

* However, if we stick to both of those constraints then we're forced
into more or less what the POC patch does.  We could provide another
subselect.c function, say SS_make_nestloop_param, which'd just wrap
generate_new_param the same as SS_make_initplan_output_param, but
we still have a pretty weird division of labor IMO.

The fundamental issue here is that it's now going to be the state of the
curOuterParams list that determines whether a new PARAM_EXEC slot is
needed.  Really that list serves the same sort of purpose as the
root->plan_params list, but its entries have very different lifespans than
the entries in plan_params.  So there's a reasonable case to be made that
we should put management of curOuterParams into subselect.c, except that
(a) it's a bit far afield from sub-selects, and (b) I'm not sure how
completely it could be decoupled from createplan.c.

If this were a HEAD-only patch I'd be tempted to do like Alvaro just did
and move all the PARAM_EXEC assignment logic and root->plan_params
and root->curOuterParams manipulation into a new file, say
optimizer/util/paramassign.c.  But that would be a little invasive
for a back-patch.

Thoughts?

			regards, tom lane

view thread (37+ messages)  latest in thread

Message-ID: <13255.1547149893@sss.pgh.pa.us>
Permalink:  ../13255.1547149893@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/13255.1547149893@sss.pgh.pa.us

 ·

reply

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, thomas.munro@enterprisedb.com, robertmhaas@gmail.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: <13255.1547149893@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