Re: BUG: pg_stat_statements query normalization issues with combined queries

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, craig(dot)ringer(at)2ndquadrant(dot)com, robertmhaas(at)gmail(dot)com, PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: BUG: pg_stat_statements query normalization issues with combined queries
Date: 2016-12-28 00:33:28
Message-ID: 20417.1482885208@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> writes:
>>> How? The issue is that stmtmulti silently skip some ';' when empty
>>> statements are found, [...]

>> Maybe you should undo that.

> I was trying to be minimally invasive in the parser, i.e. not to change
> any rules.

That's fairly silly when the alternative is to be maximally invasive
at the parse-nodes level, thereby affecting lots and lots of code that
isn't really related to the problem at hand.

>> I do not like this. You are making what should be a small patch into
>> a giant, invasive one.

> I would not say that the current patch is giant & invasive, if you
> abstract the two added fields to high-level statements.

It's touching every single utility statement type, which is not only
pretty invasive in itself, but will break any pending patches that
add more utility statement types. And you've not followed through on the
implications of adding those fields in, for instance, src/backend/nodes/;
so the patch will be even bigger once all the required tidying-up is done.

> I understand that you suggest to add a new intermediate structure:

> typedef struct {
> NodeTag tag;
> int location, length;
> Node *stmt;
> } ParseNode;

> So that raw_parser would return a List<ParseNode> instead of a List<Node>,
> and the statements would be unmodified.

Yeah, that's what I was thinking of. There aren't very many places that
would need to know about that, I believe; possibly just gram.y itself
and transformTopLevelStmt(). The stuff in between only knows that it's
passing around lists of statement parse trees, it doesn't know much about
what's in those trees.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tsunakawa, Takayuki 2016-12-28 00:47:10 Re: Hooks
Previous Message Fabrízio de Royes Mello 2016-12-28 00:17:23 Re: proposal: session server side variables