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

po 17. 4. 2023 o 15:26 Richard Guo <guofenglinux@gmail.com> napísal(a):
>
>
> On Sun, Apr 16, 2023 at 1:20 AM Miroslav Bendik <miroslav.bendik@gmail.com> 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 this is a meaningful optimization.  I reviewed the patch and
> here are the comments from me.
>
> * I understand the new param 'match_pathkeys_length_p' is used to tell
> how many presorted keys are useful.  I think list_length(orderbyclauses)
> will do the same.  So there is no need to add the new param, thus we can
> reduce the code diffs.
>
> * Now that match_pathkeys_to_index() returns a prefix of the pathkeys
> rather than returns NIL immediately when there is a failure to match, it
> seems the two local variables 'orderby_clauses' and 'clause_columns' are
> not necessary any more.  I think we can instead lappend the matched
> 'expr' and 'indexcol' to '*orderby_clauses_p' and '*clause_columns_p'
> directly.  In this way we can still call 'return' when we come to a
> failure to match.
>
> * In build_index_paths(), I think the diff can be reduced to
>
> -    if (orderbyclauses)
> -        useful_pathkeys = root->query_pathkeys;
> -    else
> -        useful_pathkeys = NIL;
> +    useful_pathkeys = list_truncate(list_copy(root->query_pathkeys),
> +                                    list_length(orderbyclauses));
>
> * Several comments in match_pathkeys_to_index() are out of date.  We
> need to revise them to cope with the change.
>
> * I think it's better to provide a test case.
>
> Thanks
> Richard

Thank you for advice,
here is an updated patch with proposed changes.

-- 
Best regards
Miroslav