Honor usage of OidIsValid() macro

Postgres source code define the macro `OidIsValid()` to check if the Oid is valid or not (comparing against the `InvalidOid` type). See `src/include/c.h` in Postgres source three. Changed all direct comparisons against `InvalidOid` for the `OidIsValid` call and add a coccinelle check to make sure the future changes will use it correctly.
timescale · Nov 3, 2022 · 2ef09cb · 2ef09cb
1 parent 7dd45cf
commit 2ef09cb
Show file tree

Hide file tree

Showing 28 changed files with 75 additions and 44 deletions.
diff --git a/.gitignore b/.gitignore
@@ -32,3 +32,4 @@ typedef.list
 /debug/
 /cmake-build-debug/
 /.idea/
+coccinelle.diff
diff --git a/coccinelle/oidisvalid.cocci b/coccinelle/oidisvalid.cocci
@@ -0,0 +1,30 @@
+//
+// find comparisons against `InvalidOid`
+//
+// Postgres has `OidIsValid()` macro to check if a given Oid is valid or not
+// (see https://github.com/postgres/postgres/blob/master/src/include/c.h).
+//
+// This script look for comparisons to `InvalidOid` and recommend the usage of
+// `OidIsValid()` instead.
+//
+// For example:
+//    `oid != InvalidOid` should be `OidIsValid(oid)`
+//    `oid == InvalidOid` should be `!OidIsValid(oid)`
+//
+@@
+symbol InvalidOid;
+expression oid;
+@@
+
+/* use OidIsValid() instead of comparing against InvalidOid */
+- (oid != InvalidOid)
++ OidIsValid(oid)
+
+@@
+symbol InvalidOid;
+expression oid;
+@@
+
+/* use OidIsValid() instead of comparing against InvalidOid */
+- (oid == InvalidOid)
++ !OidIsValid(oid)
diff --git a/src/custom_type_cache.c b/src/custom_type_cache.c
@@ -41,7 +41,7 @@ ts_custom_type_cache_get(CustomType type)
 
 	tinfo = &typeinfo[type];
 
-	if (tinfo->type_oid == InvalidOid)
+	if (!OidIsValid(tinfo->type_oid))
 	{
 		Oid schema_oid = LookupExplicitNamespace(tinfo->schema_name, false);
 		Oid type_oid = GetSysCacheOid2(TYPENAMENSP,

diff --git a/src/dimension.c b/src/dimension.c
@@ -937,7 +937,7 @@ ts_dimension_transform_value(const Dimension *dim, Oid collation, Datum value, O
 	{
 		if (NULL != dim->partitioning)
 			*restype = dim->partitioning->partfunc.rettype;
-		else if (const_datum_type != InvalidOid)
+		else if (OidIsValid(const_datum_type))
 			*restype = const_datum_type;
 		else
 			*restype = dim->fd.column_type;

diff --git a/src/extension.c b/src/extension.c
@@ -197,7 +197,7 @@ extension_update_state()
 	if (new_state == EXTENSION_STATE_CREATED || new_state == EXTENSION_STATE_TRANSITIONING)
 	{
 		ts_extension_oid = get_extension_oid(EXTENSION_NAME, true /* missing_ok */);
-		Assert(ts_extension_oid != InvalidOid);
+		Assert(OidIsValid(ts_extension_oid));
 	}
 	else
 	{
@@ -241,7 +241,7 @@ ts_extension_schema_oid(void)
 	systable_endscan(scandesc);
 	table_close(rel, AccessShareLock);
 
-	if (schema == InvalidOid)
+	if (!OidIsValid(schema))
 		elog(ERROR, "extension schema not found");
 	return schema;
 }

diff --git a/src/hypertable.c b/src/hypertable.c
@@ -1240,7 +1240,8 @@ ts_is_hypertable(Oid relid)
 {
 	if (!OidIsValid(relid))
 		return false;
-	return hypertable_relid_lookup(relid) != InvalidOid;
+
+	return OidIsValid(hypertable_relid_lookup(relid));
 }
 
 /*
@@ -1640,7 +1641,7 @@ ts_hypertable_check_partitioning(const Hypertable *ht, int32 id_of_updated_dimen
 {
 	const Dimension *dim;
 
-	Assert(id_of_updated_dimension != InvalidOid);
+	Assert(OidIsValid(id_of_updated_dimension));
 
 	dim = ts_hyperspace_get_dimension_by_id(ht->space, id_of_updated_dimension);
 

diff --git a/src/hypertable_restrict_info.c b/src/hypertable_restrict_info.c
@@ -405,7 +405,7 @@ dimension_values_create_from_array(Const *c, bool user_or)
 
 	/* it's an array type, lets get the base element type */
 	base_el_type = get_element_type(c->consttype);
-	if (base_el_type == InvalidOid)
+	if (!OidIsValid(base_el_type))
 		elog(ERROR,
 			 "invalid base element type for array type: \"%s\"",
 			 format_type_be(c->consttype));

diff --git a/src/indexing.c b/src/indexing.c
@@ -277,7 +277,7 @@ ts_indexing_relation_has_primary_or_unique_index(Relation htrel)
 	ListCell *lc;
 	bool result = false;
 
-	if (htrel->rd_pkindex != InvalidOid)
+	if (OidIsValid(htrel->rd_pkindex))
 		return true;
 
 	foreach (lc, indexoidlist)

diff --git a/src/loader/loader.c b/src/loader/loader.c
@@ -205,7 +205,7 @@ extension_owner(void)
 	systable_endscan(scandesc);
 	table_close(rel, AccessShareLock);
 
-	if (extension_owner == InvalidOid)
+	if (!OidIsValid(extension_owner))
 		elog(ERROR, "extension not found while getting owner");
 
 	return extension_owner;
@@ -346,7 +346,7 @@ stop_workers_on_db_drop(DropdbStmt *drop_db_statement)
 	 */
 	Oid dropped_db_oid = get_database_oid(drop_db_statement->dbname, drop_db_statement->missing_ok);
 
-	if (dropped_db_oid != InvalidOid)
+	if (OidIsValid(dropped_db_oid))
 	{
 		ereport(LOG,
 				(errmsg("TimescaleDB background worker scheduler for database %u will be stopped",

diff --git a/src/nodes/chunk_insert_state.c b/src/nodes/chunk_insert_state.c
@@ -558,7 +558,7 @@ lock_associated_compressed_chunk(int32 chunk_id, bool *has_compressed_chunk)
 	{
 		Oid compress_chunk_relid =
 			ts_chunk_get_relid(compressed_chunk_id, /* missing_ok = */ false);
-		Assert(compress_chunk_relid != InvalidOid);
+		Assert(OidIsValid(compress_chunk_relid));
 
 		*has_compressed_chunk = true;
 		return table_open(compress_chunk_relid, RowExclusiveLock);

diff --git a/src/partitioning.c b/src/partitioning.c
@@ -202,8 +202,7 @@ ts_partitioning_info_create(const char *schema, const char *partfunc, const char
 	{
 		TypeCacheEntry *tce = lookup_type_cache(columntype, TYPECACHE_HASH_FLAGS);
 
-		if (tce->hash_proc == InvalidOid &&
-			ts_partitioning_func_is_closed_default(schema, partfunc))
+		if (!OidIsValid(tce->hash_proc) && ts_partitioning_func_is_closed_default(schema, partfunc))
 			elog(ERROR, "could not find hash function for type %s", format_type_be(columntype));
 	}
 
@@ -448,7 +447,7 @@ ts_get_partition_hash(PG_FUNCTION_ARGS)
 		fcinfo->flinfo->fn_extra = pfc;
 	}
 
-	if (pfc->tce->hash_proc == InvalidOid)
+	if (!OidIsValid(pfc->tce->hash_proc))
 		elog(ERROR, "could not find hash function for type %u", pfc->argtype);
 
 	/* use the supplied collation, if it exists, otherwise use the default for

diff --git a/src/planner/agg_bookend.c b/src/planner/agg_bookend.c
@@ -162,9 +162,9 @@ initialize_func_strategy(FuncStrategy *func_strategy, char *name, int nargs, Oid
 static FuncStrategy *
 get_func_strategy(Oid func_oid)
 {
-	if (first_func_strategy.func_oid == InvalidOid)
+	if (!OidIsValid(first_func_strategy.func_oid))
 		initialize_func_strategy(&first_func_strategy, "first", 2, first_last_arg_types);
-	if (last_func_strategy.func_oid == InvalidOid)
+	if (!OidIsValid(last_func_strategy.func_oid))
 		initialize_func_strategy(&last_func_strategy, "last", 2, first_last_arg_types);
 	if (first_func_strategy.func_oid == func_oid)
 		return &first_func_strategy;
@@ -478,7 +478,7 @@ find_first_last_aggs_walker(Node *node, List **context)
 		sort_tce = lookup_type_cache(sort_oid, TYPECACHE_BTREE_OPFAMILY);
 		aggsortop =
 			get_opfamily_member(sort_tce->btree_opf, sort_oid, sort_oid, func_strategy->strategy);
-		if (aggsortop == InvalidOid)
+		if (!OidIsValid(aggsortop))
 			elog(ERROR,
 				 "Cannot resolve sort operator for function \"%s\" and type \"%s\"",
 				 format_procedure(aggref->aggfnoid),

diff --git a/src/planner/expand_hypertable.c b/src/planner/expand_hypertable.c
@@ -79,13 +79,13 @@ static Oid ts_chunks_arg_types[] = { RECORDOID, INT4ARRAYOID };
 static void
 init_chunk_exclusion_func()
 {
-	if (chunk_exclusion_func == InvalidOid)
+	if (!OidIsValid(chunk_exclusion_func))
 	{
 		List *l = list_make2(makeString(INTERNAL_SCHEMA_NAME), makeString(CHUNK_EXCL_FUNC_NAME));
 		chunk_exclusion_func =
 			LookupFuncName(l, lengthof(ts_chunks_arg_types), ts_chunks_arg_types, false);
 	}
-	Assert(chunk_exclusion_func != InvalidOid);
+	Assert(OidIsValid(chunk_exclusion_func));
 }
 
 static bool

diff --git a/src/planner/partialize.c b/src/planner/partialize.c
@@ -112,7 +112,7 @@ has_partialize_function(Query *parse, PartializeAggFixAggref fix_aggref)
 	List *name = list_make2(makeString(INTERNAL_SCHEMA_NAME), makeString(TS_PARTIALFN));
 
 	partialfnoid = LookupFuncName(name, lengthof(argtyp), argtyp, false);
-	Assert(partialfnoid != InvalidOid);
+	Assert(OidIsValid(partialfnoid));
 	state.fnoid = partialfnoid;
 	check_for_partialize_function_call((Node *) parse->targetList, &state);
 

diff --git a/src/planner/planner.c b/src/planner/planner.c
@@ -690,7 +690,7 @@ get_or_add_baserel_from_cache(Oid chunk_reloid, Oid parent_reloid)
 		/*
 		 * This is a chunk. Look up the hypertable for it.
 		 */
-		if (parent_reloid != InvalidOid)
+		if (OidIsValid(parent_reloid))
 		{
 			/* Sanity check on the caller-specified hypertable reloid. */
 			Assert(ts_hypertable_id_to_relid(hypertable_id) == parent_reloid);

diff --git a/src/process_utility.c b/src/process_utility.c
@@ -3891,7 +3891,7 @@ process_create_rule_start(ProcessUtilityArgs *args)
 {
 	RuleStmt *stmt = (RuleStmt *) args->parsetree;
 
-	if (ts_hypertable_relid(stmt->relation) == InvalidOid)
+	if (!OidIsValid(ts_hypertable_relid(stmt->relation)))
 		return DDL_CONTINUE;
 
 	ereport(ERROR,

diff --git a/src/telemetry/functions.c b/src/telemetry/functions.c
@@ -81,7 +81,7 @@ allowed_extension_functions(const char **visible_extensions, int num_visible_ext
 		SysScanDesc scan;
 		Oid extension_id = visible_extension_ids[i];
 
-		if (extension_id == InvalidOid)
+		if (!OidIsValid(extension_id))
 			continue;
 
 		// Look in the (referenced object class, referenced object) index for

diff --git a/src/ts_catalog/catalog.c b/src/ts_catalog/catalog.c
@@ -384,7 +384,7 @@ catalog_database_info_init(CatalogDatabaseInfo *info)
 	info->schema_id = get_namespace_oid(CATALOG_SCHEMA_NAME, false);
 	info->owner_uid = catalog_owner();
 
-	if (info->schema_id == InvalidOid)
+	if (!OidIsValid(info->schema_id))
 		elog(ERROR, "OID lookup failed for schema \"%s\"", CATALOG_SCHEMA_NAME);
 }
 
@@ -426,7 +426,7 @@ ts_catalog_table_info_init(CatalogTableInfo *tables_info, int max_tables,
 		schema_oid = get_namespace_oid(table_ary[i].schema_name, false);
 		id = get_relname_relid(table_ary[i].table_name, schema_oid);
 
-		if (id == InvalidOid)
+		if (!OidIsValid(id))
 			elog(ERROR,
 				 "OID lookup failed for table \"%s.%s\"",
 				 table_ary[i].schema_name,
@@ -441,7 +441,7 @@ ts_catalog_table_info_init(CatalogTableInfo *tables_info, int max_tables,
 		{
 			id = get_relname_relid(index_ary[i].names[j], schema_oid);
 
-			if (id == InvalidOid)
+			if (!OidIsValid(id))
 				elog(ERROR, "OID lookup failed for table index \"%s\"", index_ary[i].names[j]);
 
 			tables_info[i].index_ids[j] = id;

diff --git a/src/ts_catalog/continuous_agg.h b/src/ts_catalog/continuous_agg.h
@@ -26,7 +26,7 @@
 			(newuid) = ts_catalog_database_info_get()->owner_uid;                                  \
 		else                                                                                       \
 			(newuid) = InvalidOid;                                                                 \
-		if ((newuid) != InvalidOid)                                                                \
+		if (OidIsValid((newuid)))                                                                  \
 		{                                                                                          \
 			GetUserIdAndSecContext(&(saved_uid), &(saved_secctx));                                 \
 			SetUserIdAndSecContext(uid, (saved_secctx) | SECURITY_LOCAL_USERID_CHANGE);            \
@@ -36,7 +36,7 @@
 #define RESTORE_USER(newuid, saved_uid, saved_secctx)                                              \
 	do                                                                                             \
 	{                                                                                              \
-		if ((newuid) != InvalidOid)                                                                \
+		if (OidIsValid((newuid)))                                                                  \
 			SetUserIdAndSecContext(saved_uid, saved_secctx);                                       \
 	} while (0);
 

diff --git a/src/utils.h b/src/utils.h
@@ -117,7 +117,7 @@ extern TSDLLEXPORT List *ts_get_reloptions(Oid relid);
 	(to_type *) ts_create_struct_from_slot(slot, mctx, sizeof(to_type), sizeof(form_type));
 
 /* note PG10 has_superclass but PG96 does not so use this */
-#define is_inheritance_child(relid) (ts_inheritance_parent_relid(relid) != InvalidOid)
+#define is_inheritance_child(relid) (OidIsValid(ts_inheritance_parent_relid((relid))))
 
 #define is_inheritance_parent(relid)                                                               \
 	(find_inheritance_children(table_relid, AccessShareLock) != NIL)

diff --git a/tsl/src/bgw_policy/compression_api.c b/tsl/src/bgw_policy/compression_api.c
@@ -148,7 +148,7 @@ validate_compress_after_type(Oid partitioning_type, Oid compress_after_type)
 	{
 		expected_type = INTERVALOID;
 	}
-	if (expected_type != InvalidOid)
+	if (OidIsValid(expected_type))
 	{
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),

diff --git a/tsl/src/chunk_api.c b/tsl/src/chunk_api.c
@@ -690,12 +690,12 @@ convert_strings_to_type_id(Datum *input_strings)
 										input_strings[ENCODED_TYPE_NAMESPACE]);
 	Oid result;
 
-	Assert(arg_namespace != InvalidOid);
+	Assert(OidIsValid(arg_namespace));
 	result = GetSysCacheOid2(TYPENAMENSP,
 							 Anum_pg_type_oid,
 							 input_strings[ENCODED_TYPE_NAME],
 							 ObjectIdGetDatum(arg_namespace));
-	Assert(result != InvalidOid);
+	Assert(OidIsValid(result));
 	return result;
 }
 
@@ -708,14 +708,14 @@ convert_strings_to_op_id(Datum *input_strings)
 	Oid rarg = convert_strings_to_type_id(RargSubarrayForOpArray(input_strings));
 	Oid result;
 
-	Assert(proc_namespace != InvalidOid);
+	Assert(OidIsValid(proc_namespace));
 	result = GetSysCacheOid4(OPERNAMENSP,
 							 Anum_pg_operator_oid,
 							 input_strings[ENCODED_OP_NAME],
 							 ObjectIdGetDatum(larg),
 							 ObjectIdGetDatum(rarg),
 							 ObjectIdGetDatum(proc_namespace));
-	Assert(result != InvalidOid);
+	Assert(OidIsValid(result));
 	return result;
 }
 
@@ -766,7 +766,7 @@ collect_colstat_slots(const HeapTuple tuple, const Form_pg_statistic formdata, D
 		 * and other projects
 		 */
 #define PG_STATS_KINDS_MAX 99
-		if (kind == InvalidOid || kind > PG_STATS_KINDS_MAX)
+		if (!OidIsValid(kind) || kind > PG_STATS_KINDS_MAX)
 		{
 			nulls[numbers_idx] = true;
 			nulls[values_idx] = true;
@@ -995,7 +995,7 @@ chunk_update_colstats(Chunk *chunk, int16 attnum, float nullfract, int32 width,
 		int nelems;
 		Datum *decoded_data;
 
-		if (value_oid == InvalidOid)
+		if (!OidIsValid(value_oid))
 		{
 			nulls[i++] = true;
 			continue;
@@ -1193,7 +1193,7 @@ chunk_process_remote_colstats_row(StatsProcessContext *ctx, TupleFactory *tf, Tu
 		 * for assignment by the core PostgreSQL project. Beyond that are for PostGIS
 		 * and other projects
 		 */
-		if (slot_kinds[i] == InvalidOid || slot_kinds[i] > PG_STATS_KINDS_MAX)
+		if (!OidIsValid(slot_kinds[i]) || slot_kinds[i] > PG_STATS_KINDS_MAX)
 			continue;
 
 		for (k = 0; k < STRINGS_PER_OP_OID; ++k)

diff --git a/tsl/src/compression/api.c b/tsl/src/compression/api.c
@@ -319,7 +319,7 @@ find_chunk_to_merge_into(Hypertable *ht, Chunk *current_chunk)
 	/* If there is no previous adjacent chunk along the time dimension or
 	 * if it hasn't been compressed yet, we can't merge.
 	 */
-	if (!previous_chunk || previous_chunk->fd.compressed_chunk_id == InvalidOid)
+	if (!previous_chunk || !OidIsValid(previous_chunk->fd.compressed_chunk_id))
 		return NULL;
 
 	Assert(previous_chunk->cube->num_slices > 0);

diff --git a/tsl/src/compression/compression.c b/tsl/src/compression/compression.c
@@ -697,7 +697,7 @@ get_sequence_number_for_current_group(Relation table_rel, Oid index_oid,
 		return SEQUENCE_NUM_GAP;
 
 	/* If there is a suitable index, use index scan otherwise fallback to heap scan. */
-	bool is_index_scan = index_oid != InvalidOid;
+	bool is_index_scan = OidIsValid(index_oid);
 
 	int i, num_scankeys = 0;
 	int32 result = 0;

diff --git a/tsl/src/compression/create.c b/tsl/src/compression/create.c
@@ -303,7 +303,7 @@ compresscolinfo_init(CompressColInfo *cc, Oid srctbl_relid, List *segmentby_cols
 				cc->col_meta[colno].orderby_nullsfirst = ordercol->nullsfirst;
 			}
 		}
-		if (attroid == InvalidOid)
+		if (!OidIsValid(attroid))
 		{
 			attroid = compresseddata_oid; /* default type for column */
 			cc->col_meta[colno].algo_id = get_default_algorithm_id(attr->atttypid);

diff --git a/tsl/src/continuous_aggs/create.c b/tsl/src/continuous_aggs/create.c
@@ -934,8 +934,8 @@ cagg_agg_validate(Node *node, void *context)
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 					 errmsg("ordered set/hypothetical aggregates are not supported")));
 		}
-		if (aggform->aggcombinefn == InvalidOid ||
-			(aggform->aggtranstype == INTERNALOID && aggform->aggdeserialfn == InvalidOid))
+		if (!OidIsValid(aggform->aggcombinefn) ||
+			(aggform->aggtranstype == INTERNALOID && !OidIsValid(aggform->aggdeserialfn)))
 		{
 			ReleaseSysCache(aggtuple);
 			ereport(ERROR,

diff --git a/tsl/src/data_node.c b/tsl/src/data_node.c
@@ -1314,7 +1314,7 @@ data_node_block_or_allow_new_chunks(const char *node_name, Oid const table_id, b
 									bool block_chunks)
 {
 	int affected = 0;
-	bool all_hypertables = table_id == InvalidOid ? true : false;
+	bool all_hypertables = !OidIsValid(table_id);
 	List *hypertable_data_nodes = NIL;
 	ForeignServer *server = data_node_get_foreign_server(node_name, ACL_USAGE, true, false);