Re: [BUGS] BUG #14870: wrong query results when using WITH with UPDATE

Marko Tiikkaja <marko@joh.to> writes:
> Does anyone care to care about this?  We've had two report of what I think
> is the same problem, yet nobody's properly analyzed the problem.

Hm, what's the other report that you think is related?

I got around to tracing through this finally, and there seem to be a
couple of interrelated issues.  The real core of the problem is that
it didn't occur to me while writing CteScanNext that calling the input
plan could result in invoking a different CteScan on the same worktable.
This means that when we come back from the ExecProcNode call, the CTE's
shared tuplestore might now have somebody else's read pointer selected
as the active read pointer.  Since tuplestore_puttupleslot has different
effects on the active read pointer than on inactive ones, the wrong things
happen.  The fix is trivial: just reselect our read pointer as active
before storing the fetched tuple, viz

        /*
         * Append a copy of the returned tuple to tuplestore.  NOTE: because
         * our read pointer is certainly in EOF state, its read position will
         * move forward over the added tuple.  This is what we want.  Also,
         * any other readers will *not* move past the new tuple, which is what
         * they want.
         */
+       tuplestore_select_read_pointer(tuplestorestate, node->readptr);
        tuplestore_puttupleslot(tuplestorestate, cteslot);

This could, conceivably, account for all sorts of weird misbehavior if
the same CTE is read in different levels of a plan.  However, it wasn't
immediately obvious to me why the given examples involving EPQ would
cause it, because in fact these queries *don't* cause recursion of
CteScanNext.  There is recursion of ExecCteScan, but the recursive
call happens while projecting the result tuple of the outer CteScan
node, not while it's fetching an input tuple.  In fact, since what
we're calling is supposedly just the CTE subplan, it's not real
clear how it could contain another scan of itself.

The answer turns out to be that if EPQ is fired, then we instantiate a new
copy of the whole outer plan tree (though apparently not the InitPlan),
resulting in an additional ExecInitCteScan call that allocates an
additional read pointer on the same tuplestore, and it's that one being
the active one that causes the observed misbehavior.

This is kind of annoying because the EPQ-instantiated CteScan will never
actually read the tuplestore at all, so the extra read pointer it creates
is useless and just adds overhead to the tuplestore.  Since we can't
currently truncate a CteScan's tuplestore anyway, it might not be worth
worrying about today, and I certainly wouldn't back-patch a fix for it.
But if we ever get smarter about that, we should try to avoid touching
the tuplestore in EPQ subplans.  This seems related to the indexscan EPQ
bug we fixed the other day in 2e668c522...

            regards, tom lane