Fix queryId calculation for queries with grouping clause (#665)

Problem description:
if 'pg_stat_statements' extension is enabled and a query containing GROUP BY
clause with ROLLUP, CUBE or GROUPING SETS or with GROUPING function is
executed, queryId calculation doesn't take into account these grouping clause
parameters and GROUPING functions. So, semantically different queries are
treated by pg_stat_statements as equivalent queries. Plus warning messages
appear during jumbling.
Also, jumbling logic doesn't handle properly queries with group_id() function
call and calls of functions with 'anytable' parameter.
First type of queries causes warnings.
Second type of query causes error message "unrecognized RTE kind: 7".

Expected correct behavior: queries with different grouping clauses or different
usage of the GROUPING function are treated by pg_stat_statements as different
queries, and no warning messages appear during the query execution.

Cause:
In JumbleExpr there is no handling for node tags T_GroupingClause,
T_GroupingFunc, T_GroupId and T_TableValueExpr. Jumble hashing is used
as queryId. Error message "unrecognized RTE kind: 7" caused by not handled
range table type RTE_TABLEFUNCTION in JumbleRangeTable.

Fix:
Handling logic for missed node tags was added.
According to comments in queryjumble.c, main guideline how to handle
query tree is: "Rule of thumb for what to include is that we should ignore
anything not semantically significant (such as alias names) as well as
anything that can be deduced from child nodes (else we'd just be double-
hashing that piece of information)."

For T_GroupingFunc we append to the jumble the list of
arguments. Field 'ngrpcols' is not appended to the jumble, because it can be
deduced from the list of groupsets in grouping clause. 'ngrpcols' is the
number of unique grouping attributes in grouping clause. Equivalent queries
must have the same groupsets in grouping clause. Thus they will have the same
'ngrpcols'. So, adding 'ngrpcols' to jumble is redundant.
Plus handling for the T_Integer tag was added, because it is required to parse
the list of arguments. List of arguments (field 'args' of structure
GroupingFunc) is a list of T_Integer. The T_Integer element is the index of
GROUPING function parameter inside the array of unique grouping attributes
from grouping clause. We need to jumble value of this T_Integer, because
changing parameter in GROUPING function changes this index (and definitely
changes semantic of the query).

For T_GroupingClause we append to the jumble the grouping type
and list of groupsets. Field 'location' is not appended to the jumble because
it is the textual location from parser and is not semantically significant.

For T_GroupId - this tag was added to switch case inside JumbleExpr to
suppress warning. The node tag for it was already handled. Struct GroupId
doesn't have any additional fields, that could be added to jumble.

For RTE_TABLEFUNCTION - 'functions' field of RangeTblEntry was added to the
jumble. RangeTblEntry in case of RTE_TABLEFUNCTION uses 'functions' and
'subquery' fields (other significant fields are null). But 'subquery' is
duplicated in TableValueExpr below, so do not jumble it.

For T_TableValueExpr - 'subquery' field of TableValueExpr (the subquery
that is inside "TABLE()" statement) was added to the jumble.

Changes from original commit:
1. Cases for T_GroupId and T_GroupingFunc are already present, no fix needed.
2. Case for T_Integer is not necessary since GPDB 7 uses T_IntList, removed.
3. T_GroupingClause is now named T_GroupingSet. Its case was not correct in
   GPDB 7, changed to match implementation from GPDB 6.
4. Test renamed to gp_pg_stat_statements, since there were already tests
   present in GPDB 7.
5. Currently the tests for pg_stat_statements do not set up
   shared_preload_libraries and they don't disable optimizer. Changed the new
   test to match existing ones, added a comment about optimizer to Makefile.
6. Fixed test output for GPDB 7. Notably, ROLLUP now outputs 1 row even if
   the table has no rows in it.

(cherry picked from commit fa44e50)

GNUmakefile.in

@@ -12,6 +12,7 @@ $(call recurse,all install,src config)
 
 all:
 	$(MAKE) -C contrib/auto_explain all
+	$(MAKE) -C contrib/pg_stat_statements all
 	$(MAKE) -C contrib/citext all
 	$(MAKE) -C contrib/file_fdw all
 	$(MAKE) -C contrib/postgres_fdw all
@@ -63,6 +64,7 @@ html man:
 
 install:
 	$(MAKE) -C contrib/auto_explain $@
+	$(MAKE) -C contrib/pg_stat_statements $@
 	$(MAKE) -C contrib/citext $@
 	$(MAKE) -C contrib/file_fdw $@
 	$(MAKE) -C contrib/postgres_fdw $@
@@ -182,6 +184,7 @@ $(call recurse,checkprep,  src/test src/pl src/interfaces/ecpg contrib src/bin g
 # recipe body).
 ICW_TARGETS  = src/test src/pl src/interfaces/gppc
 ICW_TARGETS += contrib/auto_explain contrib/citext contrib/btree_gin
+ICW_TARGETS += contrib/pg_stat_statements
 ICW_TARGETS += contrib/file_fdw contrib/postgres_fdw contrib/formatter_fixedwidth
 ICW_TARGETS += contrib/extprotocol contrib/dblink contrib/pg_trgm
 ICW_TARGETS += contrib/indexscan contrib/hstore contrib/ltree contrib/pgcrypto contrib/isn

contrib/pg_stat_statements/Makefile

@@ -14,9 +14,10 @@ PGFILEDESC = "pg_stat_statements - execution statistics of SQL statements"
 LDFLAGS_SL += $(filter -lm, $(LIBS))
 
 REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/pg_stat_statements/pg_stat_statements.conf
-REGRESS = pg_stat_statements olap_setup olap_group
+REGRESS = pg_stat_statements olap_setup olap_group gp_pg_stat_statements
 # Disabled because these tests require "shared_preload_libraries=pg_stat_statements",
 # which typical installcheck users do not have (e.g. buildfarm clients).
+# Only run with optimizer='off'
 NO_INSTALLCHECK = 1
 
 ifdef USE_PGXS

contrib/pg_stat_statements/expected/gp_pg_stat_statements.out

@@ -0,0 +1,142 @@
+CREATE TABLE t(a int, b text) DISTRIBUTED BY (a);
+-- Known issue: query is not added to pg_stat_statements statistics in
+-- case it is planned by GPORCA. So disable GPORCA during tests.
+SET optimizer=off;
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset 
+--------------------------
+
+(1 row)
+
+SELECT GROUPING (a) FROM t GROUP BY ROLLUP(a, b);
+ grouping 
+----------
+        1
+(1 row)
+
+-- launch not equivalent query
+SELECT GROUPING (b) FROM t GROUP BY ROLLUP(a, b);
+ grouping 
+----------
+        1
+(1 row)
+
+-- check group_id() in a query
+SELECT group_id() FROM t GROUP BY ROLLUP(a, b);
+ group_id 
+----------
+        0
+(1 row)
+
+-- check that queries have separate entries
+SELECT query, calls FROM pg_stat_statements ORDER BY query;
+                      query                       | calls 
+--------------------------------------------------+-------
+ SELECT group_id() FROM t GROUP BY ROLLUP(a, b)   |     1
+ SELECT GROUPING (a) FROM t GROUP BY ROLLUP(a, b) |     1
+ SELECT GROUPING (b) FROM t GROUP BY ROLLUP(a, b) |     1
+ SELECT pg_stat_statements_reset()                |     1
+(4 rows)
+
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset 
+--------------------------
+
+(1 row)
+
+-- check that different grouping options result in separate entries
+SELECT COUNT (*) FROM t GROUP BY ROLLUP(a, b);
+ count 
+-------
+     0
+(1 row)
+
+SELECT COUNT (*) FROM t GROUP BY CUBE(a, b);
+ count 
+-------
+     0
+(1 row)
+
+SELECT COUNT (*) FROM t GROUP BY GROUPING SETS(a, b);
+ count 
+-------
+(0 rows)
+
+SELECT COUNT (*) FROM t GROUP BY GROUPING SETS((a), (a, b));
+ count 
+-------
+(0 rows)
+
+SELECT COUNT (*) FROM t GROUP BY a, b;
+ count 
+-------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query;
+                            query                            | calls 
+-------------------------------------------------------------+-------
+ SELECT COUNT (*) FROM t GROUP BY a, b                       |     1
+ SELECT COUNT (*) FROM t GROUP BY CUBE(a, b)                 |     1
+ SELECT COUNT (*) FROM t GROUP BY GROUPING SETS((a), (a, b)) |     1
+ SELECT COUNT (*) FROM t GROUP BY GROUPING SETS(a, b)        |     1
+ SELECT COUNT (*) FROM t GROUP BY ROLLUP(a, b)               |     1
+ SELECT pg_stat_statements_reset()                           |     1
+(6 rows)
+
+-- check several parameters options in ROLLUP
+-- all should result in separate entries
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset 
+--------------------------
+
+(1 row)
+
+SELECT COUNT (*) FROM t GROUP BY ROLLUP(a, b);
+ count 
+-------
+     0
+(1 row)
+
+SELECT COUNT (*) FROM t GROUP BY ROLLUP(b);
+ count 
+-------
+     0
+(1 row)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query;
+                     query                     | calls 
+-----------------------------------------------+-------
+ SELECT COUNT (*) FROM t GROUP BY ROLLUP(a, b) |     1
+ SELECT COUNT (*) FROM t GROUP BY ROLLUP(b)    |     1
+ SELECT pg_stat_statements_reset()             |     1
+(3 rows)
+
+--- check anytable parameter for a function
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset 
+--------------------------
+
+(1 row)
+
+-- call of anytable_out will cause an error,
+-- thus prevent actual call by adding FALSE condition
+SELECT * FROM anytable_out(TABLE(SELECT * FROM t)) WHERE 1 = 0;
+ anytable_out 
+--------------
+(0 rows)
+
+SELECT * FROM anytable_out(TABLE(SELECT * FROM t WHERE a=0)) WHERE 1 = 0;
+ anytable_out 
+--------------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query;
+                                    query                                    | calls 
+-----------------------------------------------------------------------------+-------
+ SELECT * FROM anytable_out(TABLE(SELECT * FROM t)) WHERE $1 = $2            |     1
+ SELECT * FROM anytable_out(TABLE(SELECT * FROM t WHERE a=$1)) WHERE $2 = $3 |     1
+ SELECT pg_stat_statements_reset()                                           |     1
+(3 rows)
+
+RESET optimizer;
+DROP TABLE t;

contrib/pg_stat_statements/sql/gp_pg_stat_statements.sql

@@ -0,0 +1,54 @@
+-- start_ignore
+CREATE EXTENSION IF NOT EXISTS pg_stat_statements;
+DROP TABLE IF EXISTS t;
+-- end_ignore
+CREATE TABLE t(a int, b text) DISTRIBUTED BY (a);
+
+-- Known issue: query is not added to pg_stat_statements statistics in
+-- case it is planned by GPORCA. So disable GPORCA during tests.
+SET optimizer=off;
+
+SELECT pg_stat_statements_reset();
+
+SELECT GROUPING (a) FROM t GROUP BY ROLLUP(a, b);
+-- launch not equivalent query
+SELECT GROUPING (b) FROM t GROUP BY ROLLUP(a, b);
+-- check group_id() in a query
+SELECT group_id() FROM t GROUP BY ROLLUP(a, b);
+
+-- check that queries have separate entries
+SELECT query, calls FROM pg_stat_statements ORDER BY query;
+
+SELECT pg_stat_statements_reset();
+
+-- check that different grouping options result in separate entries
+SELECT COUNT (*) FROM t GROUP BY ROLLUP(a, b);
+SELECT COUNT (*) FROM t GROUP BY CUBE(a, b);
+SELECT COUNT (*) FROM t GROUP BY GROUPING SETS(a, b);
+SELECT COUNT (*) FROM t GROUP BY GROUPING SETS((a), (a, b));
+SELECT COUNT (*) FROM t GROUP BY a, b;
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query;
+
+-- check several parameters options in ROLLUP
+-- all should result in separate entries
+SELECT pg_stat_statements_reset();
+
+SELECT COUNT (*) FROM t GROUP BY ROLLUP(a, b);
+SELECT COUNT (*) FROM t GROUP BY ROLLUP(b);
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query;
+
+--- check anytable parameter for a function
+SELECT pg_stat_statements_reset();
+
+-- call of anytable_out will cause an error,
+-- thus prevent actual call by adding FALSE condition
+SELECT * FROM anytable_out(TABLE(SELECT * FROM t)) WHERE 1 = 0;
+SELECT * FROM anytable_out(TABLE(SELECT * FROM t WHERE a=0)) WHERE 1 = 0;
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query;
+
+RESET optimizer;
+
+DROP TABLE t;

src/backend/utils/misc/queryjumble.c

@@ -185,6 +185,7 @@ JumbleRangeTable(JumbleState *jstate, List *rtable)
 				APP_JUMB(rte->jointype);
 				break;
 			case RTE_FUNCTION:
+			case RTE_TABLEFUNCTION:
 				JumbleExpr(jstate, (Node *) rte->functions);
 				break;
 			case RTE_TABLEFUNC:
@@ -646,6 +647,7 @@ JumbleExpr(JumbleState *jstate, Node *node)
 			{
 				GroupingSet *gsnode = (GroupingSet *) node;
 
+				APP_JUMB(gsnode->kind);
 				JumbleExpr(jstate, (Node *) gsnode->content);
 			}
 			break;
@@ -706,6 +708,13 @@ JumbleExpr(JumbleState *jstate, Node *node)
 				JumbleExpr(jstate, (Node *) tsc->repeatable);
 			}
 			break;
+		case T_TableValueExpr:
+			{
+				TableValueExpr *tve = (TableValueExpr *) node;
+
+				JumbleQueryInternal(jstate, (Query *) tve->subquery);
+			}
+			break;
 		default:
 			/* Only a warning, since we can stumble along anyway */
 			elog(WARNING, "unrecognized node type: %d",