pg_surgery: Fix infinite loop on large TID arrays

heap_force_common() tracked the current position in the caller-supplied
tid[] using OffsetNumber, which is only 16 bits wide, so when the array
held more than 65535 entries, the updated index wrapped around and the
outer loop never reached the exit condition.  A SQL call with a
sufficiently large TID array would then run until interrupted.

Fix by tracking the tid[] position using int instead of OffsetNumber.
A regress case based on the report is included.

Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Reported-by: Yuelin Wang <1217816127@qq.com>
Backpatch-through: 14
Bug: #19607
Discussion: https://postgr.es/m/19607-2f256a66481c514b@postgresql.org

diff --git a/contrib/pg_surgery/expected/heap_surgery.out b/contrib/pg_surgery/expected/heap_surgery.out
index df7d13b09086f4aa1dac042210cca6b0bde46950..42586137d88cdc9dd34847678caf76f876d9bd09 100644 (file)
--- a/contrib/pg_surgery/expected/heap_surgery.out
+++ b/contrib/pg_surgery/expected/heap_surgery.out
@@ -134,6 +134,23 @@ select heap_force_kill('htab2'::regclass, ARRAY['(0, 3)']::tid[]);
  
 (1 row)
 
+-- a tid[] larger than 65535 entries must still finish
+create temp table htab3(a int);
+insert into htab3 values (1);
+select heap_force_kill(
+       'htab3'::regclass,
+       array(select '(0,1)'::tid from generate_series(1, 65536)));
+ heap_force_kill 
+-----------------
+ 
+(1 row)
+
+select count(*) from htab3;
+ count 
+-------
+     0
+(1 row)
+
 -- materialized view.
 -- note that we don't commit the transaction, so autovacuum can't interfere.
 begin;

diff --git a/contrib/pg_surgery/heap_surgery.c b/contrib/pg_surgery/heap_surgery.c
index 181b7d1e2106353f268a91ff1cef769c7b383aee..51f3f3c49eb532f54a6e9952d5bc78af26d3bb46 100644 (file)
--- a/contrib/pg_surgery/heap_surgery.c
+++ b/contrib/pg_surgery/heap_surgery.c
@@ -44,7 +44,7 @@ static Datum heap_force_common(FunctionCallInfo fcinfo,
                                                           HeapTupleForceOption heap_force_opt);
 static void sanity_check_tid_array(ArrayType *ta, int *ntids);
 static BlockNumber find_tids_one_page(ItemPointer tids, int ntids,
-                                                                         OffsetNumber *next_start_ptr);
+                                                                         int *next_start_ptr);
 
 /*-------------------------------------------------------------------------
  * heap_force_kill()
@@ -91,7 +91,7 @@ heap_force_common(FunctionCallInfo fcinfo, HeapTupleForceOption heap_force_opt)
        int                     ntids,
                                nblocks;
        Relation        rel;
-       OffsetNumber curr_start_ptr,
+       int                     curr_start_ptr,
                                next_start_ptr;
        bool            include_this_tid[MaxHeapTuplesPerPage];
 
@@ -413,7 +413,7 @@ sanity_check_tid_array(ArrayType *ta, int *ntids)
  * ------------------------------------------------------------------------
  */
 static BlockNumber
-find_tids_one_page(ItemPointer tids, int ntids, OffsetNumber *next_start_ptr)
+find_tids_one_page(ItemPointer tids, int ntids, int *next_start_ptr)
 {
        int                     i;
        BlockNumber prev_blkno,

diff --git a/contrib/pg_surgery/sql/heap_surgery.sql b/contrib/pg_surgery/sql/heap_surgery.sql
index 6526b27535de4fb69a5517a6cde95de0b0478d16..c4e933da13aea68c93abbefe40e7e3ed0b4f5d4e 100644 (file)
--- a/contrib/pg_surgery/sql/heap_surgery.sql
+++ b/contrib/pg_surgery/sql/heap_surgery.sql
@@ -65,6 +65,14 @@ select heap_force_kill('htab2'::regclass, ARRAY[NULL]::tid[]);
 -- but we should be able to kill the one tuple we have
 select heap_force_kill('htab2'::regclass, ARRAY['(0, 3)']::tid[]);
 
+-- a tid[] larger than 65535 entries must still finish
+create temp table htab3(a int);
+insert into htab3 values (1);
+select heap_force_kill(
+       'htab3'::regclass,
+       array(select '(0,1)'::tid from generate_series(1, 65536)));
+select count(*) from htab3;
+
 -- materialized view.
 -- note that we don't commit the transaction, so autovacuum can't interfere.
 begin;