Funny representation in pg_stat_statements.query. - Mailing list pgsql-hackers

Hello, I noticed that pg_stat_statements.query can have funny values.

| =# select pg_stat_statements_reset();
| =# select current_timestamp(0);
| <snip>
| =# select query from pg_stat_statements;
|                query                
| ------------------------------------
|  select ?(0);
|  select pg_stat_statements_reset();

The same thing happenes for CURRENT_DATE, CURRENT_TIME, LOCALTIME
and LOCALTIMESTAMP which are specially treated, that is,
immediately replaced with a combination of a source function and
a typecast in gram.y.

The story is as follows,

At first, for instance, CURRENT_TIMESTAMP(0) is replaced with
'now()::timestamptz(0)' and "CURRENT_TIMESTAMP"'s location is
used as that of 'now' and the location of the const '1' of
LOCALTIME is used as that of the const '1' for 'timestamptz'.

Let's assume the orignal query was,

| pos: Query
|  0: SELECT
|  7: CURRENT_TIMESTAMP(
| 25: 0
|   : );

This is parsed in gram.y as follows, the location for
'CURRENT_TIMESTAMP' above (7) is used as the location for the
CONST "now".

| TypeCast {
|   TypeCast {
|     A_Const("now")
|     TypeName {names = ["text"], loc = -1 (undef) }
|     loc = 7
|   }
|   Typename {names = ["timestamptz"], loc = -1 }
|   typemods = [Const {1, loc = 25}]
| }

Then this is transformed into,

| FuncExpr {
|   funcid = 'timestamptz'
|   args = [
|     CoerceViaIO {
|       arg = Const {type = "text", value = "now", loc = 7 }
|       loc = -1
|     }
|     Const { type = "int4", value = 1, loc = -1 }
|   ]
| }

Finally pg_stat_statements picks the location '7' as a location
of some constant and replaces the token there with '?'
nevertheless it is originally the location of 'CURRENT_TIMESTAMP'
which is not a constant for users.

I found two ways to fix this issue. Both of them prevents wrong
masking but the original constant parameter ('0') becomes won't
be masked. This behavior seems sane enough for the porpose.

A. Making pg_stat_statements conscious of the token types to  prevent wrong masking.
 20140117_remove_needless_location_setting.patch

B. Letting gram.y not set the location for the problematic nodes.
 20140117_skip_nonconstants_on_nomalization.patch 

I don't have firm idea which is preferable. Or the possible
another solution.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index f0b9507..30da7aa 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -11355,7 +11355,7 @@ func_expr_common_subexpr:                     * to rely on it.)                     */
         Node *n;
 
-                    n = makeStringConstCast("now", @1, SystemTypeName("text"));
+                    n = makeStringConstCast("now", -1, SystemTypeName("text"));                    $$ =
makeTypeCast(n,SystemTypeName("date"), -1);                }            | CURRENT_TIME
 
@@ -11365,7 +11365,7 @@ func_expr_common_subexpr:                     * See comments for CURRENT_DATE.
  */                    Node *n;
 
-                    n = makeStringConstCast("now", @1, SystemTypeName("text"));
+                    n = makeStringConstCast("now", -1, SystemTypeName("text"));                    $$ =
makeTypeCast(n,SystemTypeName("timetz"), -1);                }            | CURRENT_TIME '(' Iconst ')'
 
@@ -11376,7 +11376,7 @@ func_expr_common_subexpr:                     */                    Node *n;
TypeName*d;
 
-                    n = makeStringConstCast("now", @1, SystemTypeName("text"));
+                    n = makeStringConstCast("now", -1, SystemTypeName("text"));                    d =
SystemTypeName("timetz");                   d->typmods = list_make1(makeIntConst($3, @3));                    $$ =
makeTypeCast(n,d, -1);
 
@@ -11397,7 +11397,7 @@ func_expr_common_subexpr:                     */                    Node *n;
TypeName*d;
 
-                    n = makeStringConstCast("now", @1, SystemTypeName("text"));
+                    n = makeStringConstCast("now", -1, SystemTypeName("text"));                    d =
SystemTypeName("timestamptz");                   d->typmods = list_make1(makeIntConst($3, @3));                    $$ =
makeTypeCast(n,d, -1);
 
@@ -11409,7 +11409,7 @@ func_expr_common_subexpr:                     * See comments for CURRENT_DATE.
  */                    Node *n;
 
-                    n = makeStringConstCast("now", @1, SystemTypeName("text"));
+                    n = makeStringConstCast("now", -1, SystemTypeName("text"));                    $$ =
makeTypeCast((Node*)n, SystemTypeName("time"), -1);                }            | LOCALTIME '(' Iconst ')'
 
@@ -11420,7 +11420,7 @@ func_expr_common_subexpr:                     */                    Node *n;
TypeName*d;
 
-                    n = makeStringConstCast("now", @1, SystemTypeName("text"));
+                    n = makeStringConstCast("now", -1, SystemTypeName("text"));                    d =
SystemTypeName("time");                   d->typmods = list_make1(makeIntConst($3, @3));                    $$ =
makeTypeCast((Node*)n, d, -1);
 
@@ -11432,7 +11432,7 @@ func_expr_common_subexpr:                     * See comments for CURRENT_DATE.
  */                    Node *n;
 
-                    n = makeStringConstCast("now", @1, SystemTypeName("text"));
+                    n = makeStringConstCast("now", -1, SystemTypeName("text"));                    $$ =
makeTypeCast(n,SystemTypeName("timestamp"), -1);                }            | LOCALTIMESTAMP '(' Iconst ')'
 
@@ -11443,7 +11443,7 @@ func_expr_common_subexpr:                     */                    Node *n;
TypeName*d;
 
-                    n = makeStringConstCast("now", @1, SystemTypeName("text"));
+                    n = makeStringConstCast("now", -1, SystemTypeName("text"));                    d =
SystemTypeName("timestamp");                   d->typmods = list_make1(makeIntConst($3, @3));                    $$ =
makeTypeCast(n,d, -1); 
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 2f069b7..9be9305 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -53,6 +53,7 @@#include "parser/analyze.h"#include "parser/parsetree.h"#include "parser/scanner.h"
+#include "parser/gram.h"#include "pgstat.h"#include "storage/fd.h"#include "storage/ipc.h"
@@ -2141,6 +2142,12 @@ fill_in_constant_lengths(pgssJumbleState *jstate, const char *query)
break;   /* out of inner for-loop */                }
 
+                /* Skip tokens other than constants */
+                if (!(tok == FCONST || tok == SCONST || tok == BCONST ||
+                      tok == XCONST || tok == ICONST || tok == NULL_P ||
+                      tok == TRUE_P || tok == FALSE_P))
+                    break;    /* out of inner for-loop */
+                /*                 * We now rely on the assumption that flex has placed a zero                 * byte
afterthe text of the current token in scanbuf.