Re: Funny representation in pg_stat_statements.query.

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: tgl(at)sss(dot)pgh(dot)pa(dot)us
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Funny representation in pg_stat_statements.query.
Date: 2014-01-20 05:15:56
Message-ID: 20140120.141556.20744456.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thank you.

tgl> > Hello, I noticed that pg_stat_statements.query can have funny values.
tgl>
tgl> I don't think that's an acceptable reason for lobotomizing the parser's
tgl> ability to print error cursors, which is what your first patch does
tgl> (and without even any documentation that would keep someone from
tgl> changing it back).

Yes, Nevertheless I don't think that replacement the keywords
with the fuction 'now' cannot results in error and the function
'now' itself cannot throw errors before significant changes is
made, of course there is no "never".

tgl> The second patch might be all right, though I'm a bit disturbed by its
tgl> dependency on gram.h constants. The numeric values of those change on a
tgl> regular basis, and who's to say that these are exactly the set of tokens
tgl> that we care about?

I felt the same anxiety. The code seems largely unstable because
mainly of the last reason. There seems no viable (and stable)
means to check and/or keep the pertinence of the token set. So I
also begged(?) for the third way.

tgl> In the end, really the cleanest fix for this would be to get rid of the
tgl> grammar's translation of these special functions into hacky expressions.
tgl> They ought to get translated into some new node type(s) that could be
tgl> reverse-listed in standard form by ruleutils.c. I've wanted to do that
tgl> for years, but never got around to it ...

Yes, that's what somewhat worried me. That way enables us to use
the lexical locations without extra care. (although also seems to
be a bit too heavy labor for the gain..)

- CURRENT_TIME and the like are parsed into the nodes directly
represents the node specs in gram.y

- add transform.. uh.. transformDatetimeFuncs or something to
transform the nodes into executable form, perhaps. But it
should be after pg_stat_statements refer to the query tree.

- modify ruleutils.c in corresponding part if needed.

Since this becomes more than a simple bug fix, I think it is too
late for 9.4 now. I'll work on this in the longer term.

Thanks for the suggestion.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Craig Ringer 2014-01-20 06:15:13 Re: [v9.4] row level security
Previous Message Amit Kapila 2014-01-20 05:12:57 Re: ALTER SYSTEM SET typos and fix for temporary file name management