Re: pg_stat_statements vs. SELECT FOR UPDATE

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
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)

In response to

Browse pgsql-hackers by date

  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