From: Tom Lane <tgl@sss.pgh.pa.us>
To: Dmitry Dolgov <9erthalion6@gmail.com>
cc: exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17908: plpython_to_hstore() crashes with a non-dict argument
In-reply-to: <20230427082727.4na67kulnnlxkzq3@ddolgov.remote.csb>
References: <17908-3f19a125d56a11d6@postgresql.org>
 <20230427082727.4na67kulnnlxkzq3@ddolgov.remote.csb>
Comments: In-reply-to Dmitry Dolgov <9erthalion6@gmail.com>
	message dated "Thu, 27 Apr 2023 10:27:27 +0200"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1113608.1682606789.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Thu, 27 Apr 2023 10:46:29 -0400
Message-ID: <1113609.1682606789@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1113609.1682606789%40sss.pgh.pa.us>
Precedence: bulk

Dmitry Dolgov <9erthalion6@gmail.com> writes:
>> On Tue, Apr 25, 2023 at 01:00:02PM +0000, PG Bug reporting form wrote:
>> As noted in https://bugs.python.org/issue5945, the behaviour of
>> PyMapping_Check() changed in Python 3, so it can't be used anymore to
>> differentiate real dictionaries from strings or alike.

> Thanks for finding this! That's indeed very annoying.

Yuck.

> After a quick
> investigation looks like a proposed solution from [1] is to use
> PyType_HasFeature with Py_TPFLAGS_MAPPING:
>     -       if (!PyMapping_Check(dict))
>     +       dict_type =3D Py_TYPE(dict);
>     +       if (!PyType_HasFeature(dict_type, Py_TPFLAGS_MAPPING))
> But this flag is available only starting from Python 3.10, so not
> backward compatible :(

Yeah, doesn't sound hugely workable; I don't think we can require 3.10
yet.  The issue5945 discussion suggested

>>> And recommend `PyMapping_Check() && !PySequence_Check()` for true mapp=
ing test?

I haven't verified this but will go test it shortly.  If it's true,
it would explain why our other uses of PyMapping_Check haven't
failed, because (whether by chance or not I dunno) they are not
reached if PySequence_Check is true.

			regards, tom lane