Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kMZ5M-0003Xl-C0 for pgsql-hackers@arkaria.postgresql.org; Sun, 27 Sep 2020 15:59:36 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1kMZ5L-0002jW-Aa for pgsql-hackers@arkaria.postgresql.org; Sun, 27 Sep 2020 15:59:35 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kMZ5L-0002jO-3z for pgsql-hackers@lists.postgresql.org; Sun, 27 Sep 2020 15:59:35 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kMZ5E-0002My-Vm for pgsql-hackers@lists.postgresql.org; Sun, 27 Sep 2020 15:59:34 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 08RFxQbt120053; Sun, 27 Sep 2020 11:59:26 -0400 From: Tom Lane To: David Rowley cc: PostgreSQL Developers Subject: Re: Get rid of runtime handling of AlternativeSubPlan? In-reply-to: References: <1992952.1592785225@sss.pgh.pa.us> <3740825.1598743596@sss.pgh.pa.us> <3968496.1598888506@sss.pgh.pa.us> <4117348.1598894531@sss.pgh.pa.us> <63837.1601154207@sss.pgh.pa.us> Comments: In-reply-to David Rowley message dated "Sun, 27 Sep 2020 21:48:20 +1300" MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-ID: <120051.1601222366.1@sss.pgh.pa.us> Date: Sun, 27 Sep 2020 11:59:26 -0400 Message-ID: <120052.1601222366@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk David Rowley writes: > On Sun, 27 Sep 2020 at 10:03, Tom Lane 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