On Fri, Dec 6, 2013 at 12:24 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
>> There seems to be no problem even if we use bigint as the type of
>> unsigned 32-bit integer like queryid. For example, txid_current()
>> returns the transaction ID, i.e., unsigned 32-bit integer, as bigint.
>> Could you tell me what the problem is when using bigint for queryid?
>
> We're talking about the output of some view, right, not internal storage?
> +1 for using bigint for that. Using OID is definitely an abuse, because
> the value *isn't* an OID. And besides, what if we someday decide we need
> 64-bit keys not 32-bit?
Fair enough. I was concerned about the cost of external storage of
64-bit integers (unlike query text, they might have to be stored many
times for many distinct intervals or something like that), but in
hindsight that was fairly miserly of me.
Attached revision displays signed 64-bit integers instead.
--
Peter Geoghegan
diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
new file mode 100644
index e8aed61..95a2767
*** a/contrib/pg_stat_statements/Makefile
--- b/contrib/pg_stat_statements/Makefile
*************** MODULE_big = pg_stat_statements
*** 4,11 ****
OBJS = pg_stat_statements.o
EXTENSION = pg_stat_statements
! DATA = pg_stat_statements--1.1.sql pg_stat_statements--1.0--1.1.sql \
! pg_stat_statements--unpackaged--1.0.sql
ifdef USE_PGXS
PG_CONFIG = pg_config
--- 4,11 ----
OBJS = pg_stat_statements.o
EXTENSION = pg_stat_statements
! DATA = pg_stat_statements--1.2.sql pg_stat_statements--1.1--1.2.sql \
! pg_stat_statements--1.0--1.1.sql pg_stat_statements--unpackaged--1.0.sql
ifdef USE_PGXS
PG_CONFIG = pg_config
diff --git a/contrib/pg_stat_statements/pg_stat_statements--1.1--1.2.sql b/contrib/pg_stat_statements/pg_stat_statements--1.1--1.2.sql
new file mode 100644
index ...7824e3e
*** a/contrib/pg_stat_statements/pg_stat_statements--1.1--1.2.sql
--- b/contrib/pg_stat_statements/pg_stat_statements--1.1--1.2.sql
***************
*** 0 ****
--- 1,43 ----
+ /* contrib/pg_stat_statements/pg_stat_statements--1.1--1.2.sql */
+
+ -- complain if script is sourced in psql, rather than via ALTER EXTENSION
+ \echo Use "ALTER EXTENSION pg_stat_statements UPDATE" to load this file. \quit
+
+ /* First we have to remove them from the extension */
+ ALTER EXTENSION pg_stat_statements DROP VIEW pg_stat_statements;
+ ALTER EXTENSION pg_stat_statements DROP FUNCTION pg_stat_statements();
+
+ /* Then we can drop them */
+ DROP VIEW pg_stat_statements;
+ DROP FUNCTION pg_stat_statements();
+
+ /* Now redefine */
+ CREATE FUNCTION pg_stat_statements(
+ OUT userid oid,
+ OUT dbid oid,
+ OUT queryid bigint,
+ OUT query text,
+ OUT calls int8,
+ OUT total_time float8,
+ OUT rows int8,
+ OUT shared_blks_hit int8,
+ OUT shared_blks_read int8,
+ OUT shared_blks_dirtied int8,
+ OUT shared_blks_written int8,
+ OUT local_blks_hit int8,
+ OUT local_blks_read int8,
+ OUT local_blks_dirtied int8,
+ OUT local_blks_written int8,
+ OUT temp_blks_read int8,
+ OUT temp_blks_written int8,
+ OUT blk_read_time float8,
+ OUT blk_write_time float8
+ )
+ RETURNS SETOF record
+ AS 'MODULE_PATHNAME'
+ LANGUAGE C;
+
+ CREATE VIEW pg_stat_statements AS
+ SELECT * FROM pg_stat_statements();
+
+ GRANT SELECT ON pg_stat_statements TO PUBLIC;
diff --git a/contrib/pg_stat_statements/pg_stat_statements--1.1.sql b/contrib/pg_stat_statements/pg_stat_statements--1.1.sql
new file mode .
index 42e4d68..e69de29
*** a/contrib/pg_stat_statements/pg_stat_statements--1.1.sql
--- b/contrib/pg_stat_statements/pg_stat_statements--1.1.sql
***************
*** 1,43 ****
- /* contrib/pg_stat_statements/pg_stat_statements--1.1.sql */
-
- -- complain if script is sourced in psql, rather than via CREATE EXTENSION
- \echo Use "CREATE EXTENSION pg_stat_statements" to load this file. \quit
-
- -- Register functions.
- CREATE FUNCTION pg_stat_statements_reset()
- RETURNS void
- AS 'MODULE_PATHNAME'
- LANGUAGE C;
-
- CREATE FUNCTION pg_stat_statements(
- OUT userid oid,
- OUT dbid oid,
- OUT query text,
- OUT calls int8,
- OUT total_time float8,
- OUT rows int8,
- OUT shared_blks_hit int8,
- OUT shared_blks_read int8,
- OUT shared_blks_dirtied int8,
- OUT shared_blks_written int8,
- OUT local_blks_hit int8,
- OUT local_blks_read int8,
- OUT local_blks_dirtied int8,
- OUT local_blks_written int8,
- OUT temp_blks_read int8,
- OUT temp_blks_written int8,
- OUT blk_read_time float8,
- OUT blk_write_time float8
- )
- RETURNS SETOF record
- AS 'MODULE_PATHNAME'
- LANGUAGE C;
-
- -- Register a view on the function for ease of use.
- CREATE VIEW pg_stat_statements AS
- SELECT * FROM pg_stat_statements();
-
- GRANT SELECT ON pg_stat_statements TO PUBLIC;
-
- -- Don't want this to be available to non-superusers.
- REVOKE ALL ON FUNCTION pg_stat_statements_reset() FROM PUBLIC;
--- 0 ----
diff --git a/contrib/pg_stat_statements/pg_stat_statements--1.2.sql b/contrib/pg_stat_statements/pg_stat_statements--1.2.sql
new file mode 100644
index ...80b74a1
*** a/contrib/pg_stat_statements/pg_stat_statements--1.2.sql
--- b/contrib/pg_stat_statements/pg_stat_statements--1.2.sql
***************
*** 0 ****
--- 1,44 ----
+ /* contrib/pg_stat_statements/pg_stat_statements--1.2.sql */
+
+ -- complain if script is sourced in psql, rather than via CREATE EXTENSION
+ \echo Use "CREATE EXTENSION pg_stat_statements" to load this file. \quit
+
+ -- Register functions.
+ CREATE FUNCTION pg_stat_statements_reset()
+ RETURNS void
+ AS 'MODULE_PATHNAME'
+ LANGUAGE C;
+
+ CREATE FUNCTION pg_stat_statements(
+ OUT userid oid,
+ OUT dbid oid,
+ OUT queryid bigint,
+ OUT query text,
+ OUT calls int8,
+ OUT total_time float8,
+ OUT rows int8,
+ OUT shared_blks_hit int8,
+ OUT shared_blks_read int8,
+ OUT shared_blks_dirtied int8,
+ OUT shared_blks_written int8,
+ OUT local_blks_hit int8,
+ OUT local_blks_read int8,
+ OUT local_blks_dirtied int8,
+ OUT local_blks_written int8,
+ OUT temp_blks_read int8,
+ OUT temp_blks_written int8,
+ OUT blk_read_time float8,
+ OUT blk_write_time float8
+ )
+ RETURNS SETOF record
+ AS 'MODULE_PATHNAME'
+ LANGUAGE C;
+
+ -- Register a view on the function for ease of use.
+ CREATE VIEW pg_stat_statements AS
+ SELECT * FROM pg_stat_statements();
+
+ GRANT SELECT ON pg_stat_statements TO PUBLIC;
+
+ -- Don't want this to be available to non-superusers.
+ REVOKE ALL ON FUNCTION pg_stat_statements_reset() FROM PUBLIC;
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
new file mode 100644
index 12322b8..5890109
*** a/contrib/pg_stat_statements/pg_stat_statements.c
--- b/contrib/pg_stat_statements/pg_stat_statements.c
*************** PG_MODULE_MAGIC;
*** 67,73 ****
#define PGSS_DUMP_FILE "global/pg_stat_statements.stat"
/* This constant defines the magic number in the stats file header */
! static const uint32 PGSS_FILE_HEADER = 0x20120328;
/* XXX: Should USAGE_EXEC reflect execution time and/or buffer usage? */
#define USAGE_EXEC(duration) (1.0)
--- 67,75 ----
#define PGSS_DUMP_FILE "global/pg_stat_statements.stat"
/* This constant defines the magic number in the stats file header */
! static const uint32 PGSS_FILE_HEADER = 0x20131115;
! /* Postgres major version number, changes in which invalidate all entries */
! static const uint32 PGSS_PG_MAJOR_VERSION = PG_VERSION_NUM / 100;
/* XXX: Should USAGE_EXEC reflect execution time and/or buffer usage? */
#define USAGE_EXEC(duration) (1.0)
*************** static const uint32 PGSS_FILE_HEADER = 0
*** 80,85 ****
--- 82,97 ----
#define JUMBLE_SIZE 1024 /* query serialization buffer size */
/*
+ * Extension version number, for supporting older extension versions' objects
+ */
+ typedef enum pgssVersion
+ {
+ PGSS_V1_0 = 0,
+ PGSS_V1_1,
+ PGSS_V1_2
+ } pgssVersion;
+
+ /*
* Hashtable key that defines the identity of a hashtable entry. We separate
* queries by user and by database even if they are otherwise identical.
*
*************** pgss_shmem_startup(void)
*** 390,395 ****
--- 402,408 ----
FILE *file;
uint32 header;
int32 num;
+ int32 pgver;
int32 i;
int query_size;
int buffer_size;
*************** pgss_shmem_startup(void)
*** 465,470 ****
--- 478,485 ----
if (fread(&header, sizeof(uint32), 1, file) != 1 ||
header != PGSS_FILE_HEADER ||
+ fread(&pgver, sizeof(uint32), 1, file) != 1 ||
+ pgver != PGSS_PG_MAJOR_VERSION ||
fread(&num, sizeof(int32), 1, file) != 1)
goto error;
*************** pgss_shmem_shutdown(int code, Datum arg)
*** 565,570 ****
--- 580,587 ----
if (fwrite(&PGSS_FILE_HEADER, sizeof(uint32), 1, file) != 1)
goto error;
+ if (fwrite(&PGSS_PG_MAJOR_VERSION, sizeof(uint32), 1, file) != 1)
+ goto error;
num_entries = hash_get_num_entries(pgss_hash);
if (fwrite(&num_entries, sizeof(int32), 1, file) != 1)
goto error;
*************** pg_stat_statements_reset(PG_FUNCTION_ARG
*** 1069,1075 ****
}
#define PG_STAT_STATEMENTS_COLS_V1_0 14
! #define PG_STAT_STATEMENTS_COLS 18
/*
* Retrieve statement statistics.
--- 1086,1093 ----
}
#define PG_STAT_STATEMENTS_COLS_V1_0 14
! #define PG_STAT_STATEMENTS_COLS_V1_1 18
! #define PG_STAT_STATEMENTS_COLS 19
/*
* Retrieve statement statistics.
*************** pg_stat_statements(PG_FUNCTION_ARGS)
*** 1086,1092 ****
bool is_superuser = superuser();
HASH_SEQ_STATUS hash_seq;
pgssEntry *entry;
! bool sql_supports_v1_1_counters = true;
if (!pgss || !pgss_hash)
ereport(ERROR,
--- 1104,1110 ----
bool is_superuser = superuser();
HASH_SEQ_STATUS hash_seq;
pgssEntry *entry;
! pgssVersion detected_version;
if (!pgss || !pgss_hash)
ereport(ERROR,
*************** pg_stat_statements(PG_FUNCTION_ARGS)
*** 1107,1114 ****
/* Build a tuple descriptor for our result type */
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
! if (tupdesc->natts == PG_STAT_STATEMENTS_COLS_V1_0)
! sql_supports_v1_1_counters = false;
per_query_ctx = rsinfo->econtext->ecxt_per_query_memory;
oldcontext = MemoryContextSwitchTo(per_query_ctx);
--- 1125,1145 ----
/* Build a tuple descriptor for our result type */
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
!
! switch (tupdesc->natts)
! {
! case PG_STAT_STATEMENTS_COLS_V1_0:
! detected_version = PGSS_V1_0;
! break;
! case PG_STAT_STATEMENTS_COLS_V1_1:
! detected_version = PGSS_V1_1;
! break;
! case PG_STAT_STATEMENTS_COLS:
! detected_version = PGSS_V1_2;
! break;
! default:
! elog(ERROR, "pgss version unrecognized from tuple descriptor");
! }
per_query_ctx = rsinfo->econtext->ecxt_per_query_memory;
oldcontext = MemoryContextSwitchTo(per_query_ctx);
*************** pg_stat_statements(PG_FUNCTION_ARGS)
*** 1140,1145 ****
--- 1171,1179 ----
{
char *qstr;
+ if (detected_version >= PGSS_V1_2)
+ values[i++] = Int64GetDatumFast((int64) entry->key.queryid);
+
qstr = (char *)
pg_do_encoding_conversion((unsigned char *) entry->query,
entry->query_len,
*************** pg_stat_statements(PG_FUNCTION_ARGS)
*** 1150,1156 ****
--- 1184,1195 ----
pfree(qstr);
}
else
+ {
+ if (detected_version >= PGSS_V1_2)
+ nulls[i++] = true;
+
values[i++] = CStringGetTextDatum("<insufficient privilege>");
+ }
/* copy counters to a local variable to keep locking time short */
{
*************** pg_stat_statements(PG_FUNCTION_ARGS)
*** 1170,1193 ****
values[i++] = Int64GetDatumFast(tmp.rows);
values[i++] = Int64GetDatumFast(tmp.shared_blks_hit);
values[i++] = Int64GetDatumFast(tmp.shared_blks_read);
! if (sql_supports_v1_1_counters)
values[i++] = Int64GetDatumFast(tmp.shared_blks_dirtied);
values[i++] = Int64GetDatumFast(tmp.shared_blks_written);
values[i++] = Int64GetDatumFast(tmp.local_blks_hit);
values[i++] = Int64GetDatumFast(tmp.local_blks_read);
! if (sql_supports_v1_1_counters)
values[i++] = Int64GetDatumFast(tmp.local_blks_dirtied);
values[i++] = Int64GetDatumFast(tmp.local_blks_written);
values[i++] = Int64GetDatumFast(tmp.temp_blks_read);
values[i++] = Int64GetDatumFast(tmp.temp_blks_written);
! if (sql_supports_v1_1_counters)
{
values[i++] = Float8GetDatumFast(tmp.blk_read_time);
values[i++] = Float8GetDatumFast(tmp.blk_write_time);
}
! Assert(i == (sql_supports_v1_1_counters ?
! PG_STAT_STATEMENTS_COLS : PG_STAT_STATEMENTS_COLS_V1_0));
tuplestore_putvalues(tupstore, tupdesc, values, nulls);
}
--- 1209,1235 ----
values[i++] = Int64GetDatumFast(tmp.rows);
values[i++] = Int64GetDatumFast(tmp.shared_blks_hit);
values[i++] = Int64GetDatumFast(tmp.shared_blks_read);
! if (detected_version >= PGSS_V1_1)
values[i++] = Int64GetDatumFast(tmp.shared_blks_dirtied);
values[i++] = Int64GetDatumFast(tmp.shared_blks_written);
values[i++] = Int64GetDatumFast(tmp.local_blks_hit);
values[i++] = Int64GetDatumFast(tmp.local_blks_read);
! if (detected_version >= PGSS_V1_1)
values[i++] = Int64GetDatumFast(tmp.local_blks_dirtied);
values[i++] = Int64GetDatumFast(tmp.local_blks_written);
values[i++] = Int64GetDatumFast(tmp.temp_blks_read);
values[i++] = Int64GetDatumFast(tmp.temp_blks_written);
! if (detected_version >= PGSS_V1_1)
{
values[i++] = Float8GetDatumFast(tmp.blk_read_time);
values[i++] = Float8GetDatumFast(tmp.blk_write_time);
}
! Assert(i == (detected_version == PGSS_V1_0?
! PG_STAT_STATEMENTS_COLS_V1_0:
! detected_version == PGSS_V1_1?
! PG_STAT_STATEMENTS_COLS_V1_1:
! PG_STAT_STATEMENTS_COLS));
tuplestore_putvalues(tupstore, tupdesc, values, nulls);
}
diff --git a/contrib/pg_stat_statements/pg_stat_statements.control b/contrib/pg_stat_statements/pg_stat_statements.control
new file mode 100644
index 428fbb2..6ecf2b6
*** a/contrib/pg_stat_statements/pg_stat_statements.control
--- b/contrib/pg_stat_statements/pg_stat_statements.control
***************
*** 1,5 ****
# pg_stat_statements extension
comment = 'track execution statistics of all SQL statements executed'
! default_version = '1.1'
module_pathname = '$libdir/pg_stat_statements'
relocatable = true
--- 1,5 ----
# pg_stat_statements extension
comment = 'track execution statistics of all SQL statements executed'
! default_version = '1.2'
module_pathname = '$libdir/pg_stat_statements'
relocatable = true
diff --git a/doc/src/sgml/pgstatstatements.sgml b/doc/src/sgml/pgstatstatements.sgml
new file mode 100644
index c02fdf4..c607710
*** a/doc/src/sgml/pgstatstatements.sgml
--- b/doc/src/sgml/pgstatstatements.sgml
***************
*** 23,33 ****
<title>The <structname>pg_stat_statements</structname> View</title>
<para>
! The statistics gathered by the module are made available via a system view
! named <structname>pg_stat_statements</>. This view contains one row for
! each distinct query, database ID, and user ID (up to the maximum
! number of distinct statements that the module can track). The columns
! of the view are shown in <xref linkend="pgstatstatements-columns">.
</para>
<table id="pgstatstatements-columns">
--- 23,34 ----
<title>The <structname>pg_stat_statements</structname> View</title>
<para>
! The statistics gathered by the module are made available via a
! system view named <structname>pg_stat_statements</>. This view
! contains one row for each distinct database ID, user ID and query
! ID (up to the maximum number of distinct statements that the module
! can track). The columns of the view are shown in
! <xref linkend="pgstatstatements-columns">.
</para>
<table id="pgstatstatements-columns">
***************
*** 57,63 ****
<entry>OID of database in which the statement was executed</entry>
</row>
! <row>
<entry><structfield>query</structfield></entry>
<entry><type>text</type></entry>
<entry></entry>
--- 58,71 ----
<entry>OID of database in which the statement was executed</entry>
</row>
! <row>
! <entry><structfield>queryid</structfield></entry>
! <entry><type>bigint</type></entry>
! <entry></entry>
! <entry>Internal hash identifier, computed from the entry's post-parse-analysis tree</entry>
! </row>
!
! <row>
<entry><structfield>query</structfield></entry>
<entry><type>text</type></entry>
<entry></entry>
***************
*** 189,197 ****
</para>
<para>
! For security reasons, non-superusers are not allowed to see the text of
! queries executed by other users. They can see the statistics, however,
! if the view has been installed in their database.
</para>
<para>
--- 197,206 ----
</para>
<para>
! For security reasons, non-superusers are not allowed to see the SQL
! text or queryid of queries executed by other users. They can see
! the statistics, however, if the view has been installed in their
! database.
</para>
<para>
***************
*** 209,216 ****
When a constant's value has been ignored for purposes of matching the
query to other queries, the constant is replaced by <literal>?</literal>
in the <structname>pg_stat_statements</> display. The rest of the query
! text is that of the first query that had the particular hash value
! associated with the <structname>pg_stat_statements</> entry.
</para>
<para>
--- 218,226 ----
When a constant's value has been ignored for purposes of matching the
query to other queries, the constant is replaced by <literal>?</literal>
in the <structname>pg_stat_statements</> display. The rest of the query
! text is that of the first query that had the particular
! <structfield>queryid</> hash value associated with the
! <structname>pg_stat_statements</> entry.
</para>
<para>
***************
*** 223,232 ****
</para>
<para>
! Since the hash value is computed on the post-parse-analysis representation
! of the queries, the opposite is also possible: queries with identical texts
! might appear as separate entries, if they have different meanings as a
! result of factors such as different <varname>search_path</> settings.
</para>
</sect2>
--- 233,277 ----
</para>
<para>
! Since the <structfield>queryid</> hash value is computed on the
! post-parse-analysis representation of the queries, the opposite is
! also possible: queries with identical texts might appear as
! separate entries, if they have different meanings as a result of
! factors such as different <varname>search_path</> settings.
! </para>
!
! <para>
! Consumers of <literal>pg_stat_statements</> may wish to use
! <structfield>queryid</> (perhaps in composite with
! <structfield>dbid</> and <structfield>userid</>) as a more stable
! and reliable identifier for each entry than its query text.
! However, it is important to understand that there are only limited
! guarantees around the stability of the <structfield>queryid</> hash
! value. Since the identifier is derived from the
! post-parse-analysis tree, its value is a function of, among other
! things, the internal identifiers that comprise this representation.
! This has some counterintuitive implications. For example, a query
! against a table that is fingerprinted by
! <literal>pg_stat_statements</> will appear distinct to a
! subsequently executed query that a reasonable observer might judge
! to be a non-distinct, if in the interim the table was dropped and
! re-created. The hashing process is sensitive to difference in
! machine architecture and other facets of the platform.
! Furthermore, it is not safe to assume that <structfield>queryid</>
! will be stable across major versions of <productname>PostgreSQL</>.
! </para>
!
! <para>
! As a rule of thumb, an assumption of the stability or comparability
! of <structfield>querid</> values should be predicated on the the
! underlying catalog metadata and hash function implementation
! details exactly matching. Any two servers participating in any
! variety of replication based on physical WAL-replay can be expected
! to have identical <structfield>querid</> values for the same query.
! Logical replication schemes do not have replicas comparable in all
! relevant regards, and so <structfield>querid</> will not be a
! useful identifier for accumulating costs for the entire replica
! set. If in doubt, direct testing is recommended.
</para>
</sect2>
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
