From 54d4c3de6bbb52421b15f6035db662f683059c86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabr=C3=ADzio=20de=20Royes=20Mello?= Date: Mon, 22 May 2023 16:35:45 -0300 Subject: [PATCH] Introduce utility function ts_get_relation_relid In several places of our code base we use a combination of `get_namespace_oid` and `get_relname_relid` to return the Oid of a schema qualified relation, so refactored the code to encapsulate this behavior in a single function. --- src/chunk.c | 12 ++++-------- src/chunk_scan.c | 6 +++--- src/extension_utils.c | 8 ++------ src/hypertable.c | 5 ++--- src/process_utility.c | 3 +-- src/ts_catalog/catalog.c | 22 ++++++++++------------ src/ts_catalog/continuous_agg.c | 13 +++++++------ src/utils.h | 15 +++++++++++++++ test/src/bgw/log.c | 3 ++- test/src/bgw/params.c | 3 ++- tsl/src/bgw_policy/job.c | 11 +++++------ tsl/src/bgw_policy/reorder_api.c | 3 +-- tsl/src/continuous_aggs/create.c | 30 +++++++++++++++--------------- 13 files changed, 69 insertions(+), 65 deletions(-) diff --git a/src/chunk.c b/src/chunk.c index d65bfd107..7a33067cf 100644 --- a/src/chunk.c +++ b/src/chunk.c @@ -1691,8 +1691,8 @@ chunk_tuple_found(TupleInfo *ti, void *arg) * ts_chunk_build_from_tuple_and_stub() since chunk_resurrect() also uses * that function and, in that case, the chunk object is needed to create * the data table and related objects. */ - chunk->table_id = get_relname_relid(chunk->fd.table_name.data, - get_namespace_oid(chunk->fd.schema_name.data, true)); + chunk->table_id = + ts_get_relation_relid(NameStr(chunk->fd.schema_name), NameStr(chunk->fd.table_name), true); chunk->hypertable_relid = ts_hypertable_id_to_relid(chunk->fd.hypertable_id); chunk->relkind = get_rel_relkind(chunk->table_id); @@ -2804,12 +2804,8 @@ ts_chunk_get_relid(int32 chunk_id, bool missing_ok) Oid relid = InvalidOid; if (chunk_simple_scan_by_id(chunk_id, &form, missing_ok)) - { - Oid schemaid = get_namespace_oid(NameStr(form.schema_name), missing_ok); - - if (OidIsValid(schemaid)) - relid = get_relname_relid(NameStr(form.table_name), schemaid); - } + relid = + ts_get_relation_relid(NameStr(form.schema_name), NameStr(form.table_name), missing_ok); if (!OidIsValid(relid) && !missing_ok) ereport(ERROR, diff --git a/src/chunk_scan.c b/src/chunk_scan.c index 740e59a43..0a561fb5f 100644 --- a/src/chunk_scan.c +++ b/src/chunk_scan.c @@ -19,6 +19,7 @@ #include "chunk.h" #include "chunk_constraint.h" #include "ts_catalog/chunk_data_node.h" +#include "utils.h" /* * Scan for chunks matching a query. @@ -117,7 +118,6 @@ ts_chunk_scan_by_chunk_ids(const Hyperspace *hs, const List *chunk_ids, unsigned * Schema oid isn't likely to change, so cache it. */ char *last_schema_name = NULL; - Oid last_schema_oid = InvalidOid; for (int i = 0; i < unlocked_chunk_count; i++) { Chunk *chunk = unlocked_chunks[i]; @@ -126,10 +126,10 @@ ts_chunk_scan_by_chunk_ids(const Hyperspace *hs, const List *chunk_ids, unsigned if (last_schema_name == NULL || strcmp(last_schema_name, current_schema_name) != 0) { last_schema_name = current_schema_name; - last_schema_oid = get_namespace_oid(current_schema_name, false); } - chunk->table_id = get_relname_relid(NameStr(chunk->fd.table_name), last_schema_oid); + chunk->table_id = + ts_get_relation_relid(last_schema_name, NameStr(chunk->fd.table_name), false); Assert(OidIsValid(chunk->table_id)); } diff --git a/src/extension_utils.c b/src/extension_utils.c index 0c2880e29..588775f07 100644 --- a/src/extension_utils.c +++ b/src/extension_utils.c @@ -28,6 +28,7 @@ #include "compat/compat.h" #include "extension_constants.h" +#include "utils.h" #define EXTENSION_PROXY_TABLE "cache_inval_extension" @@ -114,12 +115,7 @@ extension_version(void) static Oid get_proxy_table_relid() { - Oid nsid = get_namespace_oid(CACHE_SCHEMA_NAME, true); - - if (!OidIsValid(nsid)) - return InvalidOid; - - return get_relname_relid(EXTENSION_PROXY_TABLE, nsid); + return ts_get_relation_relid(CACHE_SCHEMA_NAME, EXTENSION_PROXY_TABLE, true); } inline static bool diff --git a/src/hypertable.c b/src/hypertable.c index 23d9247f1..fb311935c 100644 --- a/src/hypertable.c +++ b/src/hypertable.c @@ -243,12 +243,11 @@ ts_hypertable_formdata_fill(FormData_hypertable *fd, const TupleInfo *ti) Hypertable * ts_hypertable_from_tupleinfo(const TupleInfo *ti) { - Oid namespace_oid; Hypertable *h = MemoryContextAllocZero(ti->mctx, sizeof(Hypertable)); ts_hypertable_formdata_fill(&h->fd, ti); - namespace_oid = get_namespace_oid(NameStr(h->fd.schema_name), false); - h->main_table_relid = get_relname_relid(NameStr(h->fd.table_name), namespace_oid); + h->main_table_relid = + ts_get_relation_relid(NameStr(h->fd.schema_name), NameStr(h->fd.table_name), false); h->space = ts_dimension_scan(h->fd.id, h->main_table_relid, h->fd.num_dimensions, ti->mctx); h->chunk_cache = ts_subspace_store_init(h->space, ti->mctx, ts_guc_max_cached_chunks_per_hypertable); diff --git a/src/process_utility.c b/src/process_utility.c index 2cc29393b..64a938621 100644 --- a/src/process_utility.c +++ b/src/process_utility.c @@ -3178,8 +3178,7 @@ process_alter_column_type_end(Hypertable *ht, AlterTableCmd *cmd) static void process_altertable_clusteron_end(Hypertable *ht, AlterTableCmd *cmd) { - Oid index_relid = - get_relname_relid(cmd->name, get_namespace_oid(NameStr(ht->fd.schema_name), false)); + Oid index_relid = ts_get_relation_relid(NameStr(ht->fd.schema_name), cmd->name, false); /* If this is part of changing the type of a column that is used in a clustered index * the above lookup might fail. But in this case we don't need to mark the index clustered diff --git a/src/ts_catalog/catalog.c b/src/ts_catalog/catalog.c index 03edc7608..6eb520c6d 100644 --- a/src/ts_catalog/catalog.c +++ b/src/ts_catalog/catalog.c @@ -22,6 +22,7 @@ #include "ts_catalog/catalog.h" #include "extension.h" #include "cache_invalidate.h" +#include "utils.h" static const TableInfoDef catalog_table_names[_MAX_CATALOG_TABLES + 1] = { [HYPERTABLE] = { @@ -433,13 +434,13 @@ ts_catalog_table_info_init(CatalogTableInfo *tables_info, int max_tables, for (i = 0; i < max_tables; i++) { - Oid schema_oid; Oid id; const char *sequence_name; Size number_indexes, j; - schema_oid = get_namespace_oid(table_ary[i].schema_name, false); - id = get_relname_relid(table_ary[i].table_name, schema_oid); + id = ts_get_relation_relid((char *) table_ary[i].schema_name, + (char *) table_ary[i].table_name, + false); if (!OidIsValid(id)) elog(ERROR, @@ -454,7 +455,9 @@ ts_catalog_table_info_init(CatalogTableInfo *tables_info, int max_tables, for (j = 0; j < number_indexes; j++) { - id = get_relname_relid(index_ary[i].names[j], schema_oid); + id = ts_get_relation_relid((char *) table_ary[i].schema_name, + (char *) index_ary[i].names[j], + true); if (!OidIsValid(id)) elog(ERROR, "OID lookup failed for table index \"%s\"", index_ary[i].names[j]); @@ -600,8 +603,6 @@ ts_catalog_get_cache_proxy_id(Catalog *catalog, CacheType type) { if (!catalog_is_valid(catalog)) { - Oid schema; - /* * The catalog can be invalid during upgrade scripts. Try a non-cached * relation lookup, but we need to be in a transaction for @@ -610,12 +611,9 @@ ts_catalog_get_cache_proxy_id(Catalog *catalog, CacheType type) if (!IsTransactionState()) return InvalidOid; - schema = get_namespace_oid(CACHE_SCHEMA_NAME, true); - - if (!OidIsValid(schema)) - return InvalidOid; - - return get_relname_relid(cache_proxy_table_names[type], schema); + return ts_get_relation_relid(CACHE_SCHEMA_NAME, + (char *) cache_proxy_table_names[type], + true); } return catalog->caches[type].inval_proxy_id; diff --git a/src/ts_catalog/continuous_agg.c b/src/ts_catalog/continuous_agg.c index 3900f4bfa..7aaae4f25 100644 --- a/src/ts_catalog/continuous_agg.c +++ b/src/ts_catalog/continuous_agg.c @@ -39,6 +39,7 @@ #include "ts_catalog/catalog.h" #include "errors.h" #include "compression_with_clause.h" +#include "utils.h" #define BUCKET_FUNCTION_SERIALIZE_VERSION 1 #define CHECK_NAME_MATCH(name1, name2) (namestrcmp(name1, name2) == 0) @@ -1824,13 +1825,13 @@ ts_continuous_agg_get_query(ContinuousAgg *cagg) * the user view doesn't have the "GROUP BY" clause anymore. */ if (ContinuousAggIsFinalized(cagg)) - cagg_view_oid = - get_relname_relid(NameStr(cagg->data.direct_view_name), - get_namespace_oid(NameStr(cagg->data.direct_view_schema), false)); + cagg_view_oid = ts_get_relation_relid(NameStr(cagg->data.direct_view_schema), + NameStr(cagg->data.direct_view_name), + false); else - cagg_view_oid = - get_relname_relid(NameStr(cagg->data.user_view_name), - get_namespace_oid(NameStr(cagg->data.user_view_schema), false)); + cagg_view_oid = ts_get_relation_relid(NameStr(cagg->data.user_view_schema), + NameStr(cagg->data.user_view_name), + false); cagg_view_rel = table_open(cagg_view_oid, AccessShareLock); cagg_view_rules = cagg_view_rel->rd_rules; diff --git a/src/utils.h b/src/utils.h index 7908271b4..0406922d4 100644 --- a/src/utils.h +++ b/src/utils.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -215,4 +216,18 @@ extern TSDLLEXPORT bool ts_data_node_is_available(const char *node_name); extern TSDLLEXPORT AttrNumber ts_map_attno(Oid src_rel, Oid dst_rel, AttrNumber attno); +/* + * Return Oid for a schema-qualified relation. + */ +static inline Oid +ts_get_relation_relid(char *schema_name, char *relation_name, bool schema_missing_ok) +{ + Oid schema_oid = get_namespace_oid(schema_name, schema_missing_ok); + + if (OidIsValid(schema_oid)) + return get_relname_relid(relation_name, schema_oid); + + return InvalidOid; +} + #endif /* TIMESCALEDB_UTILS_H */ diff --git a/test/src/bgw/log.c b/test/src/bgw/log.c index 0bda01ff4..8ce908e07 100644 --- a/test/src/bgw/log.c +++ b/test/src/bgw/log.c @@ -15,6 +15,7 @@ #include "scanner.h" #include "params.h" #include "ts_catalog/catalog.h" +#include "utils.h" #include "compat/compat.h" @@ -52,7 +53,7 @@ static void bgw_log_insert(char *msg) { Relation rel; - Oid log_oid = get_relname_relid("bgw_log", get_namespace_oid("public", false)); + Oid log_oid = ts_get_relation_relid("public", "bgw_log", false); rel = table_open(log_oid, RowExclusiveLock); bgw_log_insert_relation(rel, msg); diff --git a/test/src/bgw/params.c b/test/src/bgw/params.c index f41ac868b..aae6dd145 100644 --- a/test/src/bgw/params.c +++ b/test/src/bgw/params.c @@ -21,6 +21,7 @@ #include "log.h" #include "scanner.h" #include "ts_catalog/catalog.h" +#include "utils.h" typedef struct FormData_bgw_dsm_handle { @@ -37,7 +38,7 @@ typedef struct TestParamsWrapper static Oid get_dsm_handle_table_oid() { - return get_relname_relid("bgw_dsm_handle_store", get_namespace_oid("public", false)); + return ts_get_relation_relid("public", "bgw_dsm_handle_store", false); } static void diff --git a/tsl/src/bgw_policy/job.c b/tsl/src/bgw_policy/job.c index 87300d202..c17fc8fd1 100644 --- a/tsl/src/bgw_policy/job.c +++ b/tsl/src/bgw_policy/job.c @@ -185,8 +185,7 @@ check_valid_index(Hypertable *ht, const char *index_name) HeapTuple idxtuple; Form_pg_index index_form; - index_oid = - get_relname_relid(index_name, get_namespace_oid(NameStr(ht->fd.schema_name), false)); + index_oid = ts_get_relation_relid(NameStr(ht->fd.schema_name), (char *) index_name, false); idxtuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(index_oid)); if (!HeapTupleIsValid(idxtuple)) ereport(ERROR, @@ -266,7 +265,7 @@ policy_reorder_read_and_validate_config(Jsonb *config, PolicyReorderData *policy { policy->hypertable = ht; policy->index_relid = - get_relname_relid(index_name, get_namespace_oid(NameStr(ht->fd.schema_name), false)); + ts_get_relation_relid(NameStr(ht->fd.schema_name), (char *) index_name, false); } } @@ -316,9 +315,9 @@ policy_retention_read_and_validate_config(Jsonb *config, PolicyRetentionData *po cagg = ts_continuous_agg_find_by_mat_hypertable_id(hypertable->fd.id); if (cagg) { - const char *const view_name = NameStr(cagg->data.user_view_name); - const char *const schema_name = NameStr(cagg->data.user_view_schema); - object_relid = get_relname_relid(view_name, get_namespace_oid(schema_name, false)); + object_relid = ts_get_relation_relid(NameStr(cagg->data.user_view_schema), + NameStr(cagg->data.user_view_name), + false); } ts_cache_release(hcache); diff --git a/tsl/src/bgw_policy/reorder_api.c b/tsl/src/bgw_policy/reorder_api.c index 66410759a..0c45c1d87 100644 --- a/tsl/src/bgw_policy/reorder_api.c +++ b/tsl/src/bgw_policy/reorder_api.c @@ -91,8 +91,7 @@ check_valid_index(Hypertable *ht, Name index_name) HeapTuple idxtuple; Form_pg_index indexForm; - index_oid = get_relname_relid(NameStr(*index_name), - get_namespace_oid(NameStr(ht->fd.schema_name), false)); + index_oid = ts_get_relation_relid(NameStr(ht->fd.schema_name), NameStr(*index_name), false); idxtuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(index_oid)); if (!HeapTupleIsValid(idxtuple)) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid reorder index"))); diff --git a/tsl/src/continuous_aggs/create.c b/tsl/src/continuous_aggs/create.c index 24d06117a..30dd345b7 100644 --- a/tsl/src/continuous_aggs/create.c +++ b/tsl/src/continuous_aggs/create.c @@ -227,7 +227,6 @@ static bool function_allowed_in_cagg_definition(Oid funcid); static Const *cagg_boundary_make_lower_bound(Oid type); static Oid cagg_get_boundary_converter_funcoid(Oid typoid); -static Oid relation_oid(NameData schema, NameData name); static Query *build_union_query(CAggTimebucketInfo *tbinfo, int matpartcolno, Query *q1, Query *q2, int materialize_htid); static Query *destroy_union_query(Query *q); @@ -3013,7 +3012,9 @@ cagg_rebuild_view_definition(ContinuousAgg *agg, Hypertable *mat_ht, bool force_ Oid uid, saved_uid; /* Cagg view created by the user. */ - Oid user_view_oid = relation_oid(agg->data.user_view_schema, agg->data.user_view_name); + Oid user_view_oid = ts_get_relation_relid(NameStr(agg->data.user_view_schema), + NameStr(agg->data.user_view_name), + false); Relation user_view_rel = relation_open(user_view_oid, AccessShareLock); Query *user_query = get_view_query(user_view_rel); @@ -3049,7 +3050,9 @@ cagg_rebuild_view_definition(ContinuousAgg *agg, Hypertable *mat_ht, bool force_ .objectId = mat_ht->main_table_relid, }; - Oid direct_view_oid = relation_oid(agg->data.direct_view_schema, agg->data.direct_view_name); + Oid direct_view_oid = ts_get_relation_relid(NameStr(agg->data.direct_view_schema), + NameStr(agg->data.direct_view_name), + false); Relation direct_view_rel = relation_open(direct_view_oid, AccessShareLock); Query *direct_query = copyObject(get_view_query(direct_view_rel)); remove_old_and_new_rte_from_query(direct_query); @@ -3243,7 +3246,9 @@ cagg_flip_realtime_view_definition(ContinuousAgg *agg, Hypertable *mat_ht) Query *result_view_query; /* User view query of the user defined CAGG. */ - Oid user_view_oid = relation_oid(agg->data.user_view_schema, agg->data.user_view_name); + Oid user_view_oid = ts_get_relation_relid(NameStr(agg->data.user_view_schema), + NameStr(agg->data.user_view_name), + false); Relation user_view_rel = relation_open(user_view_oid, AccessShareLock); Query *user_query = copyObject(get_view_query(user_view_rel)); /* Keep lock until end of transaction. */ @@ -3251,7 +3256,9 @@ cagg_flip_realtime_view_definition(ContinuousAgg *agg, Hypertable *mat_ht) remove_old_and_new_rte_from_query(user_query); /* Direct view query of the original user view definition at CAGG creation. */ - Oid direct_view_oid = relation_oid(agg->data.direct_view_schema, agg->data.direct_view_name); + Oid direct_view_oid = ts_get_relation_relid(NameStr(agg->data.direct_view_schema), + NameStr(agg->data.direct_view_name), + false); Relation direct_view_rel = relation_open(direct_view_oid, AccessShareLock); Query *direct_query = copyObject(get_view_query(direct_view_rel)); /* Keep lock until end of transaction. */ @@ -3294,7 +3301,9 @@ cagg_rename_view_columns(ContinuousAgg *agg) Oid uid, saved_uid; /* User view query of the user defined CAGG. */ - Oid user_view_oid = relation_oid(agg->data.user_view_schema, agg->data.user_view_name); + Oid user_view_oid = ts_get_relation_relid(NameStr(agg->data.user_view_schema), + NameStr(agg->data.user_view_name), + false); Relation user_view_rel = relation_open(user_view_oid, AccessShareLock); Query *user_query = copyObject(get_view_query(user_view_rel)); remove_old_and_new_rte_from_query(user_query); @@ -3721,12 +3730,3 @@ destroy_union_query(Query *q) return query; } - -/* - * Return Oid for a schema-qualified relation. - */ -static Oid -relation_oid(NameData schema, NameData name) -{ - return get_relname_relid(NameStr(name), get_namespace_oid(NameStr(schema), false)); -}