Re: Incremental sort for access method with ordered scan support (amcanorderbyop)
David Rowley <dgrowleyml@gmail.com>
From: David Rowley <dgrowleyml@gmail.com>
To: Miroslav Bendik <miroslav.bendik@gmail.com>
Cc: Richard Guo <guofenglinux@gmail.com>, pgsql-hackers@postgresql.org
Date: 2023-04-19T22:37:49Z
Lists: pgsql-hackers
On Wed, 19 Apr 2023 at 16:53, Miroslav Bendik <miroslav.bendik@gmail.com> 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 query_pathkeys is NIL. I need either modify > list_copy_head (v3.1) or add checks before call (v3.2). > > I don't know if it's a good idea to modify list_copy_head. It will add > additional overhead to every call. That's a bug in list_copy_head(). Since NIL is how we represent empty Lists, crashing on some valid representation of a List is not how it should work. 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'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. David [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=e35ded29566f679e52888a8d34468bb51bc78bed
Commits
-
Allow Incremental Sorts on GiST and SP-GiST indexes
- 625d5b3ca096 17.0 landed
-
Fix list_copy_head() with empty Lists
- 63a03aea6bc8 15.3 landed
- e35ded29566f 16.0 landed