On Wed, Jan 31, 2024 at 11:21 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Richard Guo <guofenglinux@gmail.com> writes: > On Wed, Jan 31, 2024 at 5:12 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> * Why is it okay to just use pull_varnos on a tablesample expression, >> when contain_references_to() does something different?
> Hmm, the main reason is that the expression_tree_walker() function does > not handle T_RestrictInfo nodes. So we cannot just use pull_varnos or > pull_var_clause on a restrictinfo list.
Right, the extract_actual_clauses step is not wanted for the tablesample expression. But my point is that surely the handling of Vars and PlaceHolderVars should be the same, which it's not, and your v11 makes it even less so. How can it be OK to ignore Vars in the restrictinfo case?
Hmm, in this specific scenario it seems that it's not possible to have Vars in the restrictinfo list that laterally reference the outer join relation; otherwise the clause containing such Vars would not be a restriction clause but rather a join clause. So in v11 I did not check Vars in contain_references_to().
But I agree that we'd better to handle Vars and PlaceHolderVars in the same way.
I think the code structure we need to end up with is a routine that takes a RestrictInfo-free node tree (and is called directly in the tablesample case) with a wrapper routine that strips the RestrictInfos (for use on restriction lists).
How about the attached v12 patch? But I'm a little concerned about omitting PVC_RECURSE_AGGREGATES and PVC_RECURSE_WINDOWFUNCS in this case. Is it possible that aggregates or window functions appear in the tablesample expression?