[PGPRO-7880] Need to check relation Oid before locking

kovdb75 · kovdb75 · commit 6bcd9d82b91b · 2023-04-05T11:38:35.000+03:00
It need to do before relation locking: locking of invalid Oid causes an
error on replica

Tags: pg_pathman
diff --git a/expected/pathman_calamity.out b/expected/pathman_calamity.out
@@ -320,7 +320,7 @@ SELECT validate_relname(NULL);
 ERROR:  relation should not be NULL
 /* check function validate_expression() */
 SELECT validate_expression(1::regclass, NULL);					/* not ok */
-ERROR:  relation "1" does not exist
+ERROR:  identifier "1" must be normal Oid
 SELECT validate_expression(NULL::regclass, NULL);				/* not ok */
 ERROR:  'relid' should not be NULL
 SELECT validate_expression('calamity.part_test', NULL);			/* not ok */
@@ -426,19 +426,19 @@ SELECT build_sequence_name(NULL) IS NULL;
 
 /* check function partition_table_concurrently() */
 SELECT partition_table_concurrently(1::REGCLASS);				/* not ok */
-ERROR:  relation "1" has no partitions
+ERROR:  identifier "1" must be normal Oid
 SELECT partition_table_concurrently('pg_class', 0);				/* not ok */
 ERROR:  'batch_size' should not be less than 1 or greater than 10000
 SELECT partition_table_concurrently('pg_class', 1, 1E-5);		/* not ok */
 ERROR:  'sleep_time' should not be less than 0.5
 SELECT partition_table_concurrently('pg_class');				/* not ok */
-ERROR:  relation "pg_class" has no partitions
+ERROR:  identifier "1259" must be normal Oid
 /* check function stop_concurrent_part_task() */
 SELECT stop_concurrent_part_task(1::REGCLASS);					/* not ok */
 ERROR:  cannot find worker for relation "1"
 /* check function drop_range_partition_expand_next() */
 SELECT drop_range_partition_expand_next('pg_class');			/* not ok */
-ERROR:  relation "pg_class" is not a partition
+ERROR:  identifier "1259" must be normal Oid
 SELECT drop_range_partition_expand_next(NULL) IS NULL;
  ?column? 
 ----------
@@ -560,7 +560,7 @@ DROP FUNCTION calamity.dummy_cb(arg jsonb);
 SELECT add_to_pathman_config(NULL, 'val');						/* no table */
 ERROR:  'parent_relid' should not be NULL
 SELECT add_to_pathman_config(0::REGCLASS, 'val');				/* no table (oid) */
-ERROR:  relation "0" does not exist
+ERROR:  identifier "0" must be normal Oid
 SELECT add_to_pathman_config('calamity.part_test', NULL);		/* no expr */
 ERROR:  'expression' should not be NULL
 SELECT add_to_pathman_config('calamity.part_test', 'V_A_L');	/* wrong expr */
diff --git a/expected/pathman_calamity_1.out b/expected/pathman_calamity_1.out
@@ -320,7 +320,7 @@ SELECT validate_relname(NULL);
 ERROR:  relation should not be NULL
 /* check function validate_expression() */
 SELECT validate_expression(1::regclass, NULL);					/* not ok */
-ERROR:  relation "1" does not exist
+ERROR:  identifier "1" must be normal Oid
 SELECT validate_expression(NULL::regclass, NULL);				/* not ok */
 ERROR:  'relid' should not be NULL
 SELECT validate_expression('calamity.part_test', NULL);			/* not ok */
@@ -426,19 +426,19 @@ SELECT build_sequence_name(NULL) IS NULL;
 
 /* check function partition_table_concurrently() */
 SELECT partition_table_concurrently(1::REGCLASS);				/* not ok */
-ERROR:  relation "1" has no partitions
+ERROR:  identifier "1" must be normal Oid
 SELECT partition_table_concurrently('pg_class', 0);				/* not ok */
 ERROR:  'batch_size' should not be less than 1 or greater than 10000
 SELECT partition_table_concurrently('pg_class', 1, 1E-5);		/* not ok */
 ERROR:  'sleep_time' should not be less than 0.5
 SELECT partition_table_concurrently('pg_class');				/* not ok */
-ERROR:  relation "pg_class" has no partitions
+ERROR:  identifier "1259" must be normal Oid
 /* check function stop_concurrent_part_task() */
 SELECT stop_concurrent_part_task(1::REGCLASS);					/* not ok */
 ERROR:  cannot find worker for relation "1"
 /* check function drop_range_partition_expand_next() */
 SELECT drop_range_partition_expand_next('pg_class');			/* not ok */
-ERROR:  relation "pg_class" is not a partition
+ERROR:  identifier "1259" must be normal Oid
 SELECT drop_range_partition_expand_next(NULL) IS NULL;
  ?column? 
 ----------
@@ -560,7 +560,7 @@ DROP FUNCTION calamity.dummy_cb(arg jsonb);
 SELECT add_to_pathman_config(NULL, 'val');						/* no table */
 ERROR:  'parent_relid' should not be NULL
 SELECT add_to_pathman_config(0::REGCLASS, 'val');				/* no table (oid) */
-ERROR:  relation "0" does not exist
+ERROR:  identifier "0" must be normal Oid
 SELECT add_to_pathman_config('calamity.part_test', NULL);		/* no expr */
 ERROR:  'expression' should not be NULL
 SELECT add_to_pathman_config('calamity.part_test', 'V_A_L');	/* wrong expr */
diff --git a/expected/pathman_calamity_2.out b/expected/pathman_calamity_2.out
@@ -320,7 +320,7 @@ SELECT validate_relname(NULL);
 ERROR:  relation should not be NULL
 /* check function validate_expression() */
 SELECT validate_expression(1::regclass, NULL);					/* not ok */
-ERROR:  relation "1" does not exist
+ERROR:  identifier "1" must be normal Oid
 SELECT validate_expression(NULL::regclass, NULL);				/* not ok */
 ERROR:  'relid' should not be NULL
 SELECT validate_expression('calamity.part_test', NULL);			/* not ok */
@@ -426,19 +426,19 @@ SELECT build_sequence_name(NULL) IS NULL;
 
 /* check function partition_table_concurrently() */
 SELECT partition_table_concurrently(1::REGCLASS);				/* not ok */
-ERROR:  relation "1" has no partitions
+ERROR:  identifier "1" must be normal Oid
 SELECT partition_table_concurrently('pg_class', 0);				/* not ok */
 ERROR:  'batch_size' should not be less than 1 or greater than 10000
 SELECT partition_table_concurrently('pg_class', 1, 1E-5);		/* not ok */
 ERROR:  'sleep_time' should not be less than 0.5
 SELECT partition_table_concurrently('pg_class');				/* not ok */
-ERROR:  relation "pg_class" has no partitions
+ERROR:  identifier "1259" must be normal Oid
 /* check function stop_concurrent_part_task() */
 SELECT stop_concurrent_part_task(1::REGCLASS);					/* not ok */
 ERROR:  cannot find worker for relation "1"
 /* check function drop_range_partition_expand_next() */
 SELECT drop_range_partition_expand_next('pg_class');			/* not ok */
-ERROR:  relation "pg_class" is not a partition
+ERROR:  identifier "1259" must be normal Oid
 SELECT drop_range_partition_expand_next(NULL) IS NULL;
  ?column? 
 ----------
@@ -560,7 +560,7 @@ DROP FUNCTION calamity.dummy_cb(arg jsonb);
 SELECT add_to_pathman_config(NULL, 'val');						/* no table */
 ERROR:  'parent_relid' should not be NULL
 SELECT add_to_pathman_config(0::REGCLASS, 'val');				/* no table (oid) */
-ERROR:  relation "0" does not exist
+ERROR:  identifier "0" must be normal Oid
 SELECT add_to_pathman_config('calamity.part_test', NULL);		/* no expr */
 ERROR:  'expression' should not be NULL
 SELECT add_to_pathman_config('calamity.part_test', 'V_A_L');	/* wrong expr */
diff --git a/expected/pathman_calamity_3.out b/expected/pathman_calamity_3.out
@@ -324,7 +324,7 @@ SELECT validate_relname(NULL);
 ERROR:  relation should not be NULL
 /* check function validate_expression() */
 SELECT validate_expression(1::regclass, NULL);					/* not ok */
-ERROR:  relation "1" does not exist
+ERROR:  identifier "1" must be normal Oid
 SELECT validate_expression(NULL::regclass, NULL);				/* not ok */
 ERROR:  'relid' should not be NULL
 SELECT validate_expression('calamity.part_test', NULL);			/* not ok */
@@ -430,19 +430,19 @@ SELECT build_sequence_name(NULL) IS NULL;
 
 /* check function partition_table_concurrently() */
 SELECT partition_table_concurrently(1::REGCLASS);				/* not ok */
-ERROR:  relation "1" has no partitions
+ERROR:  identifier "1" must be normal Oid
 SELECT partition_table_concurrently('pg_class', 0);				/* not ok */
 ERROR:  'batch_size' should not be less than 1 or greater than 10000
 SELECT partition_table_concurrently('pg_class', 1, 1E-5);		/* not ok */
 ERROR:  'sleep_time' should not be less than 0.5
 SELECT partition_table_concurrently('pg_class');				/* not ok */
-ERROR:  relation "pg_class" has no partitions
+ERROR:  identifier "1259" must be normal Oid
 /* check function stop_concurrent_part_task() */
 SELECT stop_concurrent_part_task(1::REGCLASS);					/* not ok */
 ERROR:  cannot find worker for relation "1"
 /* check function drop_range_partition_expand_next() */
 SELECT drop_range_partition_expand_next('pg_class');			/* not ok */
-ERROR:  relation "pg_class" is not a partition
+ERROR:  identifier "1259" must be normal Oid
 SELECT drop_range_partition_expand_next(NULL) IS NULL;
  ?column? 
 ----------
@@ -564,7 +564,7 @@ DROP FUNCTION calamity.dummy_cb(arg jsonb);
 SELECT add_to_pathman_config(NULL, 'val');						/* no table */
 ERROR:  'parent_relid' should not be NULL
 SELECT add_to_pathman_config(0::REGCLASS, 'val');				/* no table (oid) */
-ERROR:  relation "0" does not exist
+ERROR:  identifier "0" must be normal Oid
 SELECT add_to_pathman_config('calamity.part_test', NULL);		/* no expr */
 ERROR:  'expression' should not be NULL
 SELECT add_to_pathman_config('calamity.part_test', 'V_A_L');	/* wrong expr */
diff --git a/src/include/utils.h b/src/include/utils.h
@@ -84,5 +84,6 @@ Datum extract_binary_interval_from_text(Datum interval_text,
 										Oid *interval_type);
 char **deconstruct_text_array(Datum array, int *array_size);
 RangeVar **qualified_relnames_to_rangevars(char **relnames, size_t nrelnames);
+void check_relation_oid(Oid relid);
 
 #endif /* PATHMAN_UTILS_H */
diff --git a/src/pathman_workers.c b/src/pathman_workers.c
@@ -712,6 +712,8 @@ partition_table_concurrently(PG_FUNCTION_ARGS)
 		ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 						errmsg("'sleep_time' should not be less than 0.5")));
 
+	check_relation_oid(relid);
+
 	/* Prevent concurrent function calls */
 	LockRelationOid(relid, lockmode);
 
diff --git a/src/pl_funcs.c b/src/pl_funcs.c
@@ -673,6 +673,7 @@ validate_expression(PG_FUNCTION_ARGS)
 	if (!PG_ARGISNULL(0))
 	{
 		relid = PG_GETARG_OID(0);
+		check_relation_oid(relid);
 	}
 	else ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 						 errmsg("'relid' should not be NULL")));
@@ -807,6 +808,7 @@ add_to_pathman_config(PG_FUNCTION_ARGS)
 	if (!PG_ARGISNULL(0))
 	{
 		relid = PG_GETARG_OID(0);
+		check_relation_oid(relid);
 	}
 	else ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 						 errmsg("'parent_relid' should not be NULL")));
@@ -1037,6 +1039,8 @@ prevent_part_modification(PG_FUNCTION_ARGS)
 {
 	Oid relid = PG_GETARG_OID(0);
 
+	check_relation_oid(relid);
+
 	/* Lock partitioned relation till transaction's end */
 	LockRelationOid(relid, ShareUpdateExclusiveLock);
 
@@ -1051,6 +1055,8 @@ prevent_data_modification(PG_FUNCTION_ARGS)
 {
 	Oid relid = PG_GETARG_OID(0);
 
+	check_relation_oid(relid);
+
 	/*
 	 * Check that isolation level is READ COMMITTED.
 	 * Else we won't be able to see new rows
diff --git a/src/pl_range_funcs.c b/src/pl_range_funcs.c
@@ -499,6 +499,7 @@ split_range_partition(PG_FUNCTION_ARGS)
 	if (!PG_ARGISNULL(0))
 	{
 		partition1 = PG_GETARG_OID(0);
+		check_relation_oid(partition1);
 	}
 	else ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 						 errmsg("'partition1' should not be NULL")));
@@ -835,6 +836,8 @@ drop_range_partition_expand_next(PG_FUNCTION_ARGS)
 	RangeEntry		   *ranges;
 	int					i;
 
+	check_relation_oid(partition);
+
 	/* Lock the partition we're going to drop */
 	LockRelationOid(partition, AccessExclusiveLock);
 
diff --git a/src/utils.c b/src/utils.c
@@ -527,3 +527,14 @@ qualified_relnames_to_rangevars(char **relnames, size_t nrelnames)
 	return rangevars;
 }
 
+/*
+ * Checks that Oid is valid (it need to do before relation locking: locking of
+ * invalid Oid causes an error on replica).
+ */
+void
+check_relation_oid(Oid relid)
+{
+	if (relid < FirstNormalObjectId)
+		ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						errmsg("identifier \"%u\" must be normal Oid", relid)));
+}

Original file line number	Diff line number	Diff line change
`@@ -673,6 +673,7 @@ validate_expression(PG_FUNCTION_ARGS)`
`673`	`673`	`if (!PG_ARGISNULL(0))`
`674`	`674`	`{`
`675`	`675`	`relid = PG_GETARG_OID(0);`
	`676`	`+ check_relation_oid(relid);`
`676`	`677`	`}`
`677`	`678`	`else ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),`
`678`	`679`	`errmsg("'relid' should not be NULL")));`
`@@ -807,6 +808,7 @@ add_to_pathman_config(PG_FUNCTION_ARGS)`
`807`	`808`	`if (!PG_ARGISNULL(0))`
`808`	`809`	`{`
`809`	`810`	`relid = PG_GETARG_OID(0);`
	`811`	`+ check_relation_oid(relid);`
`810`	`812`	`}`
`811`	`813`	`else ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),`
`812`	`814`	`errmsg("'parent_relid' should not be NULL")));`
`@@ -1037,6 +1039,8 @@ prevent_part_modification(PG_FUNCTION_ARGS)`
`1037`	`1039`	`{`
`1038`	`1040`	`Oid relid = PG_GETARG_OID(0);`
`1039`	`1041`
	`1042`	`+ check_relation_oid(relid);`
	`1043`	`+`
`1040`	`1044`	`/* Lock partitioned relation till transaction's end */`
`1041`	`1045`	`LockRelationOid(relid, ShareUpdateExclusiveLock);`
`1042`	`1046`
`@@ -1051,6 +1055,8 @@ prevent_data_modification(PG_FUNCTION_ARGS)`
`1051`	`1055`	`{`
`1052`	`1056`	`Oid relid = PG_GETARG_OID(0);`
`1053`	`1057`
	`1058`	`+ check_relation_oid(relid);`
	`1059`	`+`
`1054`	`1060`	`/*`
`1055`	`1061`	`* Check that isolation level is READ COMMITTED.`
`1056`	`1062`	`* Else we won't be able to see new rows`
Original file line number	Diff line number	Diff line change
`@@ -499,6 +499,7 @@ split_range_partition(PG_FUNCTION_ARGS)`
`499`	`499`	`if (!PG_ARGISNULL(0))`
`500`	`500`	`{`
`501`	`501`	`partition1 = PG_GETARG_OID(0);`
	`502`	`+ check_relation_oid(partition1);`
`502`	`503`	`}`
`503`	`504`	`else ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),`
`504`	`505`	`errmsg("'partition1' should not be NULL")));`
`@@ -835,6 +836,8 @@ drop_range_partition_expand_next(PG_FUNCTION_ARGS)`
`835`	`836`	`RangeEntry *ranges;`
`836`	`837`	`int i;`
`837`	`838`
	`839`	`+ check_relation_oid(partition);`
	`840`	`+`
`838`	`841`	`/* Lock the partition we're going to drop */`
`839`	`842`	`LockRelationOid(partition, AccessExclusiveLock);`
`840`	`843`