Re: Index AM API cleanup

I have committed these four patches (squashed into three). I made the error handling change in 0003 that you requested, and also the error handling change in 0002 discussed in an adjacent message. On 12.03.25 16:52, Mark Dilger wrote: On Wed, Mar 12, 2025 at 7:00 AM Peter Eisentraut

Re: Index AM API cleanup

On 12.03.25 17:08, Mark Dilger wrote: On Wed, Mar 12, 2025 at 7:25 AM Tom Lane > wrote: Peter Eisentraut mailto:pe...@eisentraut.org>> writes: > 0002: Add get_opfamily_member_for_cmptype().  This was called > get_opmethod_member() in your patch set, bu

Re: Index AM API cleanup

Hi,, skink/valgrind just started to die during the main regression tests: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2025-04-04%2011%3A01%3A56 The set of commits seem to point to the changes made as part of this thread. ==4163809== VALGRINDERROR-BEGIN ==4163809== Invalid re

Re: Index AM API cleanup

On 04.04.25 14:17, Andres Freund wrote: skink/valgrind just started to die during the main regression tests: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2025-04-04%2011%3A01%3A56 Thanks, fix is on the way. The set of commits seem to point to the changes made as part of th

Re: Index AM API cleanup

On 20.03.25 12:59, Peter Eisentraut wrote: v22-0006-Convert-from-StrategyNumber-to-CompareType.patch This is all that remains now.  I think with a bit more polishing around the edges, some comment updates, etc., this is close to ready. Here is an updated version of this patch. I have left ou

Re: Index AM API cleanup

On Thu, Mar 20, 2025 at 4:59 AM Peter Eisentraut wrote: > Here is a new version of the remaining main patch set. I've made a lot > of changes to reduce the size and scope. > > - In version v21, you had included a bunch of expected files in the > "treeb" module, which wasn't necessary, since the

Re: Index AM API cleanup

On Wed, Mar 12, 2025 at 7:25 AM Tom Lane wrote: > Peter Eisentraut writes: > > 0002: Add get_opfamily_member_for_cmptype(). This was called > > get_opmethod_member() in your patch set, but I think that name wasn't > > quite right. I also removed the opmethod argument, which was rarely > > used

Re: Index AM API cleanup

On Wed, Mar 12, 2025 at 7:00 AM Peter Eisentraut wrote: > And another small patch set extracted from the bigger one, to keep > things moving along: > > 0001: Add get_opfamily_method() in lsyscache.c, from your patch set. > Right, this came from v21-0006-*, with a slight code comment change and o

Re: Index AM API cleanup

Peter Eisentraut writes: > 0002: Add get_opfamily_member_for_cmptype(). This was called > get_opmethod_member() in your patch set, but I think that name wasn't > quite right. I also removed the opmethod argument, which was rarely > used and is somewhat redundant. Hm, that will throw an error

Re: Index AM API cleanup

And another small patch set extracted from the bigger one, to keep things moving along: 0001: Add get_opfamily_method() in lsyscache.c, from your patch set. 0002: Add get_opfamily_member_for_cmptype(). This was called get_opmethod_member() in your patch set, but I think that name wasn't quit

Re: Index AM API cleanup

While studying the patch "[PATCH v21 08/12] Allow non-btree indexes to participate in mergejoin", I figured we could do the sortsupport.c piece much simpler. The underlying issue there is similar to commits 0d2aa4d4937 and c594f1ad2ba: We're just using the btree strategy numbers to pass inform

Re: Index AM API cleanup

On 04.02.25 14:31, Peter Eisentraut wrote: On 03.02.25 12:08, Peter Eisentraut wrote: Yeah, actually this turned out to be unnecessary, because you can easily obtain the AM OID from the passed-in opclass ID.  So I have fixed it that way.  I have committed this whole patch set now with the ment

Re: Index AM API cleanup

On 03.02.25 12:08, Peter Eisentraut wrote: Yeah, actually this turned out to be unnecessary, because you can easily obtain the AM OID from the passed-in opclass ID.  So I have fixed it that way.  I have committed this whole patch set now with the mentioned adjustments.  I'll post a rebased vers

Re: Index AM API cleanup

lso need to pass the opclass to the index AM API functions, so that access methods like gist can use it. Actually, I changed that to pass opfamily and opcintype instead. I think this matches better with the rest of the "Index AM API cleanup" patch set, because it's more common to ha

Re: Index AM API cleanup

> On Jan 24, 2025, at 11:18 PM, Peter Eisentraut wrote: > > I've been working on integrating Mark's "Index AM API cleanup" patch set with > the existing gist strategy number mapping from Paul's application time patch > set. Here is what I'v

Re: Index AM API cleanup

I've been working on integrating Mark's "Index AM API cleanup" patch set with the existing gist strategy number mapping from Paul's application time patch set. Here is what I've come up with. The previously committed patch (v19.1) already changed the gist strat

Re: Index AM API cleanup

On 15.01.25 22:15, Nathan Bossart wrote: On Wed, Jan 15, 2025 at 03:31:12PM +0100, Peter Eisentraut wrote: On 04.12.24 15:49, Peter Eisentraut wrote: Here is a patch set in that direction.  It renames RowCompareType to CompareType and updates the surrounding commentary a bit.  And then I'm chan

Re: Index AM API cleanup

On Wed, Jan 15, 2025 at 03:31:12PM +0100, Peter Eisentraut wrote: > On 04.12.24 15:49, Peter Eisentraut wrote: >> Here is a patch set in that direction.  It renames RowCompareType to >> CompareType and updates the surrounding commentary a bit.  And then I'm >> changing the gist strategy mapping to

Re: Index AM API cleanup

On 04.12.24 15:49, Peter Eisentraut wrote: On 27.11.24 13:57, Peter Eisentraut wrote: I think, however, that we should rename RowCompareType.  Otherwise, it's just going to be confusing forevermore.  I suggest to rename it simply to CompareType. I'm going to try to code up the gist support on

Re: Index AM API cleanup

On 8/26/24 08:10, Mark Dilger wrote: > Paul, it seems what you are doing in v39-0001-Add-stratnum-GiST-support-function.patch is similar to what I am doing in v17-0012-Convert-strategies-to-and-from-row-compare-types.patch. Thank you inviting me to share some thoughts here! The goals of this pa

Re: Index AM API cleanup

On 27.11.24 13:57, Peter Eisentraut wrote: I think, however, that we should rename RowCompareType.  Otherwise, it's just going to be confusing forevermore.  I suggest to rename it simply to CompareType. I'm going to try to code up the gist support on top of this patch set to make sure that it

Re: Index AM API cleanup

> On Nov 27, 2024, at 4:57 AM, Peter Eisentraut wrote: > > On 26.11.24 15:27, Andrew Dunstan wrote: >> On 2024-11-19 Tu 6:26 PM, Mark Dilger wrote: On Nov 16, 2024, at 9:10 AM, Kirill Reshke wrote: Hi! Can we please have a rebased version of this patch series? >>> Sorry for th

Re: Index AM API cleanup

On 26.11.24 15:27, Andrew Dunstan wrote: On 2024-11-19 Tu 6:26 PM, Mark Dilger wrote: On Nov 16, 2024, at 9:10 AM, Kirill Reshke wrote: Hi! Can we please have a rebased version of this patch series? Sorry for the delay, and thanks for your interest.  I will try to get around to rebasing in t

Re: Index AM API cleanup

> On Nov 16, 2024, at 9:10 AM, Kirill Reshke wrote: > > Hi! Can we please have a rebased version of this patch series? Sorry for the delay, and thanks for your interest. I will try to get around to rebasing in the next few days. — Mark Dilger EnterpriseDB: http://www.enterprisedb.com The

Re: Index AM API cleanup

On Thu, 31 Oct 2024 at 15:02, Mark Dilger wrote: > > > > > On Oct 30, 2024, at 12:54 PM, Peter Eisentraut wrote: > > > > So this is the idea. To take a step back, I can see the following > > options: > > > > 1. Require all index AMs to use btree-compatible strategy numbers. > > (Previously rej

Re: Index AM API cleanup

> On Oct 30, 2024, at 12:54 PM, Peter Eisentraut wrote: > > So this is the idea. To take a step back, I can see the following > options: > > 1. Require all index AMs to use btree-compatible strategy numbers. > (Previously rejected, too much upgrading mess.) > > 2. Provide mapping between

Re: Index AM API cleanup

I want to call out one particular aspect that is central to this patch series that needs more broader discussion and agreement. The problem is that btree indexes (and to a lesser extent hash indexes) are hard-coded in various places. This includes various places in the optimizer (see patch v17-0

Re: Index AM API cleanup

On 24.09.24 11:09, Mark Dilger wrote: On Sep 24, 2024, at 10:50 AM, Peter Eisentraut wrote: Next, I have reviewed patches v17-0010-Track-sort-direction-in-SortGroupClause.patch v17-0011-Track-scan-reversals-in-MergeJoin.patch Both of these seem ok and sensible to me. They take the concept

Re: Index AM API cleanup

> On Sep 24, 2024, at 10:50 AM, Peter Eisentraut wrote: > > Next, I have reviewed patches > > v17-0010-Track-sort-direction-in-SortGroupClause.patch > v17-0011-Track-scan-reversals-in-MergeJoin.patch > > Both of these seem ok and sensible to me. > > They take the concept of the "reverse" fl

Re: Index AM API cleanup

Next, I have reviewed patches v17-0010-Track-sort-direction-in-SortGroupClause.patch v17-0011-Track-scan-reversals-in-MergeJoin.patch Both of these seem ok and sensible to me. They take the concept of the "reverse" flag that already exists in the affected code and just apply it more consistent

Re: Index AM API cleanup

On 03.09.24 19:26, Mark Dilger wrote: On Sep 3, 2024, at 9:52 AM, Peter Eisentraut wrote: Here is a cleaned-up version of the v17-0004 patch. I have applied the renaming discussed above. I have also made a wrapper function btgettreeheight() that calls _bt_getrootheight(). That way, if we e

Re: Index AM API cleanup

On Thu, Aug 22, 2024 at 11:28 AM Mark Dilger wrote: > > On Aug 22, 2024, at 1:36 AM, Alexandra Wang > > wrote: > > "make installcheck" for treeb is causing issues on my end. I can > > investigate further if it’s not a problem for others. > > The test module index AMs are not intended for use in

Re: Index AM API cleanup

> On Sep 3, 2024, at 9:52 AM, Peter Eisentraut wrote: > > Here is a cleaned-up version of the v17-0004 patch. I have applied the > renaming discussed above. I have also made a wrapper function > btgettreeheight() that calls _bt_getrootheight(). That way, if we ever want > to change the A

Re: Index AM API cleanup

On 26.08.24 17:10, Mark Dilger wrote: To make a dent, I picked out something that should be mostly harmless: Stop calling directly into _bt_getrootheight() (patch 0004). I think this patch is ok, but I might call the API function amgettreeheight instead of amgetrootheight. The former seems m

Re: Index AM API cleanup

On 2024-08-22 Th 2:28 PM, Mark Dilger wrote: On Aug 22, 2024, at 1:36 AM, Alexandra Wang wrote: I had to make some changes to the first two patches in order to run "make check" and compile the treeb code on my machine. I’ve attached my changes. Thank you for the review, and the patches!

Re: Index AM API cleanup

> On Aug 26, 2024, at 5:21 AM, Peter Eisentraut wrote: > > On 21.08.24 21:25, Mark Dilger wrote: >> The next twenty patches are a mix of fixes of various layering >> violations, such as not allowing non-core index AMs from use in replica >> identity full, or for speculative insertion, or for f

Re: Index AM API cleanup

On 21.08.24 21:25, Mark Dilger wrote: The next twenty patches are a mix of fixes of various layering violations, such as not allowing non-core index AMs from use in replica identity full, or for speculative insertion, or for foreign key constraints, or as part of merge join; with updates to the "

Re: Index AM API cleanup

Hi Mark, On Wed, Aug 21, 2024 at 2:25 PM Mark Dilger wrote: > > > For validation purposes, the first patch creates shallow copies of hash and > btree named "xash" and "xtree" and introduces some infrastructure to run the > src/test/regress and src/test/isolation tests against them without needi

Re: Index AM API cleanup

Mark Dilger writes: >> On Aug 21, 2024, at 12:34 PM, Kirill Reshke wrote: >> Hi! Why is the patch attached as .tar.bz2? Usually raw patches are sent >> here... > I worried the patch set, being greater than 1 MB, might bounce or be held up > in moderation. I'm +1 for doing it like this with su

Re: Index AM API cleanup

On 2024-08-21 We 4:09 PM, Mark Dilger wrote: On Aug 21, 2024, at 12:34 PM, Kirill Reshke wrote: Hi! Why is the patch attached as .tar.bz2? Usually raw patches are sent here... I worried the patch set, being greater than 1 MB, might bounce or be held up in moderation. Yes, it would have

Re: Index AM API cleanup

> On Aug 21, 2024, at 12:34 PM, Kirill Reshke wrote: > > Hi! Why is the patch attached as .tar.bz2? Usually raw patches are sent > here... I worried the patch set, being greater than 1 MB, might bounce or be held up in moderation. — Mark Dilger EnterpriseDB: http://www.enterprisedb.com The

Re: Index AM API cleanup

On Thu, 22 Aug 2024 at 00:25, Mark Dilger wrote: > > Hackers, > > The index access method API mostly encapsulates the functionality of in-core > index types, with some lingering oversights and layering violations. There > has been an ongoing discussion for several release cycles concerning how