Re: Get rid of runtime handling of AlternativeSubPlan?

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Get rid of runtime handling of AlternativeSubPlan?
Date: 2020-09-27 15:59:26
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

David Rowley <dgrowleyml(at)gmail(dot)com> writes:
> On Sun, 27 Sep 2020 at 10:03, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2020-09-27 17:06:13 Re: Fwd: Extending range type operators to cope with elements
Previous Message Daniel Westermann (DWE) 2020-09-27 15:58:14 Wrong example in the bloom documentation