From cef82eda1464193ab84a58610a388572d456c8c5 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Tue, 15 Oct 2019 10:40:13 -0700 Subject: [PATCH] Fix CLUSTER on expression indexes. Since the introduction of different slot types, in 1a0586de3657, we create a virtual slot in tuplesort_begin_cluster(). While that looks right, it unfortunately doesn't actually work, as ExecStoreHeapTuple() is used to store tuples in the slot. Unfortunately no regression tests for CLUSTER on expression indexes existed so far. Fix the slot type, and add bare bones tests for CLUSTER on expression indexes. Reported-By: Justin Pryzby Author: Andres Freund Discussion: https://postgr.es/m/20191011210320.GS10470@telsasoft.com Backpatch: 12, like 1a0586de3657 --- src/backend/utils/sort/tuplesort.c | 2 +- src/test/regress/expected/cluster.out | 107 ++++++++++++++++++++++++++ src/test/regress/sql/cluster.sql | 37 +++++++++ 3 files changed, 145 insertions(+), 1 deletion(-) diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c index ab55e69975..a507bd2498 100644 --- a/src/backend/utils/sort/tuplesort.c +++ b/src/backend/utils/sort/tuplesort.c @@ -933,7 +933,7 @@ tuplesort_begin_cluster(TupleDesc tupDesc, * scantuple has to point to that slot, too. */ state->estate = CreateExecutorState(); - slot = MakeSingleTupleTableSlot(tupDesc, &TTSOpsVirtual); + slot = MakeSingleTupleTableSlot(tupDesc, &TTSOpsHeapTuple); econtext = GetPerTupleExprContext(state->estate); econtext->ecxt_scantuple = slot; } diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 2bb62212ea..bdae8fe00c 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -466,10 +466,117 @@ where row(hundred, thousand, tenthous) <= row(lhundred, lthousand, ltenthous); reset enable_indexscan; reset maintenance_work_mem; +-- test CLUSTER on expression index +CREATE TABLE clstr_expression(id serial primary key, a int, b text COLLATE "C"); +INSERT INTO clstr_expression(a, b) SELECT g.i % 42, 'prefix'||g.i FROM generate_series(1, 133) g(i); +CREATE INDEX clstr_expression_minus_a ON clstr_expression ((-a), b); +CREATE INDEX clstr_expression_upper_b ON clstr_expression ((upper(b))); +-- verify indexes work before cluster +BEGIN; +SET LOCAL enable_seqscan = false; +EXPLAIN (COSTS OFF) SELECT * FROM clstr_expression WHERE upper(b) = 'PREFIX3'; + QUERY PLAN +--------------------------------------------------------------- + Index Scan using clstr_expression_upper_b on clstr_expression + Index Cond: (upper(b) = 'PREFIX3'::text) +(2 rows) + +SELECT * FROM clstr_expression WHERE upper(b) = 'PREFIX3'; + id | a | b +----+---+--------- + 3 | 3 | prefix3 +(1 row) + +EXPLAIN (COSTS OFF) SELECT * FROM clstr_expression WHERE -a = -3 ORDER BY -a, b; + QUERY PLAN +--------------------------------------------------------------- + Index Scan using clstr_expression_minus_a on clstr_expression + Index Cond: ((- a) = '-3'::integer) +(2 rows) + +SELECT * FROM clstr_expression WHERE -a = -3 ORDER BY -a, b; + id | a | b +-----+---+----------- + 129 | 3 | prefix129 + 3 | 3 | prefix3 + 45 | 3 | prefix45 + 87 | 3 | prefix87 +(4 rows) + +COMMIT; +-- and after clustering on clstr_expression_minus_a +CLUSTER clstr_expression USING clstr_expression_minus_a; +BEGIN; +SET LOCAL enable_seqscan = false; +EXPLAIN (COSTS OFF) SELECT * FROM clstr_expression WHERE upper(b) = 'PREFIX3'; + QUERY PLAN +--------------------------------------------------------------- + Index Scan using clstr_expression_upper_b on clstr_expression + Index Cond: (upper(b) = 'PREFIX3'::text) +(2 rows) + +SELECT * FROM clstr_expression WHERE upper(b) = 'PREFIX3'; + id | a | b +----+---+--------- + 3 | 3 | prefix3 +(1 row) + +EXPLAIN (COSTS OFF) SELECT * FROM clstr_expression WHERE -a = -3 ORDER BY -a, b; + QUERY PLAN +--------------------------------------------------------------- + Index Scan using clstr_expression_minus_a on clstr_expression + Index Cond: ((- a) = '-3'::integer) +(2 rows) + +SELECT * FROM clstr_expression WHERE -a = -3 ORDER BY -a, b; + id | a | b +-----+---+----------- + 129 | 3 | prefix129 + 3 | 3 | prefix3 + 45 | 3 | prefix45 + 87 | 3 | prefix87 +(4 rows) + +COMMIT; +-- and after clustering on clstr_expression_upper_b +CLUSTER clstr_expression USING clstr_expression_upper_b; +BEGIN; +SET LOCAL enable_seqscan = false; +EXPLAIN (COSTS OFF) SELECT * FROM clstr_expression WHERE upper(b) = 'PREFIX3'; + QUERY PLAN +--------------------------------------------------------------- + Index Scan using clstr_expression_upper_b on clstr_expression + Index Cond: (upper(b) = 'PREFIX3'::text) +(2 rows) + +SELECT * FROM clstr_expression WHERE upper(b) = 'PREFIX3'; + id | a | b +----+---+--------- + 3 | 3 | prefix3 +(1 row) + +EXPLAIN (COSTS OFF) SELECT * FROM clstr_expression WHERE -a = -3 ORDER BY -a, b; + QUERY PLAN +--------------------------------------------------------------- + Index Scan using clstr_expression_minus_a on clstr_expression + Index Cond: ((- a) = '-3'::integer) +(2 rows) + +SELECT * FROM clstr_expression WHERE -a = -3 ORDER BY -a, b; + id | a | b +-----+---+----------- + 129 | 3 | prefix129 + 3 | 3 | prefix3 + 45 | 3 | prefix45 + 87 | 3 | prefix87 +(4 rows) + +COMMIT; -- clean up DROP TABLE clustertest; DROP TABLE clstr_1; DROP TABLE clstr_2; DROP TABLE clstr_3; DROP TABLE clstr_4; +DROP TABLE clstr_expression; DROP USER regress_clstr_user; diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index 522bfeead4..188183647c 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -222,10 +222,47 @@ where row(hundred, thousand, tenthous) <= row(lhundred, lthousand, ltenthous); reset enable_indexscan; reset maintenance_work_mem; +-- test CLUSTER on expression index +CREATE TABLE clstr_expression(id serial primary key, a int, b text COLLATE "C"); +INSERT INTO clstr_expression(a, b) SELECT g.i % 42, 'prefix'||g.i FROM generate_series(1, 133) g(i); +CREATE INDEX clstr_expression_minus_a ON clstr_expression ((-a), b); +CREATE INDEX clstr_expression_upper_b ON clstr_expression ((upper(b))); + +-- verify indexes work before cluster +BEGIN; +SET LOCAL enable_seqscan = false; +EXPLAIN (COSTS OFF) SELECT * FROM clstr_expression WHERE upper(b) = 'PREFIX3'; +SELECT * FROM clstr_expression WHERE upper(b) = 'PREFIX3'; +EXPLAIN (COSTS OFF) SELECT * FROM clstr_expression WHERE -a = -3 ORDER BY -a, b; +SELECT * FROM clstr_expression WHERE -a = -3 ORDER BY -a, b; +COMMIT; + +-- and after clustering on clstr_expression_minus_a +CLUSTER clstr_expression USING clstr_expression_minus_a; +BEGIN; +SET LOCAL enable_seqscan = false; +EXPLAIN (COSTS OFF) SELECT * FROM clstr_expression WHERE upper(b) = 'PREFIX3'; +SELECT * FROM clstr_expression WHERE upper(b) = 'PREFIX3'; +EXPLAIN (COSTS OFF) SELECT * FROM clstr_expression WHERE -a = -3 ORDER BY -a, b; +SELECT * FROM clstr_expression WHERE -a = -3 ORDER BY -a, b; +COMMIT; + +-- and after clustering on clstr_expression_upper_b +CLUSTER clstr_expression USING clstr_expression_upper_b; +BEGIN; +SET LOCAL enable_seqscan = false; +EXPLAIN (COSTS OFF) SELECT * FROM clstr_expression WHERE upper(b) = 'PREFIX3'; +SELECT * FROM clstr_expression WHERE upper(b) = 'PREFIX3'; +EXPLAIN (COSTS OFF) SELECT * FROM clstr_expression WHERE -a = -3 ORDER BY -a, b; +SELECT * FROM clstr_expression WHERE -a = -3 ORDER BY -a, b; +COMMIT; + -- clean up DROP TABLE clustertest; DROP TABLE clstr_1; DROP TABLE clstr_2; DROP TABLE clstr_3; DROP TABLE clstr_4; +DROP TABLE clstr_expression; + DROP USER regress_clstr_user;