|From:||Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>|
|To:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>|
|Subject:||Re: pg_stat_statements vs. SELECT FOR UPDATE|
|Views:||Raw Message | Whole Thread | Download mbox|
>>>>> "Tom" == Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
Tom> +1 for not ignoring rowMarks, but this patch seems like a hack to
Tom> me. Why didn't you just add RowMarkClause as one of the known
Tom> alternative expression node types?
>> Because it's not an expression and nothing anywhere else in the
>> backend treats it as such?
Tom> Places such as outfuncs.c and copyfuncs.c don't draw a
Tom> distinction, and I don't see why pg_stat_statements should either.
Tom> It would just be one more place we'd have to fix if we ever allow
Tom> RowMarkClause in other places --- or, perhaps more realistically,
Tom> if the structure of Query.rowMarks becomes more complex than "flat
Tom> list of RowMarkClauses".
None of these possibilities seem even slightly realistic.
Tom> The other places you mention generally have some specific semantic
Tom> knowledge about rowmarks, and would have to be touched anyway if
Tom> any changes like that happen. But the jumbling code in
Tom> pg_stat_statements has no knowledge of any of that, it's just
Tom> interested in traversing the tree. So I'd put it on the same
Tom> semantic level as outfuncs.c.
JumbleExpr's header comments specifically say it's intended to handle
the same kinds of things as expression_tree_walker (barring planner-only
constructs), and RowMarkClause is not one of those things.
If it had been named JumbleNodeTree or whatever then I might agree with
you, but right now the organization of the code is more or less a
straight parallel to query_tree_walker / range_table_walker /
expression_tree_walker, and it seems to me that adding RowMarkClause as
an "expression" node would just distort that parallel for no adequate
|Next Message||Chapman Flack||2019-01-20 04:36:59||Re: PostgreSQL vs SQL/XML Standards|
|Previous Message||Amit Kapila||2019-01-20 03:44:39||Re: [PATCH] Fix Proposal - Deadlock Issue in Single User Mode When IO Failure Occurs|