Fix reference leak on AO/AOCS partition tables with unique index. (#649)

* Fix reference leak on AO/AOCS partition tables with unique index.

Fix #557.

This is introduced by GPDB commit ed364586b99.
For AO/AOCS partition tables, it's possible to update across partitions.
And it does have deleteDesc and uniqueCheckDesc if there were.
The DeleteDesc and InsertDesc are not enough to decide if the partition
has a update operation or not.
Some partitions have visimap but some not, clean them if possible.

Authored-by: Zhang Mingli [email protected]

src/backend/access/aocs/aocsam_handler.c

@@ -338,6 +338,20 @@ aoco_dml_finish(Relation relation, CmdType operation)
 			AppendOnlyVisimap_Finish_forUniquenessChecks(state->uniqueCheckDesc->visimap);
 			pfree(state->uniqueCheckDesc->visimap);
 		}
+		else
+		{
+			/*
+			 * Github issue: https://github.com/cloudberrydb/cloudberrydb/issues/557
+			 *
+			 * For partition tables, it's possible to update across partitions.
+			 * And it does have deleteDesc and uniqueCheckDesc if there were.
+			 * Some partitions have visimap but some not, clean them if possible.
+			 */
+			if (state->uniqueCheckDesc->visimap)
+			{
+				AppendOnlyVisimapStore_Finish(&state->uniqueCheckDesc->visimap->visimapStore, AccessShareLock);
+			}
+		}
 		state->uniqueCheckDesc->visimap = NULL;
 		state->uniqueCheckDesc->visiMapDelete = NULL;
 

src/backend/access/appendonly/appendonlyam_handler.c

@@ -308,6 +308,20 @@ appendonly_dml_finish(Relation relation, CmdType operation)
 			AppendOnlyVisimap_Finish_forUniquenessChecks(state->uniqueCheckDesc->visimap);
 			pfree(state->uniqueCheckDesc->visimap);
 		}
+		else
+		{
+			/*
+			 * Github issue: https://github.com/cloudberrydb/cloudberrydb/issues/557
+			 *
+			 * For partition tables, it's possible to update across partitions.
+			 * And it does have deleteDesc and uniqueCheckDesc if there were.
+			 * Some partitions have visimap but some not, clean them if possible.
+			 */
+			if (state->uniqueCheckDesc->visimap)
+			{
+				AppendOnlyVisimapStore_Finish(&state->uniqueCheckDesc->visimap->visimapStore, AccessShareLock);
+			}
+		}
 		state->uniqueCheckDesc->visimap = NULL;
 		state->uniqueCheckDesc->visiMapDelete = NULL;
 

src/test/regress/expected/.gitignore

@@ -63,3 +63,4 @@
 /directory_table.out
 /directory_table_optimizer.out
 /tag.out
+/ao_unique_index_partition.out

src/test/regress/greenplum_schedule

@@ -234,6 +234,8 @@ test: uao_ddl/alter_drop_allcol_row uao_ddl/alter_drop_allcol_column uao_ddl/alt
 
 test: uao_dml/uao_dml_cursor_row uao_dml/uao_dml_select_row uao_dml/uao_dml_cursor_column uao_dml/uao_dml_select_column uao_dml/uao_dml_unique_index_delete_row uao_dml/uao_dml_unique_index_delete_column uao_dml/uao_dml_unique_index_update_row uao_dml/uao_dml_unique_index_update_column
 
+# test ao partition unique index update
+test: ao_unique_index_partition
 
 # disable autovacuum for the test
 test: disable_autovacuum

src/test/regress/input/ao_unique_index_partition.source

@@ -0,0 +1,164 @@
+--
+-- Github issue: https://github.com/cloudberrydb/cloudberrydb/issues/557
+-- Test update on ao partition tables with unique index.
+--
+create schema t_issue_557;
+set search_path to t_issue_557;
+CREATE TABLE IF NOT EXISTS t_issue_557_ao
+(
+product_id INT,
+is_audited BOOLEAN DEFAULT FALSE,
+quantity SMALLINT,
+total_sales BIGINT,
+unit_price REAL,
+discount DOUBLE PRECISION,
+description TEXT,
+sale_date TIMESTAMP,
+order_date DATE,
+status CHAR(10),
+customer_name VARCHAR(20),
+price DECIMAL(20, 10)
+)
+DISTRIBUTED BY (product_id)
+PARTITION BY HASH(description);
+
+CREATE TABLE t_issue_557_ao_part1
+PARTITION OF t_issue_557_ao
+FOR VALUES WITH (MODULUS 3, REMAINDER 0)
+WITH (appendonly=true);
+
+CREATE TABLE t_issue_557_ao_part2
+PARTITION OF t_issue_557_ao
+FOR VALUES WITH (MODULUS 3, REMAINDER 1)
+WITH (appendonly=true);
+
+CREATE TABLE t_issue_557_ao_part3
+PARTITION OF t_issue_557_ao
+FOR VALUES WITH (MODULUS 3, REMAINDER 2)
+WITH (appendonly=true);
+-- Create Indexes
+
+-- Unique
+CREATE UNIQUE INDEX on t_issue_557_ao(product_id,description);
+
+INSERT INTO t_issue_557_ao (
+product_id,
+is_audited,
+description,
+status
+)
+SELECT
+x.id, -- product_id
+FALSE,
+'Product description ' || x.id, -- description
+'Closed'
+FROM (
+SELECT * FROM generate_series(1, 20) AS id
+) AS x;
+
+UPDATE t_issue_557_ao
+SET status = 'Closed',
+description = description || ' Audited';
+
+DELETE FROM t_issue_557_ao;
+
+INSERT INTO t_issue_557_ao (
+product_id,
+is_audited,
+description,
+status
+)
+SELECT
+x.id, -- product_id
+FALSE,
+'Product description ' || x.id, -- description
+'Closed'
+FROM (
+SELECT * FROM generate_series(1, 20) AS id
+) AS x;
+
+UPDATE t_issue_557_ao
+SET status = 'Closed',
+description = description || ' Audited';
+
+-- AOCO
+CREATE TABLE IF NOT EXISTS t_issue_557_aocs
+(
+product_id INT,
+is_audited BOOLEAN DEFAULT FALSE,
+quantity SMALLINT,
+total_sales BIGINT,
+unit_price REAL,
+discount DOUBLE PRECISION,
+description TEXT,
+sale_date TIMESTAMP,
+order_date DATE,
+status CHAR(10),
+customer_name VARCHAR(20),
+price DECIMAL(20, 10)
+)
+DISTRIBUTED BY (product_id)
+PARTITION BY HASH(description);
+
+CREATE TABLE t_issue_557_aocs_part1
+PARTITION OF t_issue_557_aocs
+FOR VALUES WITH (MODULUS 3, REMAINDER 0)
+WITH (appendonly=true, orientation=column);
+
+CREATE TABLE t_issue_557_aocs_part2
+PARTITION OF t_issue_557_aocs
+FOR VALUES WITH (MODULUS 3, REMAINDER 1)
+WITH (appendonly=true, orientation=column);
+
+CREATE TABLE t_issue_557_aocs_part3
+PARTITION OF t_issue_557_aocs
+FOR VALUES WITH (MODULUS 3, REMAINDER 2)
+WITH (appendonly=true, orientation=column);
+-- Create Indexes
+
+-- Unique
+CREATE UNIQUE INDEX on t_issue_557_aocs(product_id,description);
+
+INSERT INTO t_issue_557_aocs (
+product_id,
+is_audited,
+description,
+status
+)
+SELECT
+x.id, -- product_id
+FALSE,
+'Product description ' || x.id, -- description
+'Closed'
+FROM (
+SELECT * FROM generate_series(1, 20) AS id
+) AS x;
+
+UPDATE t_issue_557_aocs
+SET status = 'Closed',
+description = description || ' Audited';
+
+DELETE FROM t_issue_557_aocs;
+
+INSERT INTO t_issue_557_aocs (
+product_id,
+is_audited,
+description,
+status
+)
+SELECT
+x.id, -- product_id
+FALSE,
+'Product description ' || x.id, -- description
+'Closed'
+FROM (
+SELECT * FROM generate_series(1, 20) AS id
+) AS x;
+
+UPDATE t_issue_557_aocs
+SET status = 'Closed',
+description = description || ' Audited';
+
+-- start_ignore
+drop schema t_issue_557 cascade;
+-- end_ignore

src/test/regress/output/ao_unique_index_partition.source

@@ -0,0 +1,150 @@
+--
+-- Github issue: https://github.com/cloudberrydb/cloudberrydb/issues/557
+-- Test update on ao partition tables with unique index.
+--
+create schema t_issue_557;
+set search_path to t_issue_557;
+CREATE TABLE IF NOT EXISTS t_issue_557_ao
+(
+product_id INT,
+is_audited BOOLEAN DEFAULT FALSE,
+quantity SMALLINT,
+total_sales BIGINT,
+unit_price REAL,
+discount DOUBLE PRECISION,
+description TEXT,
+sale_date TIMESTAMP,
+order_date DATE,
+status CHAR(10),
+customer_name VARCHAR(20),
+price DECIMAL(20, 10)
+)
+DISTRIBUTED BY (product_id)
+PARTITION BY HASH(description);
+CREATE TABLE t_issue_557_ao_part1
+PARTITION OF t_issue_557_ao
+FOR VALUES WITH (MODULUS 3, REMAINDER 0)
+WITH (appendonly=true);
+NOTICE:  table has parent, setting distribution columns to match parent table
+CREATE TABLE t_issue_557_ao_part2
+PARTITION OF t_issue_557_ao
+FOR VALUES WITH (MODULUS 3, REMAINDER 1)
+WITH (appendonly=true);
+NOTICE:  table has parent, setting distribution columns to match parent table
+CREATE TABLE t_issue_557_ao_part3
+PARTITION OF t_issue_557_ao
+FOR VALUES WITH (MODULUS 3, REMAINDER 2)
+WITH (appendonly=true);
+NOTICE:  table has parent, setting distribution columns to match parent table
+-- Create Indexes
+-- Unique
+CREATE UNIQUE INDEX on t_issue_557_ao(product_id,description);
+INSERT INTO t_issue_557_ao (
+product_id,
+is_audited,
+description,
+status
+)
+SELECT
+x.id, -- product_id
+FALSE,
+'Product description ' || x.id, -- description
+'Closed'
+FROM (
+SELECT * FROM generate_series(1, 20) AS id
+) AS x;
+UPDATE t_issue_557_ao
+SET status = 'Closed',
+description = description || ' Audited';
+DELETE FROM t_issue_557_ao;
+INSERT INTO t_issue_557_ao (
+product_id,
+is_audited,
+description,
+status
+)
+SELECT
+x.id, -- product_id
+FALSE,
+'Product description ' || x.id, -- description
+'Closed'
+FROM (
+SELECT * FROM generate_series(1, 20) AS id
+) AS x;
+UPDATE t_issue_557_ao
+SET status = 'Closed',
+description = description || ' Audited';
+-- AOCO
+CREATE TABLE IF NOT EXISTS t_issue_557_aocs
+(
+product_id INT,
+is_audited BOOLEAN DEFAULT FALSE,
+quantity SMALLINT,
+total_sales BIGINT,
+unit_price REAL,
+discount DOUBLE PRECISION,
+description TEXT,
+sale_date TIMESTAMP,
+order_date DATE,
+status CHAR(10),
+customer_name VARCHAR(20),
+price DECIMAL(20, 10)
+)
+DISTRIBUTED BY (product_id)
+PARTITION BY HASH(description);
+CREATE TABLE t_issue_557_aocs_part1
+PARTITION OF t_issue_557_aocs
+FOR VALUES WITH (MODULUS 3, REMAINDER 0)
+WITH (appendonly=true, orientation=column);
+NOTICE:  table has parent, setting distribution columns to match parent table
+CREATE TABLE t_issue_557_aocs_part2
+PARTITION OF t_issue_557_aocs
+FOR VALUES WITH (MODULUS 3, REMAINDER 1)
+WITH (appendonly=true, orientation=column);
+NOTICE:  table has parent, setting distribution columns to match parent table
+CREATE TABLE t_issue_557_aocs_part3
+PARTITION OF t_issue_557_aocs
+FOR VALUES WITH (MODULUS 3, REMAINDER 2)
+WITH (appendonly=true, orientation=column);
+NOTICE:  table has parent, setting distribution columns to match parent table
+-- Create Indexes
+-- Unique
+CREATE UNIQUE INDEX on t_issue_557_aocs(product_id,description);
+INSERT INTO t_issue_557_aocs (
+product_id,
+is_audited,
+description,
+status
+)
+SELECT
+x.id, -- product_id
+FALSE,
+'Product description ' || x.id, -- description
+'Closed'
+FROM (
+SELECT * FROM generate_series(1, 20) AS id
+) AS x;
+UPDATE t_issue_557_aocs
+SET status = 'Closed',
+description = description || ' Audited';
+DELETE FROM t_issue_557_aocs;
+INSERT INTO t_issue_557_aocs (
+product_id,
+is_audited,
+description,
+status
+)
+SELECT
+x.id, -- product_id
+FALSE,
+'Product description ' || x.id, -- description
+'Closed'
+FROM (
+SELECT * FROM generate_series(1, 20) AS id
+) AS x;
+UPDATE t_issue_557_aocs
+SET status = 'Closed',
+description = description || ' Audited';
+-- start_ignore
+drop schema t_issue_557 cascade;
+-- end_ignore

src/test/regress/sql/.gitignore

@@ -60,3 +60,4 @@
 /external_table_union_all.sql
 /directory_table.sql
 /tag.sql
+/ao_unique_index_partition.sql