From: | Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: pg_stat_statements vs. SELECT FOR UPDATE |
Date: | 2019-01-20 03:49:05 |
Message-ID: | 87ef98577c.fsf@news-spur.riddles.org.uk |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
>>>>> "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
reason.
--
Andrew (irc:RhodiumToad)
From | Date | Subject | |
---|---|---|---|
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 |