Re: Incremental sorts and EXEC_FLAG_REWIND

On Tue, Apr 14, 2020 at 2:53 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> Hi,
>
> When initializing an incremental sort node, we have the following as
> of ExecInitIncrementalSort():
> /*
> * Incremental sort can't be used with either EXEC_FLAG_REWIND,
> * EXEC_FLAG_BACKWARD or EXEC_FLAG_MARK, because we only one of many sort
> * batches in the current sort state.
> */
> Assert((eflags & (EXEC_FLAG_BACKWARD |
> EXEC_FLAG_MARK)) == 0);
> While I don't quite follow why EXEC_FLAG_REWIND should be allowed here
> to begin with (because incremental sorts don't support rescans without
> parameter changes, right?), the comment and the assertion are telling
> a different story.

I remember changing this assertion in response to an issue I'd found
which led to rewriting the rescan implementation, but I must have
missed updating the comment.

> And I can see that child nodes of an
> IncrementalSort one use a set of eflags where these three are removed:
> /*
> * Initialize child nodes.
> *
> * We shield the child node from the need to support REWIND, BACKWARD, or
> * MARK/RESTORE.
> */
> eflags &= ~(EXEC_FLAG_REWIND | EXEC_FLAG_BACKWARD | EXEC_FLAG_MARK);
>
> I can also spot one case in the regression tests where we actually pass
> down a REWIND flag (see incremental_sort.sql) when initializing an
> IncrementalSort node:
> -- We force the planner to choose a plan with incremental sort on the right side
> -- of a nested loop join node. That way we trigger the rescan code path.
> set local enable_hashjoin = off;
> set local enable_mergejoin = off;
> set local enable_material = off;
> set local enable_sort = off;
> explain (costs off) select * from t left join (select * from (select *
> from t order by a) v order by a, b) s on s.a = t.a where t.a in (1,
> 2);
> select * from t left join (select * from (select * from t order by a)
> v order by a, b) s on s.a = t.a where t.a in (1, 2);
>
> Alexander, Tomas, any thoughts?

I'll try to respond more fully later today when I can dig up the
specific change.

In the meantime, your question is primarily about making sure the
code/comments/etc. are consistent and not a behavioral problem or
failure you've seen in testing?

James