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 18:02:00 -0500
Message-ID: <28280.1547161320@sss.pgh.pa.us> (raw)
In-Reply-To: <13255.1547149893@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>
	<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>
	<13255.1547149893@sss.pgh.pa.us>

I wrote:
> I'm having some difficulty choosing what to do refactoring-wise.
> ...
> 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.

Actually, maybe it wouldn't be that bad.  The functions I'd want to
move into a new file are currently mostly static in subselect.c.
Moving them elsewhere (and consequently making them not-static)
would therefore not break any existing API.  The exceptions are:

SS_assign_special_param

We could keep a wrapper by this name in the back branches, and thereby
avoid any API/ABI break for extension code using it (which it seems
somewhat likely that there might be).

SS_make_initplan_output_param

Although this'd just be a one-line wrapper for functionality exported
by the hypothetical new file, it's somewhat closely related to
SS_make_initplan_from_plan.  So keeping it, with its current name,
isn't outlandish.

assign_nestloop_param_var
assign_nestloop_param_placeholdervar

These are going to go away, or else change API substantially, in any
case.  Hopefully there's no extension code using them.

So what I'm now contemplating is moving these existing functions
to a new file:

int assign_param_for_var(PlannerInfo *root, Var *var)
Param *replace_outer_var(PlannerInfo *root, Var *var)
int assign_param_for_placeholdervar(PlannerInfo *root, PlaceHolderVar *phv)
Param *replace_outer_placeholdervar(PlannerInfo *root, PlaceHolderVar *phv)
Param *replace_outer_agg(PlannerInfo *root, Aggref *agg)
Param *replace_outer_grouping(PlannerInfo *root, GroupingFunc *grp)
Param *generate_new_param(PlannerInfo *root, Oid paramtype, int32 paramtypmod,
                          Oid paramcollation)
int assign_special_param(PlannerInfo *root)
	(rename of SS_assign_special_param)

along with some new functions for nestloop parameters (extracted from
createplan.c logic).

assign_param_for_var and assign_param_for_placeholdervar could remain
"static" since they aren't called from anywhere else.  (Actually I
guess they'd end up getting merged back into the replace_outer_
functions, since they'll now have just one caller apiece.)

A variant idea would involve also moving replace_correlation_vars_mutator,
which would allow the individual replace_outer_xxx functions to remain
static.  A different idea is to just move the assign_param_for_xxx
functionality, which'd require breaking replace_outer_agg and
replace_outer_grouping down into two functions apiece, since they
currently combine creation of plan_params items with creation of
the referencing Param nodes.

Anybody have a preference?

			regards, tom lane

view thread (37+ messages)

Message-ID: <28280.1547161320@sss.pgh.pa.us>
Permalink:  ../28280.1547161320@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/28280.1547161320@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: <28280.1547161320@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