remove useless functions and dead code, refactoring and locks for function replace_hash_partition()

Author: funbringer

hash.sql

@@ -18,14 +18,6 @@ CREATE OR REPLACE FUNCTION @extschema@.create_hash_partitions(
 	partition_data		BOOLEAN DEFAULT TRUE)
 RETURNS INTEGER AS
 $$
-DECLARE
-	v_child_relname		TEXT;
-	v_plain_schema		TEXT;
-	v_plain_relname		TEXT;
-	-- v_atttype			REGTYPE;
-	-- v_hashfunc			REGPROC;
-	v_init_callback		REGPROCEDURE;
-
 BEGIN
 	PERFORM @extschema@.validate_relname(parent_relid);
 
@@ -40,13 +32,6 @@ BEGIN
 	attribute := lower(attribute);
 	PERFORM @extschema@.common_relation_checks(parent_relid, attribute);
 
-	/* Fetch atttype and its hash function */
-	-- v_atttype := @extschema@.get_attribute_type(parent_relid, attribute);
-	-- v_hashfunc := @extschema@.get_type_hash_func(v_atttype);
-
-	SELECT * INTO v_plain_schema, v_plain_relname
-	FROM @extschema@.get_plain_schema_and_relname(parent_relid);
-
 	/* Insert new entry to pathman config */
 	INSERT INTO @extschema@.pathman_config (partrel, attname, parttype)
 	VALUES (parent_relid, attribute, 1);
@@ -82,21 +67,26 @@ CREATE OR REPLACE FUNCTION @extschema@.replace_hash_partition(
 RETURNS REGCLASS AS
 $$
 DECLARE
-	v_attname			TEXT;
+	parent_relid		REGCLASS;
+	part_attname		TEXT;		/* partitioned column */
+	old_constr_name		TEXT;		/* name of old_partition's constraint */
+	old_constr_def		TEXT;		/* definition of old_partition's constraint */
 	rel_persistence		CHAR;
-	v_init_callback		REGPROCEDURE;
-	v_parent_relid		REGCLASS;
-	v_part_count		INT;
-	v_part_num			INT;
+	p_init_callback		REGPROCEDURE;
+
 BEGIN
 	PERFORM @extschema@.validate_relname(old_partition);
 	PERFORM @extschema@.validate_relname(new_partition);
 
 	/* Parent relation */
-	v_parent_relid := @extschema@.get_parent_of_partition(old_partition);
+	parent_relid := @extschema@.get_parent_of_partition(old_partition);
 
 	/* Acquire lock on parent */
-	PERFORM @extschema@.lock_partitioned_relation(v_parent_relid);
+	PERFORM @extschema@.lock_partitioned_relation(parent_relid);
+
+	/* Acquire data modification lock (prevent further modifications) */
+	PERFORM @extschema@.prevent_relation_modification(old_partition);
+	PERFORM @extschema@.prevent_relation_modification(new_partition);
 
 	/* Ignore temporary tables */
 	SELECT relpersistence FROM pg_catalog.pg_class
@@ -108,52 +98,54 @@ BEGIN
 	END IF;
 
 	/* Check that new partition has an equal structure as parent does */
-	IF NOT @extschema@.validate_relations_equality(v_parent_relid, new_partition) THEN
+	IF NOT @extschema@.validate_relations_equality(parent_relid, new_partition) THEN
 		RAISE EXCEPTION 'partition must have the exact same structure as parent';
 	END IF;
 
 	/* Get partitioning key */
-	v_attname := attname FROM @extschema@.pathman_config WHERE partrel = v_parent_relid;
-	IF v_attname IS NULL THEN
-		RAISE EXCEPTION 'table "%" is not partitioned', v_parent_relid::TEXT;
+	part_attname := attname FROM @extschema@.pathman_config WHERE partrel = parent_relid;
+	IF part_attname IS NULL THEN
+		RAISE EXCEPTION 'table "%" is not partitioned', parent_relid::TEXT;
 	END IF;
 
-	/* Calculate partitions count and old partition's number */
-	v_part_count := count(*) FROM @extschema@.pathman_partition_list WHERE parent = v_parent_relid;
-	v_part_num := @extschema@.get_partition_hash(v_parent_relid, old_partition);
+	/* Fetch name of old_partition's HASH constraint */
+	old_constr_name = @extschema@.build_check_constraint_name(old_partition::REGCLASS,
+															  part_attname);
+
+	/* Fetch definition of old_partition's HASH constraint */
+	SELECT pg_catalog.pg_get_constraintdef(oid) FROM pg_catalog.pg_constraint
+	WHERE conrelid = old_partition AND conname = old_constr_name
+	INTO old_constr_def;
 
 	/* Detach old partition */
-	EXECUTE format('ALTER TABLE %s NO INHERIT %s', old_partition, v_parent_relid);
-	EXECUTE format('ALTER TABLE %s DROP CONSTRAINT IF EXISTS %s',
-		old_partition,
-		@extschema@.build_check_constraint_name(old_partition::REGCLASS,
-												v_attname));
-
-	/* Attach new one */
-	EXECUTE format('ALTER TABLE %s INHERIT %s', new_partition, v_parent_relid);
-	EXECUTE format('ALTER TABLE %s ADD CONSTRAINT %s CHECK (%s)',
+	EXECUTE format('ALTER TABLE %s NO INHERIT %s', old_partition, parent_relid);
+	EXECUTE format('ALTER TABLE %s DROP CONSTRAINT %s',
+				   old_partition,
+				   old_constr_name);
+
+	/* Attach the new one */
+	EXECUTE format('ALTER TABLE %s INHERIT %s', new_partition, parent_relid);
+	EXECUTE format('ALTER TABLE %s ADD CONSTRAINT %s %s',
 				   new_partition,
-				   @extschema@.build_check_constraint_name(new_partition::regclass,
-														   v_attname),
-				   @extschema@.build_hash_condition(new_partition::regclass,
-													v_attname,
-													v_part_count,
-													v_part_num));
+				   @extschema@.build_check_constraint_name(new_partition::REGCLASS,
+														   part_attname),
+				   old_constr_def);
 
 	/* Fetch init_callback from 'params' table */
 	WITH stub_callback(stub) as (values (0))
 	SELECT coalesce(init_callback, 0::REGPROCEDURE)
 	FROM stub_callback
 	LEFT JOIN @extschema@.pathman_config_params AS params
-	ON params.partrel = v_parent_relid
-	INTO v_init_callback;
+	ON params.partrel = parent_relid
+	INTO p_init_callback;
 
-	PERFORM @extschema@.invoke_on_partition_created_callback(v_parent_relid,
+	/* Finally invoke init_callback */
+	PERFORM @extschema@.invoke_on_partition_created_callback(parent_relid,
 															 new_partition,
-															 v_init_callback);
+															 p_init_callback);
 
 	/* Invalidate cache */
-	PERFORM @extschema@.on_update_partitions(v_parent_relid);
+	PERFORM @extschema@.on_update_partitions(parent_relid);
 
 	RETURN new_partition;
 END
@@ -292,3 +284,14 @@ LANGUAGE C STRICT;
 CREATE OR REPLACE FUNCTION @extschema@.get_hash_part_idx(INTEGER, INTEGER)
 RETURNS INTEGER AS 'pg_pathman', 'get_hash_part_idx'
 LANGUAGE C STRICT;
+
+/*
+ * Build hash condition for a CHECK CONSTRAINT
+ */
+CREATE OR REPLACE FUNCTION @extschema@.build_hash_condition(
+	parent_relid		REGCLASS,
+	attribute			TEXT,
+	part_count			INT4,
+	part_idx			INT4)
+RETURNS TEXT AS 'pg_pathman', 'build_hash_condition'
+LANGUAGE C STRICT;

init.sql

@@ -783,23 +783,3 @@ CREATE OR REPLACE FUNCTION @extschema@.invoke_on_partition_created_callback(
 	init_callback	REGPROCEDURE)
 RETURNS VOID AS 'pg_pathman', 'invoke_on_partition_created_callback'
 LANGUAGE C;
-
-/*
- * Build hash condition for a CHECK CONSTRAINT
- */
-CREATE OR REPLACE FUNCTION @extschema@.build_hash_condition(
-	parent_relid	REGCLASS,
-	attname			TEXT,
-	partitions_count INT,
-	partition_number INT)
-RETURNS TEXT AS 'pg_pathman', 'build_hash_condition'
-LANGUAGE C;
-
-/*
- * Returns hash value for specified partition (0..N)
- */
-CREATE OR REPLACE FUNCTION @extschema@.get_partition_hash(
-	parent_relid	REGCLASS,
-	partition		REGCLASS)
-RETURNS INT AS 'pg_pathman', 'get_partition_hash'
-LANGUAGE C;

src/pl_hash_funcs.c

@@ -22,10 +22,11 @@
 /* Function declarations */
 
 PG_FUNCTION_INFO_V1( create_hash_partitions_internal );
+
 PG_FUNCTION_INFO_V1( get_type_hash_func );
 PG_FUNCTION_INFO_V1( get_hash_part_idx );
+
 PG_FUNCTION_INFO_V1( build_hash_condition );
-PG_FUNCTION_INFO_V1( get_partition_hash );
 
 
 /*
@@ -90,68 +91,36 @@ get_hash_part_idx(PG_FUNCTION_ARGS)
 Datum
 build_hash_condition(PG_FUNCTION_ARGS)
 {
+	Oid				parent = PG_GETARG_OID(0);
+	text		   *attname = PG_GETARG_TEXT_P(1);
+	uint32			part_count = PG_GETARG_UINT32(2);
+	uint32			part_idx = PG_GETARG_UINT32(3);
+
 	TypeCacheEntry *tce;
+	Oid				attype;
+	char		   *attname_cstring = text_to_cstring(attname);
 
-	Oid		parent = PG_GETARG_OID(0);
-	text   *attname = PG_GETARG_TEXT_P(1);
-	uint32	partitions_count = PG_GETARG_UINT32(2);
-	uint32	partition_number = PG_GETARG_UINT32(3);
-	Oid		attyp;
-	char   *result;
+	char		   *result;
 
-	if (partition_number >= partitions_count)
-		elog(ERROR,
-			 "Partition number cannot exceed partitions count");
+	if (part_idx >= part_count)
+		elog(ERROR, "'part_idx' must be lower than 'part_count'");
 
 	/* Get attribute type and its hash function oid */
-	attyp = get_attribute_type(parent, text_to_cstring(attname), false);
-	if (attyp == InvalidOid)
-		elog(ERROR,
-			 "Relation '%s' has no attribute '%s'",
-			 get_rel_name(parent),
-			 text_to_cstring(attname));
+	attype = get_attribute_type(parent, attname_cstring, false);
+	if (attype == InvalidOid)
+		elog(ERROR, "relation \"%s\" has no attribute \"%s\"",
+					get_rel_name(parent),
+					attname_cstring);
 
-	tce = lookup_type_cache(attyp, TYPECACHE_HASH_PROC);
+	tce = lookup_type_cache(attype, TYPECACHE_HASH_PROC);
 
 	/* Create hash condition CSTRING */
 	result = psprintf("%s.get_hash_part_idx(%s(%s), %u) = %u",
 					  get_namespace_name(get_pathman_schema()),
 					  get_func_name(tce->hash_proc),
-					  text_to_cstring(attname),
-					  partitions_count,
-					  partition_number);
+					  attname_cstring,
+					  part_count,
+					  part_idx);
 
 	PG_RETURN_TEXT_P(cstring_to_text(result));
 }
-
-/*
- * Returns hash value for specified partition (0..N)
- */
-Datum
-get_partition_hash(PG_FUNCTION_ARGS)
-{
-	const PartRelationInfo *prel;
-	Oid		parent = PG_GETARG_OID(0);
-	Oid		partition = PG_GETARG_OID(1);
-	Oid	   *children;
-	int		i;
-
-	/* Validate partition type */
-	prel = get_pathman_relation_info(parent);
-	if (!prel || prel->parttype != PT_HASH)
-		elog(ERROR,
-			 "Relation '%s' isn't partitioned by hash",
-			 get_rel_name(parent));
-
-	/* Searching for partition */
-	children = PrelGetChildrenArray(prel);
-	for (i=0; i<prel->children_count; i++)
-		if (children[i] == partition)
-			PG_RETURN_UINT32(i);
-
-	/* If we get here then there is no such partition for specified parent */
-	elog(ERROR,
-		 "Relation '%s' isn't a part of partitioned table '%s'",
-		 get_rel_name(parent),
-		 get_rel_name(partition));
-}