I wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> ... Which then allowed me to
>> construct the example above, where there are two possible TID quals
>> and the logic in tidpath.c latches onto the wrong one.
> Hmm. Without having traced through it, I'm betting that the
> CurrentOfExpr qual is rejected as a tidqual because it's not
> considered leakproof.
Nah, I'm wrong: we do treat it as leakproof, and the comment about
that in contain_leaked_vars_walker shows that the interaction with
RLS quals *was* thought about. What wasn't thought about was the
possibility of RLS quals that themselves could be usable as tidquals,
which breaks this assumption in TidQualFromRestrictInfoList:
* Stop as soon as we find any usable CTID condition. In theory we
* could get CTID equality conditions from different AND'ed clauses,
* in which case we could try to pick the most efficient one. In
* practice, such usage seems very unlikely, so we don't bother; we
* just exit as soon as we find the first candidate.
The executor doesn't seem to be prepared to cope with multiple AND'ed
TID clauses (only OR'ed ones). So we need to fix this at least to the
extent of looking for a CurrentOfExpr qual, and preferring that over
anything else.
I'm also now wondering about this assumption in the executor:
/* CurrentOfExpr could never appear OR'd with something else */
Assert(list_length(tidstate->tss_tidexprs) == 1 ||
!tidstate->tss_isCurrentOf);
It still seems OK, because anything that might come in from RLS quals
would be AND'ed not OR'ed with the CurrentOfExpr.
> In general I think you're right that something less rickety than
> the disable_cost hack would be a good idea to ensure the desired
> TidPath gets chosen, but this problem is not the fault of that.
> We're not making the TidPath with the correct contents in the first
> place.
Still true.
regards, tom lane