On Tue, Apr 14, 2020 at 2:53 AM Michael Paquier <mich...@paquier.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
