From 9245abd9791a17bb9436ff83340a9cc2375c6440 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov <36882414+akuzm@users.noreply.github.com> Date: Tue, 5 Nov 2024 16:04:24 +0100 Subject: [PATCH 1/5] Replace ERRCODE_INTERNAL_ERROR with appropriate code Anything that can be triggered by user action or environment conditions is not an internal program error. --- src/nodes/chunk_dispatch/chunk_dispatch.c | 2 +- src/telemetry/telemetry.c | 2 +- src/ts_catalog/tablespace.c | 5 +++-- src/with_clause_parser.c | 2 +- test/src/telemetry/test_telemetry.c | 6 +++--- tsl/src/bgw_policy/compression_api.c | 2 +- tsl/src/nodes/decompress_chunk/exec.c | 2 +- tsl/src/nodes/decompress_chunk/planner.c | 2 +- tsl/src/nodes/frozen_chunk_dml/frozen_chunk_dml.c | 4 ++-- 9 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/nodes/chunk_dispatch/chunk_dispatch.c b/src/nodes/chunk_dispatch/chunk_dispatch.c index 4808877a7d1..cad64293de2 100644 --- a/src/nodes/chunk_dispatch/chunk_dispatch.c +++ b/src/nodes/chunk_dispatch/chunk_dispatch.c @@ -103,7 +103,7 @@ ts_chunk_dispatch_get_chunk_insert_state(ChunkDispatch *dispatch, Point *point, * Frozen chunks require at least PG14. */ if (chunk && ts_chunk_is_frozen(chunk)) - elog(ERROR, "cannot INSERT into frozen chunk \"%s\"", get_rel_name(chunk->table_id)); + ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot INSERT into frozen chunk \"%s\"", get_rel_name(chunk->table_id)))); if (chunk && IS_OSM_CHUNK(chunk)) { const Dimension *time_dim = diff --git a/src/telemetry/telemetry.c b/src/telemetry/telemetry.c index ea5196fe4c5..12a20ac96ac 100644 --- a/src/telemetry/telemetry.c +++ b/src/telemetry/telemetry.c @@ -1185,7 +1185,7 @@ ts_telemetry_main(const char *host, const char *path, const char *service) * throw an error, so we capture the error here and print debugging * information. */ ereport(NOTICE, - (errmsg("malformed telemetry response body"), + (errcode(ERRCODE_DATA_EXCEPTION), errmsg("malformed telemetry response body"), errdetail("host=%s, service=%s, path=%s: %s", host, service, diff --git a/src/ts_catalog/tablespace.c b/src/ts_catalog/tablespace.c index 2952e5abe93..ee98400a82d 100644 --- a/src/ts_catalog/tablespace.c +++ b/src/ts_catalog/tablespace.c @@ -522,10 +522,11 @@ ts_tablespace_attach_internal(Name tspcname, Oid hypertable_oid, bool if_not_att CatalogSecurityContext sec_ctx; if (NULL == tspcname) - elog(ERROR, "invalid tablespace name"); + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid tablespace name"))); if (!OidIsValid(hypertable_oid)) - elog(ERROR, "invalid hypertable"); + ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid hypertable"))); tspc_oid = get_tablespace_oid(NameStr(*tspcname), true); diff --git a/src/with_clause_parser.c b/src/with_clause_parser.c index 7a1a47899bd..50f45b309c6 100644 --- a/src/with_clause_parser.c +++ b/src/with_clause_parser.c @@ -137,7 +137,7 @@ parse_arg(WithClauseDefinition arg, DefElem *def) Oid typIOParam; if (!OidIsValid(arg.type_id)) - elog(ERROR, "argument \"%s.%s\" not implemented", def->defnamespace, def->defname); + ereport(ERROR, (errcode(ERRCODE_UNDEFINED_PARAMETER), errmsg("argument \"%s.%s\" not implemented", def->defnamespace, def->defname))); if (def->arg != NULL) value = defGetString(def); diff --git a/test/src/telemetry/test_telemetry.c b/test/src/telemetry/test_telemetry.c index 738a1776d5c..2ffd542144c 100644 --- a/test/src/telemetry/test_telemetry.c +++ b/test/src/telemetry/test_telemetry.c @@ -112,9 +112,9 @@ test_factory(ConnectionType type, int status, char *host, int port) ts_connection_destroy(conn); if (!ts_http_response_state_valid_status(rsp)) - elog(ERROR, + ereport(ERROR, (errcode(ERRCODE_IO_ERROR), errmsg( "endpoint sent back unexpected HTTP status: %d", - ts_http_response_state_status_code(rsp)); + ts_http_response_state_status_code(rsp)))); json = DirectFunctionCall1(jsonb_in, CStringGetDatum(ts_http_response_state_body_start(rsp))); @@ -135,7 +135,7 @@ ts_test_status_ssl(PG_FUNCTION_ARGS) char buf[128] = { '\0' }; if (status / 100 != 2) - elog(ERROR, "endpoint sent back unexpected HTTP status: %d", status); + ereport(ERROR, (errcode(ERRCODE_IO_ERROR), errmsg("endpoint sent back unexpected HTTP status: %d", status))); snprintf(buf, sizeof(buf) - 1, "{\"status\":%d}", status); diff --git a/tsl/src/bgw_policy/compression_api.c b/tsl/src/bgw_policy/compression_api.c index b359108d036..5d408b29926 100644 --- a/tsl/src/bgw_policy/compression_api.c +++ b/tsl/src/bgw_policy/compression_api.c @@ -142,7 +142,7 @@ policy_compression_check(PG_FUNCTION_ARGS) if (PG_ARGISNULL(0)) { - ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR), errmsg("config must not be NULL"))); + ereport(ERROR, (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), errmsg("config must not be NULL"))); } policy_compression_read_and_validate_config(PG_GETARG_JSONB_P(0), &policy_data); diff --git a/tsl/src/nodes/decompress_chunk/exec.c b/tsl/src/nodes/decompress_chunk/exec.c index c59bcfba072..2c47822d001 100644 --- a/tsl/src/nodes/decompress_chunk/exec.c +++ b/tsl/src/nodes/decompress_chunk/exec.c @@ -124,7 +124,7 @@ constify_tableoid_walker(Node *node, ConstifyTableOidContext *ctx) * segfault if any system columns get through */ if (var->varattno < SelfItemPointerAttributeNumber) - elog(ERROR, "transparent decompression only supports tableoid system column"); + ereport(ERROR, (errcode(ERRCODE_INVALID_COLUMN_REFERENCE), errmsg("transparent decompression only supports tableoid system column"))); return node; } diff --git a/tsl/src/nodes/decompress_chunk/planner.c b/tsl/src/nodes/decompress_chunk/planner.c index c1f7648c9e3..cf715efc642 100644 --- a/tsl/src/nodes/decompress_chunk/planner.c +++ b/tsl/src/nodes/decompress_chunk/planner.c @@ -61,7 +61,7 @@ check_for_system_columns(Bitmapset *attrs_used) bit = bms_next_member(attrs_used, bit); if (bit > 0 && bit + FirstLowInvalidHeapAttributeNumber < 0) - elog(ERROR, "transparent decompression only supports tableoid system column"); + ereport(ERROR, (errcode(ERRCODE_INVALID_COLUMN_REFERENCE), errmsg("transparent decompression only supports tableoid system column"))); } } diff --git a/tsl/src/nodes/frozen_chunk_dml/frozen_chunk_dml.c b/tsl/src/nodes/frozen_chunk_dml/frozen_chunk_dml.c index 85b128ffc62..9bd3f5fc665 100644 --- a/tsl/src/nodes/frozen_chunk_dml/frozen_chunk_dml.c +++ b/tsl/src/nodes/frozen_chunk_dml/frozen_chunk_dml.c @@ -73,9 +73,9 @@ frozen_chunk_dml_exec(CustomScanState *node) { FrozenChunkDmlState *state = (FrozenChunkDmlState *) node; Oid chunk_relid = state->chunk_relid; - elog(ERROR, + ereport(ERROR, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg( "cannot update/delete rows from chunk \"%s\" as it is frozen", - get_rel_name(chunk_relid)); + get_rel_name(chunk_relid)))); return NULL; } From 7c8107f30bc093dc83bd2c5e81f9e2900164b552 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov <36882414+akuzm@users.noreply.github.com> Date: Tue, 5 Nov 2024 16:12:12 +0100 Subject: [PATCH 2/5] more --- src/chunk_index.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/chunk_index.c b/src/chunk_index.c index 6c68538b252..35b9d16d63e 100644 --- a/src/chunk_index.c +++ b/src/chunk_index.c @@ -1185,6 +1185,9 @@ Datum ts_chunk_index_clone(PG_FUNCTION_ARGS) { Oid chunk_index_oid = PG_GETARG_OID(0); + if (!OidIsValid(chunk_index_oid)) + ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid chunk index"))); + Relation chunk_index_rel; Relation hypertable_rel; Relation chunk_rel; @@ -1228,7 +1231,13 @@ Datum ts_chunk_index_replace(PG_FUNCTION_ARGS) { Oid chunk_index_oid_old = PG_GETARG_OID(0); + if (!OidIsValid(chunk_index_oid_old)) + ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid chunk index"))); + Oid chunk_index_oid_new = PG_GETARG_OID(1); + if (!OidIsValid(chunk_index_oid_new)) + ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid chunk index"))); + Relation index_rel; Chunk *chunk; ChunkIndexMapping cim; From c5df44b3945c0729db842a63e7d320d08b39e6a5 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov <36882414+akuzm@users.noreply.github.com> Date: Tue, 5 Nov 2024 16:14:11 +0100 Subject: [PATCH 3/5] format --- src/nodes/chunk_dispatch/chunk_dispatch.c | 5 ++++- src/telemetry/telemetry.c | 3 ++- src/with_clause_parser.c | 4 +++- test/src/telemetry/test_telemetry.c | 11 +++++++---- tsl/src/bgw_policy/compression_api.c | 3 ++- tsl/src/nodes/decompress_chunk/exec.c | 4 +++- tsl/src/nodes/decompress_chunk/planner.c | 4 +++- tsl/src/nodes/frozen_chunk_dml/frozen_chunk_dml.c | 7 ++++--- 8 files changed, 28 insertions(+), 13 deletions(-) diff --git a/src/nodes/chunk_dispatch/chunk_dispatch.c b/src/nodes/chunk_dispatch/chunk_dispatch.c index cad64293de2..407c66fee91 100644 --- a/src/nodes/chunk_dispatch/chunk_dispatch.c +++ b/src/nodes/chunk_dispatch/chunk_dispatch.c @@ -103,7 +103,10 @@ ts_chunk_dispatch_get_chunk_insert_state(ChunkDispatch *dispatch, Point *point, * Frozen chunks require at least PG14. */ if (chunk && ts_chunk_is_frozen(chunk)) - ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot INSERT into frozen chunk \"%s\"", get_rel_name(chunk->table_id)))); + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot INSERT into frozen chunk \"%s\"", + get_rel_name(chunk->table_id)))); if (chunk && IS_OSM_CHUNK(chunk)) { const Dimension *time_dim = diff --git a/src/telemetry/telemetry.c b/src/telemetry/telemetry.c index 12a20ac96ac..a8eef3d8d15 100644 --- a/src/telemetry/telemetry.c +++ b/src/telemetry/telemetry.c @@ -1185,7 +1185,8 @@ ts_telemetry_main(const char *host, const char *path, const char *service) * throw an error, so we capture the error here and print debugging * information. */ ereport(NOTICE, - (errcode(ERRCODE_DATA_EXCEPTION), errmsg("malformed telemetry response body"), + (errcode(ERRCODE_DATA_EXCEPTION), + errmsg("malformed telemetry response body"), errdetail("host=%s, service=%s, path=%s: %s", host, service, diff --git a/src/with_clause_parser.c b/src/with_clause_parser.c index 50f45b309c6..715c65a386c 100644 --- a/src/with_clause_parser.c +++ b/src/with_clause_parser.c @@ -137,7 +137,9 @@ parse_arg(WithClauseDefinition arg, DefElem *def) Oid typIOParam; if (!OidIsValid(arg.type_id)) - ereport(ERROR, (errcode(ERRCODE_UNDEFINED_PARAMETER), errmsg("argument \"%s.%s\" not implemented", def->defnamespace, def->defname))); + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_PARAMETER), + errmsg("argument \"%s.%s\" not implemented", def->defnamespace, def->defname))); if (def->arg != NULL) value = defGetString(def); diff --git a/test/src/telemetry/test_telemetry.c b/test/src/telemetry/test_telemetry.c index 2ffd542144c..904f3fee120 100644 --- a/test/src/telemetry/test_telemetry.c +++ b/test/src/telemetry/test_telemetry.c @@ -112,9 +112,10 @@ test_factory(ConnectionType type, int status, char *host, int port) ts_connection_destroy(conn); if (!ts_http_response_state_valid_status(rsp)) - ereport(ERROR, (errcode(ERRCODE_IO_ERROR), errmsg( - "endpoint sent back unexpected HTTP status: %d", - ts_http_response_state_status_code(rsp)))); + ereport(ERROR, + (errcode(ERRCODE_IO_ERROR), + errmsg("endpoint sent back unexpected HTTP status: %d", + ts_http_response_state_status_code(rsp)))); json = DirectFunctionCall1(jsonb_in, CStringGetDatum(ts_http_response_state_body_start(rsp))); @@ -135,7 +136,9 @@ ts_test_status_ssl(PG_FUNCTION_ARGS) char buf[128] = { '\0' }; if (status / 100 != 2) - ereport(ERROR, (errcode(ERRCODE_IO_ERROR), errmsg("endpoint sent back unexpected HTTP status: %d", status))); + ereport(ERROR, + (errcode(ERRCODE_IO_ERROR), + errmsg("endpoint sent back unexpected HTTP status: %d", status))); snprintf(buf, sizeof(buf) - 1, "{\"status\":%d}", status); diff --git a/tsl/src/bgw_policy/compression_api.c b/tsl/src/bgw_policy/compression_api.c index 5d408b29926..e0239667861 100644 --- a/tsl/src/bgw_policy/compression_api.c +++ b/tsl/src/bgw_policy/compression_api.c @@ -142,7 +142,8 @@ policy_compression_check(PG_FUNCTION_ARGS) if (PG_ARGISNULL(0)) { - ereport(ERROR, (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), errmsg("config must not be NULL"))); + ereport(ERROR, + (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), errmsg("config must not be NULL"))); } policy_compression_read_and_validate_config(PG_GETARG_JSONB_P(0), &policy_data); diff --git a/tsl/src/nodes/decompress_chunk/exec.c b/tsl/src/nodes/decompress_chunk/exec.c index 2c47822d001..18d1ec1e54b 100644 --- a/tsl/src/nodes/decompress_chunk/exec.c +++ b/tsl/src/nodes/decompress_chunk/exec.c @@ -124,7 +124,9 @@ constify_tableoid_walker(Node *node, ConstifyTableOidContext *ctx) * segfault if any system columns get through */ if (var->varattno < SelfItemPointerAttributeNumber) - ereport(ERROR, (errcode(ERRCODE_INVALID_COLUMN_REFERENCE), errmsg("transparent decompression only supports tableoid system column"))); + ereport(ERROR, + (errcode(ERRCODE_INVALID_COLUMN_REFERENCE), + errmsg("transparent decompression only supports tableoid system column"))); return node; } diff --git a/tsl/src/nodes/decompress_chunk/planner.c b/tsl/src/nodes/decompress_chunk/planner.c index cf715efc642..1075547b0a4 100644 --- a/tsl/src/nodes/decompress_chunk/planner.c +++ b/tsl/src/nodes/decompress_chunk/planner.c @@ -61,7 +61,9 @@ check_for_system_columns(Bitmapset *attrs_used) bit = bms_next_member(attrs_used, bit); if (bit > 0 && bit + FirstLowInvalidHeapAttributeNumber < 0) - ereport(ERROR, (errcode(ERRCODE_INVALID_COLUMN_REFERENCE), errmsg("transparent decompression only supports tableoid system column"))); + ereport(ERROR, + (errcode(ERRCODE_INVALID_COLUMN_REFERENCE), + errmsg("transparent decompression only supports tableoid system column"))); } } diff --git a/tsl/src/nodes/frozen_chunk_dml/frozen_chunk_dml.c b/tsl/src/nodes/frozen_chunk_dml/frozen_chunk_dml.c index 9bd3f5fc665..570d49643eb 100644 --- a/tsl/src/nodes/frozen_chunk_dml/frozen_chunk_dml.c +++ b/tsl/src/nodes/frozen_chunk_dml/frozen_chunk_dml.c @@ -73,9 +73,10 @@ frozen_chunk_dml_exec(CustomScanState *node) { FrozenChunkDmlState *state = (FrozenChunkDmlState *) node; Oid chunk_relid = state->chunk_relid; - ereport(ERROR, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg( - "cannot update/delete rows from chunk \"%s\" as it is frozen", - get_rel_name(chunk_relid)))); + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot update/delete rows from chunk \"%s\" as it is frozen", + get_rel_name(chunk_relid)))); return NULL; } From 39ac4c59549aadb3993f9873c4ec5fdef6779eac Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov <36882414+akuzm@users.noreply.github.com> Date: Tue, 5 Nov 2024 22:15:42 +0100 Subject: [PATCH 4/5] ref --- tsl/test/shared/expected/compat.out | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tsl/test/shared/expected/compat.out b/tsl/test/shared/expected/compat.out index a843c0e975e..270ee7b3b50 100644 --- a/tsl/test/shared/expected/compat.out +++ b/tsl/test/shared/expected/compat.out @@ -38,10 +38,10 @@ WARNING: function _timescaledb_internal.chunk_id_from_relid(oid) is deprecated SELECT _timescaledb_internal.chunk_index_clone(0); WARNING: function _timescaledb_internal.chunk_index_clone(oid) is deprecated and has been moved to _timescaledb_functions schema. this compatibility function will be removed in a future version. -ERROR: could not open relation with OID 0 +ERROR: invalid chunk index SELECT _timescaledb_internal.chunk_index_replace(0,0); WARNING: function _timescaledb_internal.chunk_index_replace(oid,oid) is deprecated and has been moved to _timescaledb_functions schema. this compatibility function will be removed in a future version. -ERROR: could not open relation with OID 0 +ERROR: invalid chunk index SELECT _timescaledb_internal.chunk_status(0); WARNING: function _timescaledb_internal.chunk_status(regclass) is deprecated and has been moved to _timescaledb_functions schema. this compatibility function will be removed in a future version. ERROR: invalid Oid From e86b9a3b4c4d781ef282a0b343b9c3438fe2116b Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov <36882414+akuzm@users.noreply.github.com> Date: Thu, 7 Nov 2024 16:31:22 +0100 Subject: [PATCH 5/5] fix --- scripts/upload_ci_stats.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/upload_ci_stats.sh b/scripts/upload_ci_stats.sh index 086f7648cf3..c3b938ee392 100755 --- a/scripts/upload_ci_stats.sh +++ b/scripts/upload_ci_stats.sh @@ -94,8 +94,8 @@ then match($0, /^(test| ) ([^ ]+)[ ]+\.\.\.[ ]+([^ ]+) (|\(.*\))[ ]+([0-9]+) ms$/, a) { print ENVIRON["JOB_DATE"], a[2], tolower(a[3] (a[4] ? (" " a[4]) : "")), a[5]; } - match($0, /^([^0-9]+) [0-9]+ +- ([^ ]+) +([0-9]+) ms/, a) { - print ENVIRON["JOB_DATE"], a[2], a[1], a[3]; + match($0, /^([^0-9]+) [0-9]+ +[-+] ([^ ]+) +([0-9]+) ms/, a) { + print ENVIRON["JOB_DATE"], a[2], a[1], a[3]; } ' installcheck.log > tests.tsv