pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: David Rowley <dgrowleyml@gmail.com>
Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Get rid of runtime handling of AlternativeSubPlan?
Date: Sun, 27 Sep 2020 11:59:26 -0400
Message-ID: <120052.1601222366@sss.pgh.pa.us> (raw)
In-Reply-To: <CAApHDvp8aMtBPhoSEc2KeVypKOKvbV1FJ6s1EirxvkNCfR9rrQ@mail.gmail.com>
References: <1992952.1592785225@sss.pgh.pa.us>
	<3740825.1598743596@sss.pgh.pa.us>
	<CAApHDvqhcEab0aJ29C+yznHiUHNqoKvTOL+sz0p3a1SNon4ntg@mail.gmail.com>
	<3968496.1598888506@sss.pgh.pa.us>
	<4117348.1598894531@sss.pgh.pa.us>
	<CAApHDvqeh8JEqMjpCFTgHD_zu2S03nOVh2srejd+sNLza8M+mg@mail.gmail.com>
	<63837.1601154207@sss.pgh.pa.us>
	<CAApHDvp8aMtBPhoSEc2KeVypKOKvbV1FJ6s1EirxvkNCfR9rrQ@mail.gmail.com>

David Rowley <dgrowleyml@gmail.com> writes:
> On Sun, 27 Sep 2020 at 10:03, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> And if it's true, why would we change all the call sites
>> rather than improving the pg_list.h macros?

> Maybe we should. Despite the non-NIL check and length check in
> list_head(), list_second_cell(), list_third_cell() functions, the
> corresponding macro will crash anyway if those functions were to
> return NULL.

Hm, good point.

> We might as well just use list_nth_cell() to get the
> ListCell without any checks to see if the cell exists.  I  can go off
> and fix those separately. I attached a 0004 patch to help explain what
> I'm talking about.

Yeah, that should be dealt with separately.

>> It seemed to me that dealing with the general case would make
>> fix_alternative_subplan() noticeably more complex and less obviously
>> correct.  I might be wrong though; what specific coding did you have in
>> mind?

> I had thought something like 0003 (attached). It's a net reduction of
> 3 entire lines, including the removal of the comment that explained
> that there's always two in the list.

Meh.  This seems to prove my point, as it's in fact wrong; you are only
nulling out the discarded subplans-list entry in one of the two cases.
Once you fix that it's not really shorter anymore, nor clearer.  Still,
I suppose there's some value in removing the assumption about exactly
two subplans.

I'll fix up fix_alternative_subplan and push this.  I think the other
topics should be raised in separate threads.

Thanks for reviewing!

			regards, tom lane





view thread (12+ messages)

Message-ID: <120052.1601222366@sss.pgh.pa.us>
Permalink:  ../120052.1601222366@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/120052.1601222366@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-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, dgrowleyml@gmail.com, pgsql-hackers@lists.postgresql.org
  Subject: Re: Get rid of runtime handling of AlternativeSubPlan?
  In-Reply-To: <120052.1601222366@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