From d26c7441154b7b890cb8d694c20e96b07e10aa03 Mon Sep 17 00:00:00 2001 From: Sven Klemm Date: Tue, 13 Apr 2021 11:41:10 +0200 Subject: [PATCH] Use %u to format Oid instead of %d Since Oid is unsigned int we have to use %u to print it otherwise oids >= 2^31 will not work correctly. This also switches the places that print type oid to use format helper functions to resolve the oids. --- src/bgw/scheduler.c | 2 +- src/hypertable_restrict_info.c | 5 ++++- src/plan_agg_bookend.c | 8 +++++--- src/time_bucket.c | 11 ++++++----- src/utils.c | 31 +++++++++++++++++++------------ test/src/bgw/scheduler_mock.c | 4 ++-- tsl/src/bgw_policy/job_api.c | 2 +- tsl/src/compression/compression.c | 10 ++++++---- tsl/src/compression/create.c | 2 +- tsl/src/compression/deltadelta.c | 8 ++++++-- tsl/src/compression/gorilla.c | 7 +++++-- tsl/src/continuous_aggs/create.c | 2 +- tsl/src/deparse.c | 2 +- tsl/test/expected/bgw_custom.out | 4 +++- tsl/test/sql/bgw_custom.sql | 2 ++ 15 files changed, 63 insertions(+), 37 deletions(-) diff --git a/src/bgw/scheduler.c b/src/bgw/scheduler.c index e0611b82a..3d7430117 100644 --- a/src/bgw/scheduler.c +++ b/src/bgw/scheduler.c @@ -747,7 +747,7 @@ ts_bgw_scheduler_process(int32 run_for_interval_ms, if (run_for_interval_ms > 0) quit_time = TimestampTzPlusMilliseconds(start, run_for_interval_ms); - ereport(DEBUG1, (errmsg("database scheduler starting for database %d", MyDatabaseId))); + ereport(DEBUG1, (errmsg("database scheduler starting for database %u", MyDatabaseId))); /* * on SIGTERM the process will usually die from the CHECK_FOR_INTERRUPTS diff --git a/src/hypertable_restrict_info.c b/src/hypertable_restrict_info.c index 7f7ff70d7..eac220e30 100644 --- a/src/hypertable_restrict_info.c +++ b/src/hypertable_restrict_info.c @@ -8,6 +8,7 @@ #include #include #include +#include #include "compat.h" #if PG12_LT @@ -442,7 +443,9 @@ 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) - elog(ERROR, "Couldn't get base element type from array type: %d", c->consttype); + elog(ERROR, + "invalid base element type for array type: \"%s\"", + format_type_be(c->consttype)); return dimension_values_create(values, base_el_type, user_or); } diff --git a/src/plan_agg_bookend.c b/src/plan_agg_bookend.c index 5394cf5d4..26536e657 100644 --- a/src/plan_agg_bookend.c +++ b/src/plan_agg_bookend.c @@ -52,7 +52,9 @@ #include #include #include +#include #include +#include #include #include @@ -484,9 +486,9 @@ find_first_last_aggs_walker(Node *node, List **context) get_opfamily_member(sort_tce->btree_opf, sort_oid, sort_oid, func_strategy->strategy); if (aggsortop == InvalidOid) elog(ERROR, - "Can't resolve sort operator oid for function oid: %d and type: %d", - aggref->aggfnoid, - sort_oid); + "Cannot resolve sort operator for function \"%s\" and type \"%s\"", + format_procedure(aggref->aggfnoid), + format_type_be(sort_oid)); /* Used in projection */ value = (TargetEntry *) linitial(aggref->args); diff --git a/src/time_bucket.c b/src/time_bucket.c index 3cfb20af1..005482f97 100644 --- a/src/time_bucket.c +++ b/src/time_bucket.c @@ -5,11 +5,12 @@ */ #include #include -#include -#include -#include -#include #include +#include +#include +#include +#include +#include #include "utils.h" #include "time_bucket.h" @@ -283,7 +284,7 @@ ts_time_bucket_by_type(int64 interval, int64 timestamp, Oid timestamp_type) bucket_function = ts_date_bucket; break; default: - elog(ERROR, "invalid time_bucket Oid %d", timestamp_type); + elog(ERROR, "invalid time_bucket type \"%s\"", format_type_be(timestamp_type)); } time_bucketed = diff --git a/src/utils.c b/src/utils.c index 364278c34..db3a39f98 100644 --- a/src/utils.c +++ b/src/utils.c @@ -4,24 +4,25 @@ * LICENSE-APACHE for a copy of the license. */ #include -#include -#include #include #include #include #include -#include #include +#include #include #include #include #include #include #include +#include +#include #include -#include #include +#include #include +#include #include #include #include @@ -126,7 +127,7 @@ ts_time_value_to_internal(Datum time_val, Oid type_oid) if (ts_type_is_int8_binary_compatible(type_oid)) return DatumGetInt64(time_val); - elog(ERROR, "unknown time type OID %d", type_oid); + elog(ERROR, "unknown time type \"%s\"", format_type_be(type_oid)); } if (IS_INTEGER_TYPE(type_oid)) @@ -171,7 +172,7 @@ ts_time_value_to_internal(Datum time_val, Oid type_oid) return DatumGetInt64(res); default: - elog(ERROR, "unknown time type OID %d", type_oid); + elog(ERROR, "unknown time type \"%s\"", format_type_be(type_oid)); return -1; } } @@ -197,7 +198,7 @@ ts_interval_value_to_internal(Datum time_val, Oid type_oid) return interval->time + (interval->day * USECS_PER_DAY); } default: - elog(ERROR, "unknown interval type OID %d", type_oid); + elog(ERROR, "unknown interval type \"%s\"", format_type_be(type_oid)); return -1; } } @@ -214,7 +215,7 @@ ts_integer_to_internal(Datum time_val, Oid type_oid) case INT2OID: return (int64) DatumGetInt16(time_val); default: - elog(ERROR, "unknown interval type OID %d", type_oid); + elog(ERROR, "unknown interval type \"%s\"", format_type_be(type_oid)); return -1; } } @@ -335,7 +336,9 @@ ts_internal_to_time_value(int64 value, Oid type) default: if (ts_type_is_int8_binary_compatible(type)) return Int64GetDatum(value); - elog(ERROR, "unknown time type OID %d in ts_internal_to_time_value", type); + elog(ERROR, + "unknown time type \"%s\" in ts_internal_to_time_value", + format_type_be(type)); pg_unreachable(); } } @@ -377,7 +380,9 @@ ts_internal_to_interval_value(int64 value, Oid type) case INTERVALOID: return DirectFunctionCall1(ts_pg_unix_microseconds_to_interval, Int64GetDatum(value)); default: - elog(ERROR, "unknown time type OID %d in ts_internal_to_interval_value", type); + elog(ERROR, + "unknown time type \"%s\" in ts_internal_to_interval_value", + format_type_be(type)); pg_unreachable(); } } @@ -394,7 +399,9 @@ ts_integer_to_internal_value(int64 value, Oid type) case INT8OID: return Int64GetDatum(value); default: - elog(ERROR, "unknown time type OID %d in ts_internal_to_time_value", type); + elog(ERROR, + "unknown time type \"%s\" in ts_internal_to_time_value", + format_type_be(type)); pg_unreachable(); } } @@ -749,7 +756,7 @@ ts_has_row_security(Oid relid) /* Fetch relation's relrowsecurity and relforcerowsecurity flags */ tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid)); if (!HeapTupleIsValid(tuple)) - elog(ERROR, "cache lookup failed for relid %d", relid); + elog(ERROR, "cache lookup failed for relid %u", relid); classform = (Form_pg_class) GETSTRUCT(tuple); relrowsecurity = classform->relrowsecurity; relforcerowsecurity = classform->relforcerowsecurity; diff --git a/test/src/bgw/scheduler_mock.c b/test/src/bgw/scheduler_mock.c index 683c99ee8..576c2a367 100644 --- a/test/src/bgw/scheduler_mock.c +++ b/test/src/bgw/scheduler_mock.c @@ -120,8 +120,8 @@ ts_bgw_db_scheduler_test_main(PG_FUNCTION_ARGS) deserialize_test_parameters(MyBgworkerEntry->bgw_extra, &ttl, &user_oid); - elog(WARNING, "scheduler user id %d", user_oid); - elog(WARNING, "running a test in the background: db=%d ttl=%d", db_oid, ttl); + elog(WARNING, "scheduler user id %u", user_oid); + elog(WARNING, "running a test in the background: db=%u ttl=%d", db_oid, ttl); BackgroundWorkerInitializeConnectionByOid(db_oid, user_oid, 0); diff --git a/tsl/src/bgw_policy/job_api.c b/tsl/src/bgw_policy/job_api.c index 536ce6927..d657b2f2a 100644 --- a/tsl/src/bgw_policy/job_api.c +++ b/tsl/src/bgw_policy/job_api.c @@ -93,7 +93,7 @@ job_add(PG_FUNCTION_ARGS) if (func_name == NULL) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("function or procedure with OID %d does not exist", proc))); + errmsg("function or procedure with OID %u does not exist", proc))); if (pg_proc_aclcheck(proc, owner, ACL_EXECUTE) != ACLCHECK_OK) ereport(ERROR, diff --git a/tsl/src/compression/compression.c b/tsl/src/compression/compression.c index 95af7cb3d..fd0819795 100644 --- a/tsl/src/compression/compression.c +++ b/tsl/src/compression/compression.c @@ -453,7 +453,10 @@ compress_chunk_populate_sort_info_for_column(Oid table, const ColumnCompressionI tp = SearchSysCacheAttName(table, NameStr(column->attname)); if (!HeapTupleIsValid(tp)) - elog(ERROR, "table %d does not have column \"%s\"", table, NameStr(column->attname)); + elog(ERROR, + "table \"%s\" does not have column \"%s\"", + get_rel_name(table), + NameStr(column->attname)); att_tup = (Form_pg_attribute) GETSTRUCT(tp); /* Other valdation checks beyond just existence of a valid comparison operator could be useful @@ -1488,9 +1491,8 @@ update_compressed_chunk_relstats(Oid uncompressed_relid, Oid compressed_relid) { ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR), - errmsg("mismatched chunks for relstats update %d %d", - uncompressed_relid, - compressed_relid))); + errmsg("mismatched chunks for relstats update on compressed chunk \"%s\"", + get_rel_name(uncompressed_relid)))); } capture_pgclass_stats(uncompressed_relid, &uncomp_pages, &uncomp_visible, &uncomp_tuples); diff --git a/tsl/src/compression/create.c b/tsl/src/compression/create.c index a2ab4d951..e60c92ad4 100644 --- a/tsl/src/compression/create.c +++ b/tsl/src/compression/create.c @@ -451,7 +451,7 @@ create_compressed_table_indexes(Oid compresstable_relid, CompressColInfo *compre index_tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(index_addr.objectId)); if (!HeapTupleIsValid(index_tuple)) - elog(ERROR, "cache lookup failed for index relid %d", index_addr.objectId); + elog(ERROR, "cache lookup failed for index relid %u", index_addr.objectId); index_name = ((Form_pg_class) GETSTRUCT(index_tuple))->relname; elog(DEBUG1, "adding index %s ON %s.%s USING BTREE(%s, %s)", diff --git a/tsl/src/compression/deltadelta.c b/tsl/src/compression/deltadelta.c index 36a0f8aa8..52319790e 100644 --- a/tsl/src/compression/deltadelta.c +++ b/tsl/src/compression/deltadelta.c @@ -235,7 +235,9 @@ delta_delta_compressor_for_type(Oid element_type) *compressor = (ExtendedCompressor){ .base = deltadelta_timestamptz_compressor }; return &compressor->base; default: - elog(ERROR, "invalid type for delta-delta compressor %d", element_type); + elog(ERROR, + "invalid type for delta-delta compressor \"%s\"", + format_type_be(element_type)); } pg_unreachable(); @@ -506,7 +508,9 @@ convert_from_internal(DecompressResultInternal res_internal, Oid element_type) .val = TimestampGetDatum(res_internal.val), }; default: - elog(ERROR, "invalid type requested from deltadelta decompression %d", element_type); + elog(ERROR, + "invalid type requested from deltadelta decompression \"%s\"", + format_type_be(element_type)); } pg_unreachable(); diff --git a/tsl/src/compression/gorilla.c b/tsl/src/compression/gorilla.c index c22450204..b0f861ac5 100644 --- a/tsl/src/compression/gorilla.c +++ b/tsl/src/compression/gorilla.c @@ -8,8 +8,9 @@ #include #include #include -#include #include +#include +#include #include #include "compat.h" @@ -325,7 +326,9 @@ gorilla_compressor_for_type(Oid element_type) *compressor = (ExtendedCompressor){ .base = gorilla_uint64_compressor }; return &compressor->base; default: - elog(ERROR, "invalid type for Gorilla compression %d", element_type); + elog(ERROR, + "invalid type for Gorilla compression \"%s\"", + format_type_be(element_type)); } pg_unreachable(); } diff --git a/tsl/src/continuous_aggs/create.c b/tsl/src/continuous_aggs/create.c index 92e7a937c..3b4a8ec68 100644 --- a/tsl/src/continuous_aggs/create.c +++ b/tsl/src/continuous_aggs/create.c @@ -428,7 +428,7 @@ mattablecolumninfo_add_mattable_index(MatTableColumnInfo *matcolinfo, Hypertable indxtuple = SearchSysCache1(RELOID, ObjectIdGetDatum(indxaddr.objectId)); if (!HeapTupleIsValid(indxtuple)) - elog(ERROR, "cache lookup failed for index relid %d", indxaddr.objectId); + elog(ERROR, "cache lookup failed for index relid %u", indxaddr.objectId); indxname = ((Form_pg_class) GETSTRUCT(indxtuple))->relname; elog(DEBUG1, "adding index %s ON %s.%s USING BTREE(%s, %s)", diff --git a/tsl/src/deparse.c b/tsl/src/deparse.c index f184f8b2d..1643bd310 100644 --- a/tsl/src/deparse.c +++ b/tsl/src/deparse.c @@ -331,7 +331,7 @@ deparse_create_table_info(Oid relid) Relation rel = table_open(relid, AccessShareLock); if (rel == NULL) - ereport(ERROR, (errmsg("relation with id %d not found", relid))); + ereport(ERROR, (errmsg("relation with id %u not found", relid))); validate_relation(rel); diff --git a/tsl/test/expected/bgw_custom.out b/tsl/test/expected/bgw_custom.out index 9c36c6fe7..af3f27d57 100644 --- a/tsl/test/expected/bgw_custom.out +++ b/tsl/test/expected/bgw_custom.out @@ -32,8 +32,10 @@ SELECT add_job(NULL, '1h'); ERROR: function or procedure cannot be NULL SELECT add_job(0, '1h'); ERROR: function or procedure with OID 0 does not exist +-- this will return an error about Oid 4294967295 +-- while regproc is unsigned int postgres has an implicit cast from int to regproc SELECT add_job(-1, '1h'); -ERROR: function or procedure with OID -1 does not exist +ERROR: function or procedure with OID 4294967295 does not exist SELECT add_job('invalid_func', '1h'); ERROR: function "invalid_func" does not exist at character 16 SELECT add_job('custom_func', NULL); diff --git a/tsl/test/sql/bgw_custom.sql b/tsl/test/sql/bgw_custom.sql index c297da990..39f6a0ce1 100644 --- a/tsl/test/sql/bgw_custom.sql +++ b/tsl/test/sql/bgw_custom.sql @@ -36,6 +36,8 @@ $$; -- test bad input SELECT add_job(NULL, '1h'); SELECT add_job(0, '1h'); +-- this will return an error about Oid 4294967295 +-- while regproc is unsigned int postgres has an implicit cast from int to regproc SELECT add_job(-1, '1h'); SELECT add_job('invalid_func', '1h'); SELECT add_job('custom_func', NULL);