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 1qEyHe-0000A9-PO for pgsql-bugs@arkaria.postgresql.org; Thu, 29 Jun 2023 20:30:30 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1qEyHd-0003Eo-Jw for pgsql-bugs@arkaria.postgresql.org; Thu, 29 Jun 2023 20:30:29 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1qEyHd-0003Ec-BE for pgsql-bugs@lists.postgresql.org; Thu, 29 Jun 2023 20:30:29 +0000 Received: from relay7-d.mail.gandi.net ([2001:4b98:dc4:8::227]) by magus.postgresql.org with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1qEyHa-001EQn-71 for pgsql-bugs@lists.postgresql.org; Thu, 29 Jun 2023 20:30:28 +0000 Received: by mail.gandi.net (Postfix) with ESMTPSA id 80A6E20008; Thu, 29 Jun 2023 20:30:22 +0000 (UTC) Content-Type: multipart/alternative; boundary="------------UbfT5prahR0YVPGBlxyyg3qK" Message-ID: <326723ee-ee05-35bf-06fd-e4f3afc61654@dunslane.net> Date: Thu, 29 Jun 2023 16:30:21 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 Content-Language: en-US To: Tom Lane Cc: Andres Freund , exclusion@gmail.com, pgsql-bugs@lists.postgresql.org References: <17994-5c7100b51b4790e9@postgresql.org> <1306569.1687978174@sss.pgh.pa.us> <20230628194446.uu47f4vyfyjfl7dd@awork3.anarazel.de> <1314841.1687981948@sss.pgh.pa.us> <20230628205443.6gu5bmcck6t4pgyv@awork3.anarazel.de> <1509402.1688048784@sss.pgh.pa.us> <7ae37634-7626-8977-4d89-20603f0d1dcf@dunslane.net> <1583514.1688066735@sss.pgh.pa.us> From: Andrew Dunstan Subject: Re: BUG #17994: Invalidating relcache corrupts tupDesc inside ExecEvalFieldStoreDeForm() In-Reply-To: <1583514.1688066735@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk This is a multi-part message in MIME format. --------------UbfT5prahR0YVPGBlxyyg3qK Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 2023-06-29 Th 15:25, Tom Lane wrote: > Andrew Dunstan writes: >> On 2023-06-29 Th 10:26, Tom Lane wrote: >>> I'm a bit concerned about added >>> overhead, and about whether the hashtable needs invalidation support. >>> It might be better to key it off (relfilenode, attnum). >> re relfilenode: we don't have it in getmissingattr, so that would >> involve looking it up or whacking around the API. > Yeah, I was afraid of that. > >> re invalidation: that seems to suggest that the missing value could >> change under us. I don't think it can, but if it can then more than just >> this is broken. If not, how would invalidation affect us? > The scenario I'm afraid of is that we cache a missingval for table X, > then X gets dropped, then a new table Y gets created that by bad luck > has the same type OID as X did, then we add a column to Y that > requires a missingval, and now we have an entry in the hashtable that > matches Y but contains the wrong value. Admittedly, it seems very > low probability that this would all happen within the span of one > transaction, so maybe we can get away with ignoring the case. But > if we used relfilenode, we'd have at least a little more protection > because of the tombstone files that prevent immediate re-use of a > relfilenode OID. I'm not sure that it'd be bulletproof even with > relfilenode, though. Maybe we should bite the bullet and provide > invalidation based on a pg_type inval callback. > > Yeah, Robert has just convinced me, so I'll do it like that. It doesn't look too hard. cheers andrew -- Andrew Dunstan EDB:https://www.enterprisedb.com --------------UbfT5prahR0YVPGBlxyyg3qK Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit


On 2023-06-29 Th 15:25, Tom Lane wrote:
Andrew Dunstan <andrew@dunslane.net> writes:
On 2023-06-29 Th 10:26, Tom Lane wrote:
I'm a bit concerned about added
overhead, and about whether the hashtable needs invalidation support.
It might be better to key it off (relfilenode, attnum).

      
re relfilenode: we don't have it in getmissingattr, so that would 
involve looking it up or whacking around the API.
Yeah, I was afraid of that.

re invalidation: that seems to suggest that the missing value could 
change under us. I don't think it can, but if it can then more than just 
this is broken. If not, how would invalidation affect us?
The scenario I'm afraid of is that we cache a missingval for table X,
then X gets dropped, then a new table Y gets created that by bad luck
has the same type OID as X did, then we add a column to Y that
requires a missingval, and now we have an entry in the hashtable that
matches Y but contains the wrong value.  Admittedly, it seems very
low probability that this would all happen within the span of one
transaction, so maybe we can get away with ignoring the case.  But
if we used relfilenode, we'd have at least a little more protection
because of the tombstone files that prevent immediate re-use of a
relfilenode OID.  I'm not sure that it'd be bulletproof even with
relfilenode, though.  Maybe we should bite the bullet and provide
invalidation based on a pg_type inval callback.

			


Yeah, Robert has just convinced me, so I'll do it like that. It doesn't look too hard.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com
--------------UbfT5prahR0YVPGBlxyyg3qK--