Thread: (Comment)Bug in CteScanNext
In CteScanNext():
/* * If we are not at the end of the tuplestore, or are going
backwards, try * to fetch a tuple from tuplestore. */ eof_tuplestore =
tuplestore_ateof(tuplestorestate);
if (!forward && eof_tuplestore)
For the comment to be correct, wouldn't the condition need to be
(!forward || !eof_tuplestore)?
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532) mobile: 512-569-9461
Jim Nasby <Jim.Nasby@BlueTreble.com> writes:
> In CteScanNext():
> /*
> * If we are not at the end of the tuplestore, or are going
> backwards, try
> * to fetch a tuple from tuplestore.
> */
> eof_tuplestore = tuplestore_ateof(tuplestorestate);
> if (!forward && eof_tuplestore)
> For the comment to be correct, wouldn't the condition need to be
> (!forward || !eof_tuplestore)?
No. The comment is describing the overall strategy for the next ~30
lines. That first if-test is dealing with a sub-case, ie reversing
direction after reaching EOF. The code's fine, and the comments are
okay as far as they go, but maybe some rearrangement would be in order.
Or we could add something like "But first, we must deal with the special
case of reversing direction after reaching EOF."
regards, tom lane
On 9/3/16 1:30 PM, Tom Lane wrote: > Or we could add something like "But first, we must deal with the special > case of reversing direction after reaching EOF." I'm working on that, but one thing isn't clear to me... why do we only skip past the last tuple if (!node->leader->eof_cte)? Even if we've hit the end of the underlying node, the tuplestore could still return data, and AFAICT we would still need to move past the last item in the tuplestore, no? -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com 855-TREBLE2 (855-873-2532) mobile: 512-569-9461
Jim Nasby <Jim.Nasby@BlueTreble.com> writes:
> On 9/3/16 1:30 PM, Tom Lane wrote:
>> Or we could add something like "But first, we must deal with the special
>> case of reversing direction after reaching EOF."
> I'm working on that, but one thing isn't clear to me... why do we only
> skip past the last tuple if (!node->leader->eof_cte)? Even if we've hit
> the end of the underlying node, the tuplestore could still return data,
> and AFAICT we would still need to move past the last item in the
> tuplestore, no?
If tuplestore_ateof is true, then what presumably happened in the previous
call is that we fetched a row (moving forward) from the underlying plan,
appended it to the tuplestore, and returned it to the caller. If the
current call specifies moving backwards, we want to return the tuple
before that one, not the same one again.
Alternatively, if we'd already backed up some and gone forwards again (all
fetching from the tuplestore), then ateof means that the last call
returned the last row currently in the tuplestore. Again, we don't want
to return that row twice.
On the other hand, if eof_cte is true, then what happened on the last
call is that we tried to fetch forwards, reached EOF on the underlying
query, and returned NULL. In that case, a backwards fetch *should*
produce the last row in the tuplestore.
regards, tom lane
On 9/6/16 12:00 PM, Tom Lane wrote: > On the other hand, if eof_cte is true, then what happened on the last > call is that we tried to fetch forwards, reached EOF on the underlying > query, and returned NULL. In that case, a backwards fetch *should* > produce the last row in the tuplestore. Patch attached. -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com 855-TREBLE2 (855-873-2532) mobile: 512-569-9461