Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1tpVzz-006w3e-Ga for pgsql-committers@arkaria.postgresql.org; Tue, 04 Mar 2025 17:24:07 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1tpVzx-0034YH-HA for pgsql-committers@arkaria.postgresql.org; Tue, 04 Mar 2025 17:24:05 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1tpVzx-0034Wd-4y for pgsql-committers@lists.postgresql.org; Tue, 04 Mar 2025 17:24:05 +0000 Received: from mail-oa1-x2f.google.com ([2001:4860:4864:20::2f]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.96) (envelope-from ) id 1tpVzt-000wmQ-2j for pgsql-committers@lists.postgresql.org; Tue, 04 Mar 2025 17:24:03 +0000 Received: by mail-oa1-x2f.google.com with SMTP id 586e51a60fabf-2c1c9b7bd9aso1013090fac.0 for ; Tue, 04 Mar 2025 09:24:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb.com; s=google; t=1741109041; x=1741713841; darn=lists.postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=f4rIHEbWxgkXkKqxZcNieq2vmVsIGwZvSBdXp1FBolE=; b=JJzCK/sN9tPV1wN3tiub0Becf2DmkZJIXgD6hevszMmR5Ijy+Jz3cI3vDEk3/nAaLN 5gQKRQUbMds4mwn2+nYJP8/76HQDB00InKDU5ZtXSXEyrd+4Q2Gv4v0wXZAmkxl2eF6I qY+JHcIKSb7r+F+sZb8PbaAXH1vOfaOAI+ZPDFUZrUYISyzMQRD8wPoIyHZ/Dt7ndNAC EV7A6KrMWPXCtFKqRJuGZO4z/qqqLZfs6bLj2dTGZqkPfnXJjccwpP8DogepgKOmyNZD vurjBqcqLKW7/p4IFAEzXrQWUjSoVgf0OL7R+iUPZWgwvvB8hSA6a9LP/5ulHE1P76qp w9OQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741109041; x=1741713841; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=f4rIHEbWxgkXkKqxZcNieq2vmVsIGwZvSBdXp1FBolE=; b=n6av3F4iN1ohG/kXFD/5KaLp8fS5Z3HvVHu/GpKQQCFB/zp/6l499G/tGC2os+KBEy 8ZP9PTpN4LHAwiS9pmTO1PISp9toL5t2dcNAGMIaJltRjVvrl1E8jhwR6y81YbsEcMLL uOWFPIgshw53KjiTTxFwLZB5A7sHHMKQE4moYjxZdPzGlS0Rzp2rXSkYCJuGMrpegZWG XbOVM5oeLlA4BGxkud8UAT9dCVN5/vcebgOg5D9ZjTGygfhTFoyPJoxLgPXhgV78ugMB FXuSxVaoOubd4gD3SOpm6VfAyVJOhPpRySV+vSazIH9H/C5PiMSjDxo4XrXRbqC+3Cob U2rg== X-Gm-Message-State: AOJu0Yx+6XuTWBKVV6+ywcuDCKCDwoQTCkyOyDk4gzSEnTDSjycuYzNt 0BAJ5K700vb6L1uUtGFc2yUqPmp3cvCvBfMz12E6UunYsr0/oWZrSOewzJqc0g8ddcsswgKNhfU TGN7oPYLlozZJwcFHL4gQZF9Cf08PkdzoO6nL X-Gm-Gg: ASbGncvFEoKqO/4pv7QKOJEbvnt/U4xq/eTAlgSd5BRtfIpJoxWlVZwswtJfolHnRr+ wlvy6HfunIagQxx16UQZSqPMqy/hU7/btN03j3odWK/bwRlyTBMbqlVKmMgYUVd6rnzwLIESAZ/ s6f57yck8BsWm9MGdQJt7KSlg68A== X-Google-Smtp-Source: AGHT+IFQD5oc2UHBF8M/QDC27MsJSFVHPUqJJ71wbR2iShrBYt94YOhQ6GeUH4KA4OWHCE8vAqZE7SBx8SKVGmjdYGQ= X-Received: by 2002:a05:6870:1595:b0:29d:c624:7c3d with SMTP id 586e51a60fabf-2c1787a3731mr11729257fac.32.1741109040828; Tue, 04 Mar 2025 09:24:00 -0800 (PST) MIME-Version: 1.0 References: <2557074.1740673653@sss.pgh.pa.us> <1789fe14-c19d-4025-9201-0eb8faa0840b@eisentraut.org> In-Reply-To: <1789fe14-c19d-4025-9201-0eb8faa0840b@eisentraut.org> From: Mark Dilger Date: Tue, 4 Mar 2025 09:23:49 -0800 X-Gm-Features: AQ5f1JrHjR-RW4Qz_OEQERAJDwaJ43Camxo-OHqzAJJYwckNmlVax3Nc75ibrK0 Message-ID: Subject: Re: pgsql: Generalize hash and ordering support in amapi To: Peter Eisentraut Cc: pgsql-committers@lists.postgresql.org, Tom Lane Content-Type: multipart/alternative; boundary="000000000000e4ef7c062f8789af" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --000000000000e4ef7c062f8789af Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Mar 4, 2025 at 8:46=E2=80=AFAM Peter Eisentraut wrote: > On 27.02.25 23:17, Mark Dilger wrote: > > The logic in equality_ops_are_compatible() was trusting that equality > > operators found in an opfamily for btree or hash were ok, but not > > trusting operators found in opfamilies of other AMs. Now, after the > > patch, other AMs can be marked as suitable. That's really the core of > > what the flag means: "Can the system trust that equality operators > > found in opfamilies of the AM are well-behaved", or something like > > that. > > Yeah, what might be a good English identifier for that? > > > I also object strongly to the fact that the comments for > > equality_ops_are_compatible and comparison_ops_are_compatible > > were not modified: > > > > * This is trivially true if they are the same operator. Otherwis= e, > > * we look to see if they can be found in the same btree or hash > > opfamily. > > > > * This is trivially true if they are the same operator. Otherwis= e, > > * we look to see if they can be found in the same btree opfamily. > > > > I agree these comments need updating. > > Mark, can you suggest updated wording for those? > > Yes, happily, though I think I already did, in the v21 patch set. Here is the meat of that patch: - * This is trivially true if they are the same operator. Otherwise, - * we look to see if they can be found in the same btree or hash opfamily. - * Either finding allows us to assume that they have compatible notions - * of equality. (The reason we need to do these pushups is that one might - * be a cross-type operator; for instance int24eq vs int4eq.) + * This is trivially true if they are the same operator. Otherwise, we look to + * see if they can be found in the same opfamily of an index AM which + * advertises amcancrosscompare. Either finding allows us to assume that they + * have compatible notions of equality. (The reason we need to do these + * pushups is that one might be a cross-type operator; for instance int24eq vs + * int4eq.) and - * This is trivially true if they are the same operator. Otherwise, - * we look to see if they can be found in the same btree opfamily. - * For example, '<' and '>=3D' ops match if they belong to the same family= . + * This is trivially true if they are the same operator. Otherwise, we look to + * see if they can be found in the same opfamily of an index AM that advertises + * both amcancrosscompare and amcanorder. For example, '<' and '>=3D' ops match + * if they belong to the same family. * - * (This is identical to equality_ops_are_compatible(), except that we - * don't bother to examine hash opclasses.) + * (This is identical to equality_ops_are_compatible(), except that we don't + * bother to examine opclasses for index AMs which cannot do ordering, such as + * the hash index AM.) See v21-0003-Update-syscache-code-comments.patch for the whole thing, including a commit message about why this is needed. --=20 =E2=80=94 Mark Dilger EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company --000000000000e4ef7c062f8789af Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Tue, Mar 4, = 2025 at 8:46=E2=80=AFAM Peter Eisentraut <peter@eisentraut.org> wrote:
On = 27.02.25 23:17, Mark Dilger wrote:
> The logic in equality_ops_are_compatible() was trusting that equality =
> operators found in an opfamily=C2=A0for btree or hash were ok, but not=
> trusting operators found in opfamilies=C2=A0of other AMs.=C2=A0 Now, a= fter the
> patch, other AMs can be marked as suitable.=C2=A0 That's really th= e core of
> what the flag means: =C2=A0"Can the system trust that equality op= erators
> found in opfamilies of the AM are well-behaved", or something lik= e
> that.

Yeah, what might be a good English identifier for that?

>=C2=A0 =C2=A0 =C2=A0I also object strongly to the fact that the comment= s for
>=C2=A0 =C2=A0 =C2=A0equality_ops_are_compatible and comparison_ops_are_= compatible
>=C2=A0 =C2=A0 =C2=A0were not modified:
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0* This is trivially true if they are the sam= e operator.=C2=A0 Otherwise,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0* we look to see if they can be found in the= same btree or hash
>=C2=A0 =C2=A0 =C2=A0opfamily.
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0* This is trivially true if they are the sam= e operator.=C2=A0 Otherwise,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0* we look to see if they can be found in the= same btree opfamily.
>
> I agree these comments need updating.

Mark, can you suggest updated wording for those?


Yes, happily, though I think I alrea= dy did, in the v21 patch set.=C2=A0 Here is the meat of that patch:

- * This is trivially true if they are the same operator.= =C2=A0 Otherwise,
- * we look to see if they can be found in the same bt= ree or hash opfamily.
- * Either finding allows us to assume that they h= ave compatible notions
- * of equality. =C2=A0(The reason we need to do = these pushups is that one might
- * be a cross-type operator; for instan= ce int24eq vs int4eq.)
+ * This is trivially true if they are the same o= perator.=C2=A0 Otherwise, we look to
+ * see if they can be found in the= same opfamily of an index AM which
+ * advertises amcancrosscompare.=C2= =A0 Either finding allows us to assume that they
+ * have compatible not= ions of equality. =C2=A0(The reason we need to do these
+ * pushups is t= hat one might be a cross-type operator; for instance int24eq vs
+ * int4= eq.)

and

- * This is = trivially true if they are the same operator.=C2=A0 Otherwise,
- * we lo= ok to see if they can be found in the same btree opfamily.
- * For examp= le, '<' and '>=3D' ops match if they belong to the sa= me family.
+ * This is trivially true if they are the same operator.=C2= =A0 Otherwise, we look to
+ * see if they can be found in the same opfam= ily of an index AM that advertises
+ * both amcancrosscompare and amcano= rder.=C2=A0 For example, '<' and '>=3D' ops match
= + * if they belong to the same family.
=C2=A0 *
- * (This is identica= l to equality_ops_are_compatible(), except that we
- * don't bother = to examine hash opclasses.)
+ * (This is identical to equality_ops_are_c= ompatible(), except that we don't
+ * bother to examine opclasses fo= r index AMs which cannot do ordering, such as
+ * the hash index AM.)

See=C2=A0v21-0003-Update-syscache-code-comments.= patch for the whole thing, including a commit message about why this is nee= ded.

-- <= br>
=E2=80=94=
Mark Dilger<= /span>
<= span style=3D"color:rgb(0,0,0);font-family:Helvetica;font-size:18px">Enterp= riseDB:=C2=A0http://www.= enterprisedb.com
The Enterprise PostgreSQL Company
--000000000000e4ef7c062f8789af--