Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix queryId calculation for queries with grouping clause #665

Merged
merged 17 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions GNUmakefile.in
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ $(call recurse,all install,src config)
all:
$(MAKE) -C contrib/amcheck 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/formatter all
Expand Down Expand Up @@ -67,6 +68,7 @@ html man:
install:
$(MAKE) -C contrib/amcheck $@
$(MAKE) -C contrib/auto_explain $@
$(MAKE) -C contrib/pg_stat_statements $@
$(MAKE) -C contrib/citext $@
#$(MAKE) -C contrib/file_fdw $@ # GPDB_91_MERGE_FIXME: disable installation until it's officially supported.
$(MAKE) -C contrib/formatter $@
Expand Down Expand Up @@ -174,6 +176,7 @@ $(call recurse,check-world,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/amcheck contrib/auto_explain contrib/citext
ICW_TARGETS += contrib/pg_stat_statements
ICW_TARGETS += contrib/formatter_fixedwidth
ICW_TARGETS += contrib/extprotocol
ICW_TARGETS += contrib/pg_trgm contrib/btree_gin contrib/isn
Expand Down
2 changes: 2 additions & 0 deletions contrib/pg_stat_statements/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ 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

REGRESS = test_pg_stat_statements

ifdef USE_PGXS
PG_CONFIG = pg_config
PGXS := $(shell $(PG_CONFIG) --pgxs)
Expand Down
135 changes: 135 additions & 0 deletions contrib/pg_stat_statements/expected/test_pg_stat_statements.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
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
----------
(0 rows)

-- launch not equivalent query
SELECT GROUPING (b) FROM t GROUP BY ROLLUP(a, b);
grouping
----------
(0 rows)

-- check group_id() in a query
SELECT group_id() FROM t GROUP BY ROLLUP(a, b);
?column?
----------
(0 rows)

-- 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
RekGRpth marked this conversation as resolved.
Show resolved Hide resolved
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 rows)

SELECT COUNT (*) FROM t GROUP BY CUBE(a, b);
count
-------
(0 rows)

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 rows)

SELECT COUNT (*) FROM t GROUP BY ROLLUP(b);
count
-------
(0 rows)

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
SELECT * FROM anytable_out(TABLE(SELECT * FROM t WHERE a=?)) WHERE ? = ?; | 1
SELECT pg_stat_statements_reset(); | 1
(3 rows)

RESET optimizer;
DROP TABLE t;
61 changes: 61 additions & 0 deletions contrib/pg_stat_statements/sql/test_pg_stat_statements.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
-- start_ignore
\! gpconfig -c shared_preload_libraries -v 'pg_stat_statements';
\! gpstop -raq -M fast;
\c
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,
RekGRpth marked this conversation as resolved.
Show resolved Hide resolved
-- 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;
-- start_ignore
\! gpconfig -r shared_preload_libraries;
\! gpstop -raq -M fast;
-- end_ignore
29 changes: 29 additions & 0 deletions src/backend/utils/misc/queryjumble.c
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,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_VALUES:
Expand Down Expand Up @@ -597,6 +598,34 @@ JumbleExpr(JumbleState *jstate, Node *node)
JumbleExpr(jstate, rtfunc->funcexpr);
}
break;
case T_GroupId:
/*
* Struct GroupId has only the node tag, already added to the jumble
* in the code above. So just leave case empty to suppress warning.
*/
break;
case T_Integer:
APP_JUMB(intVal(node));
break;
case T_TableValueExpr:
{
TableValueExpr *tve = (TableValueExpr *) node;
JumbleQueryInternal(jstate, (Query *) tve->subquery);
}
break;
case T_GroupingClause:
{
GroupingClause *gc = (GroupingClause *) node;
APP_JUMB(gc->groupType);
JumbleExpr(jstate, (Node *) gc->groupsets);
}
break;
case T_GroupingFunc:
{
GroupingFunc *gf = (GroupingFunc *) node;
JumbleExpr(jstate, (Node *) gf->args);
}
break;
default:
/* Only a warning, since we can stumble along anyway */
elog(WARNING, "unrecognized node type: %d",
Expand Down