Re: Incremental sort for access method with ordered scan support (amcanorderbyop)

On 7/5/23 2:15 AM, Richard Guo wrote: On Tue, Jul 4, 2023 at 7:15 PM David Rowley > wrote: On Tue, 4 Jul 2023 at 20:12, Richard Guo mailto:guofengli...@gmail.com>> wrote: > The v4 patch looks good to me (maybe some cosmetic tweaks are still > needed

Re: Incremental sort for access method with ordered scan support (amcanorderbyop)

On Tue, Jul 4, 2023 at 7:15 PM David Rowley wrote: > On Tue, 4 Jul 2023 at 20:12, Richard Guo wrote: > > The v4 patch looks good to me (maybe some cosmetic tweaks are still > > needed for the comments). I think it's now 'Ready for Committer'. > > I agree. I went and hit the comments with a

Re: Incremental sort for access method with ordered scan support (amcanorderbyop)

On Tue, 4 Jul 2023 at 20:12, Richard Guo wrote: > The v4 patch looks good to me (maybe some cosmetic tweaks are still > needed for the comments). I think it's now 'Ready for Committer'. I agree. I went and hit the comments with a large hammer and while there also adjusted the regression tests.

Re: Incremental sort for access method with ordered scan support (amcanorderbyop)

On Sun, Jul 2, 2023 at 12:02 PM Miroslav Bendik wrote: > Thanks, for suggestions. > > On Sun 02. 07. 2023 at 10:18 Richard Guo wrote: > > 1. For comment "On success, the result list is ordered by pathkeys.", I > > think it'd be more accurate if we say something like "On success, the > > result

Re: Incremental sort for access method with ordered scan support (amcanorderbyop)

Thanks, for suggestions. On Sun 02. 07. 2023 at 10:18 Richard Guo wrote: > 1. For comment "On success, the result list is ordered by pathkeys.", I > think it'd be more accurate if we say something like "On success, the > result list is ordered by pathkeys or a prefix list of pathkeys." >

Re: Incremental sort for access method with ordered scan support (amcanorderbyop)

On Thu, Apr 20, 2023 at 9:37 PM Miroslav Bendik wrote: > Thanks for this fix. Now the version > am_orderbyop_incremental_sort_v3.1.patch [1] works without issues > using the master branch. The v3.1 patch looks good to me, except that the comments around match_pathkeys_to_index still need some

Re: Incremental sort for access method with ordered scan support (amcanorderbyop)

On Fri, Apr 21, 2023 at 5:43 AM David Rowley wrote: > On Thu, 20 Apr 2023 at 18:46, Richard Guo wrote: > > I searched the codes and found some other places where the manipulation > > of lists can be improved in a similar way. > I'd be happy to discuss our thought about List inefficiencies, but

Re: Incremental sort for access method with ordered scan support (amcanorderbyop)

On Thu, 20 Apr 2023 at 18:46, Richard Guo wrote: > > > On Thu, Apr 20, 2023 at 6:38 AM David Rowley wrote: >> >> That function is pretty new and was exactly added so we didn't have to >> write list_truncate(list_copy(...), n) anymore. That gets pretty >> wasteful when the input List is long and

Re: Incremental sort for access method with ordered scan support (amcanorderbyop)

>I searched the codes and found some other places where the manipulation >of lists can be improved in a similar way. >* lappend(list_copy(list), datum) as in get_required_extension(). >This is not very efficient as after list_copy it would need to enlarge >the list immediately. It can be improved

Re: Incremental sort for access method with ordered scan support (amcanorderbyop)

> I've just pushed a fix to master for this. See [1]. If you base your > patch atop of that you should be able to list list_copy_head() without > any issues. Thanks for this fix. Now the version am_orderbyop_incremental_sort_v3.1.patch [1] works without issues using the master branch. [1]

Re: Incremental sort for access method with ordered scan support (amcanorderbyop)

On Thu, Apr 20, 2023 at 6:38 AM David Rowley wrote: > That function is pretty new and was exactly added so we didn't have to > write list_truncate(list_copy(...), n) anymore. That gets pretty > wasteful when the input List is long and we only need a small portion > of it. I searched the codes

Re: Incremental sort for access method with ordered scan support (amcanorderbyop)

On Wed, 19 Apr 2023 at 16:53, Miroslav Bendik wrote: > > 2. You can use list_copy_head(root->query_pathkeys, > > list_length(orderbyclauses)); instead of: > > > > + useful_pathkeys = list_truncate(list_copy(root->query_pathkeys), > > + list_length(orderbyclauses)); > > This code will crash if

Re: Incremental sort for access method with ordered scan support (amcanorderbyop)

Sorry for spamming, but I found a more elegant way to check if query_paths is NIL without modified list_copy_head. Here is a third iteration of this patch. -- Miroslav diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index b8000da56d..735b41552a 100644

Re: Incremental sort for access method with ordered scan support (amcanorderbyop)

Thanks for feedback > 2. You can use list_copy_head(root->query_pathkeys, > list_length(orderbyclauses)); instead of: > > + useful_pathkeys = list_truncate(list_copy(root->query_pathkeys), > + list_length(orderbyclauses)); This code will crash if query_pathkeys is NIL. I need either modify

Re: Incremental sort for access method with ordered scan support (amcanorderbyop)

On Tue, 18 Apr 2023 at 19:29, Miroslav Bendik wrote: > here is an updated patch with proposed changes. Here's a quick review: 1. I don't think this is required. match_pathkeys_to_index() sets these to NIL and they're set accordingly by the other code paths. - List*orderbyclauses; - List

Re: Incremental sort for access method with ordered scan support (amcanorderbyop)

po 17. 4. 2023 o 15:26 Richard Guo napísal(a): > > > On Sun, Apr 16, 2023 at 1:20 AM Miroslav Bendik > wrote: >> >> Postgres allows incremental sort only for ordered indexes. Function >> build_index_paths dont build partial order paths for access methods >> with order support. My patch adds

Re: Incremental sort for access method with ordered scan support (amcanorderbyop)

On Sun, Apr 16, 2023 at 1:20 AM Miroslav Bendik wrote: > Postgres allows incremental sort only for ordered indexes. Function > build_index_paths dont build partial order paths for access methods > with order support. My patch adds support for incremental ordering > with access method. I think

Incremental sort for access method with ordered scan support (amcanorderbyop)

Current version of postgresql don't support incremental sort using ordered scan on access method. Example database: CREATE TABLE t (id serial, p point, PRIMARY KEY(id)); INSERT INTO t (SELECT generate_series(1, 1000), point(random(), random())); CREATE INDEX p_idx ON t USING gist(p);