From 59bafb5adb16e25331af53cf1b70c81140d05e3d Mon Sep 17 00:00:00 2001 From: NJrslv Date: Wed, 21 Jan 2026 12:53:47 +0000 Subject: [PATCH] [yagp_hooks_collector] Make gen of norm plan/query noexcept The extension generates normalized query text and plan using jumbling functions. Those functions may fail when translating to wide character if the current locale cannot handle the character set. Fix changed functions that generate normalized query text/plan to noexcept versions so we can check if error occured and continute execution. The test checks that even when those functions fail, the plan is still executed. This test is partially taken from src/test/regress/gp_locale.sql. --- gpcontrib/yagp_hooks_collector/Makefile | 2 +- .../expected/yagp_locale.out | 23 +++++++++++++++ .../yagp_hooks_collector/sql/yagp_locale.sql | 29 +++++++++++++++++++ .../yagp_hooks_collector/src/ProtoUtils.cpp | 19 +++++++----- .../src/memory/gpdbwrappers.cpp | 11 +++---- .../src/memory/gpdbwrappers.h | 4 +-- 6 files changed, 71 insertions(+), 17 deletions(-) create mode 100644 gpcontrib/yagp_hooks_collector/expected/yagp_locale.out create mode 100644 gpcontrib/yagp_hooks_collector/sql/yagp_locale.sql diff --git a/gpcontrib/yagp_hooks_collector/Makefile b/gpcontrib/yagp_hooks_collector/Makefile index eb6541b7687..d145ae46dbe 100644 --- a/gpcontrib/yagp_hooks_collector/Makefile +++ b/gpcontrib/yagp_hooks_collector/Makefile @@ -1,7 +1,7 @@ MODULE_big = yagp_hooks_collector EXTENSION = yagp_hooks_collector DATA = $(wildcard *--*.sql) -REGRESS = yagp_cursors yagp_dist yagp_select yagp_utf8_trim yagp_utility yagp_guc_cache yagp_uds +REGRESS = yagp_cursors yagp_dist yagp_select yagp_utf8_trim yagp_utility yagp_guc_cache yagp_uds yagp_locale PROTO_BASES = yagpcc_plan yagpcc_metrics yagpcc_set_service PROTO_OBJS = $(patsubst %,src/protos/%.pb.o,$(PROTO_BASES)) diff --git a/gpcontrib/yagp_hooks_collector/expected/yagp_locale.out b/gpcontrib/yagp_hooks_collector/expected/yagp_locale.out new file mode 100644 index 00000000000..6689b6a4ed3 --- /dev/null +++ b/gpcontrib/yagp_hooks_collector/expected/yagp_locale.out @@ -0,0 +1,23 @@ +-- The extension generates normalized query text and plan using jumbling functions. +-- Those functions may fail when translating to wide character if the current locale +-- cannot handle the character set. This test checks that even when those functions +-- fail, the plan is still generated and executed. This test is partially taken from +-- gp_locale. +-- start_ignore +DROP DATABASE IF EXISTS yagp_test_locale; +-- end_ignore +CREATE DATABASE yagp_test_locale WITH LC_COLLATE='C' LC_CTYPE='C' TEMPLATE=template0; +\c yagp_test_locale +CREATE EXTENSION yagp_hooks_collector; +SET yagpcc.ignored_users_list TO ''; +SET yagpcc.enable_utility TO TRUE; +SET yagpcc.enable TO TRUE; +CREATE TABLE yagp_hi_안녕세계 (a int, 안녕세계1 text, 안녕세계2 text, 안녕세계3 text) DISTRIBUTED BY (a); +INSERT INTO yagp_hi_안녕세계 VALUES(1, '안녕세계1 first', '안녕세2 first', '안녕세계3 first'); +-- Should not see error here +UPDATE yagp_hi_안녕세계 SET 안녕세계1='안녕세계1 first UPDATE' WHERE 안녕세계1='안녕세계1 first'; +RESET yagpcc.enable; +RESET yagpcc.enable_utility; +RESET yagpcc.ignored_users_list; +DROP TABLE yagp_hi_안녕세계; +DROP EXTENSION yagp_hooks_collector; diff --git a/gpcontrib/yagp_hooks_collector/sql/yagp_locale.sql b/gpcontrib/yagp_hooks_collector/sql/yagp_locale.sql new file mode 100644 index 00000000000..65d867d1680 --- /dev/null +++ b/gpcontrib/yagp_hooks_collector/sql/yagp_locale.sql @@ -0,0 +1,29 @@ +-- The extension generates normalized query text and plan using jumbling functions. +-- Those functions may fail when translating to wide character if the current locale +-- cannot handle the character set. This test checks that even when those functions +-- fail, the plan is still generated and executed. This test is partially taken from +-- gp_locale. + +-- start_ignore +DROP DATABASE IF EXISTS yagp_test_locale; +-- end_ignore + +CREATE DATABASE yagp_test_locale WITH LC_COLLATE='C' LC_CTYPE='C' TEMPLATE=template0; +\c yagp_test_locale + +CREATE EXTENSION yagp_hooks_collector; + +SET yagpcc.ignored_users_list TO ''; +SET yagpcc.enable_utility TO TRUE; +SET yagpcc.enable TO TRUE; + +CREATE TABLE yagp_hi_안녕세계 (a int, 안녕세계1 text, 안녕세계2 text, 안녕세계3 text) DISTRIBUTED BY (a); +INSERT INTO yagp_hi_안녕세계 VALUES(1, '안녕세계1 first', '안녕세2 first', '안녕세계3 first'); +-- Should not see error here +UPDATE yagp_hi_안녕세계 SET 안녕세계1='안녕세계1 first UPDATE' WHERE 안녕세계1='안녕세계1 first'; + +RESET yagpcc.enable; +RESET yagpcc.enable_utility; +RESET yagpcc.ignored_users_list; +DROP TABLE yagp_hi_안녕세계; +DROP EXTENSION yagp_hooks_collector; diff --git a/gpcontrib/yagp_hooks_collector/src/ProtoUtils.cpp b/gpcontrib/yagp_hooks_collector/src/ProtoUtils.cpp index 8ebbe19e289..f9119ca4b14 100644 --- a/gpcontrib/yagp_hooks_collector/src/ProtoUtils.cpp +++ b/gpcontrib/yagp_hooks_collector/src/ProtoUtils.cpp @@ -96,13 +96,15 @@ void set_query_plan(yagpcc::SetQueryReq *req, QueryDesc *query_desc, *qi->mutable_plan_text() = trim_str_shrink_utf8(es.str->data, es.str->len, config.max_plan_size()); StringInfo norm_plan = ya_gpdb::gen_normplan(es.str->data); - *qi->mutable_template_plan_text() = trim_str_shrink_utf8( - norm_plan->data, norm_plan->len, config.max_plan_size()); - qi->set_plan_id( - hash_any((unsigned char *)norm_plan->data, norm_plan->len)); + if (norm_plan) { + *qi->mutable_template_plan_text() = trim_str_shrink_utf8( + norm_plan->data, norm_plan->len, config.max_plan_size()); + qi->set_plan_id( + hash_any((unsigned char *)norm_plan->data, norm_plan->len)); + ya_gpdb::pfree(norm_plan->data); + } qi->set_query_id(query_desc->plannedstmt->queryId); ya_gpdb::pfree(es.str->data); - ya_gpdb::pfree(norm_plan->data); } ya_gpdb::mem_ctx_switch_to(oldcxt); } @@ -116,8 +118,11 @@ void set_query_text(yagpcc::SetQueryReq *req, QueryDesc *query_desc, query_desc->sourceText, strlen(query_desc->sourceText), config.max_text_size()); char *norm_query = ya_gpdb::gen_normquery(query_desc->sourceText); - *qi->mutable_template_query_text() = trim_str_shrink_utf8( - norm_query, strlen(norm_query), config.max_text_size()); + if (norm_query) { + *qi->mutable_template_query_text() = trim_str_shrink_utf8( + norm_query, strlen(norm_query), config.max_text_size()); + ya_gpdb::pfree(norm_query); + } } } diff --git a/gpcontrib/yagp_hooks_collector/src/memory/gpdbwrappers.cpp b/gpcontrib/yagp_hooks_collector/src/memory/gpdbwrappers.cpp index 763e32e539c..8cc483a39de 100644 --- a/gpcontrib/yagp_hooks_collector/src/memory/gpdbwrappers.cpp +++ b/gpcontrib/yagp_hooks_collector/src/memory/gpdbwrappers.cpp @@ -204,15 +204,12 @@ void ya_gpdb::instr_end_loop(Instrumentation *instr) { wrap_throw(::InstrEndLoop, instr); } -char *ya_gpdb::gen_normquery(const char *query) { - return wrap_throw(::gen_normquery, query); +char *ya_gpdb::gen_normquery(const char *query) noexcept { + return wrap_noexcept(::gen_normquery, query); } -StringInfo ya_gpdb::gen_normplan(const char *exec_plan) { - if (!exec_plan) - throw std::runtime_error("Invalid execution plan string"); - - return wrap_throw(::gen_normplan, exec_plan); +StringInfo ya_gpdb::gen_normplan(const char *exec_plan) noexcept { + return wrap_noexcept(::gen_normplan, exec_plan); } char *ya_gpdb::get_rg_name_for_id(Oid group_id) { diff --git a/gpcontrib/yagp_hooks_collector/src/memory/gpdbwrappers.h b/gpcontrib/yagp_hooks_collector/src/memory/gpdbwrappers.h index 920fc1ae6e7..e080ef5cdd4 100644 --- a/gpcontrib/yagp_hooks_collector/src/memory/gpdbwrappers.h +++ b/gpcontrib/yagp_hooks_collector/src/memory/gpdbwrappers.h @@ -38,8 +38,8 @@ HeapTuple heap_form_tuple(TupleDesc tupleDescriptor, Datum *values, CdbExplain_ShowStatCtx *cdbexplain_showExecStatsBegin(QueryDesc *query_desc, instr_time starttime); void instr_end_loop(Instrumentation *instr); -char *gen_normquery(const char *query); -StringInfo gen_normplan(const char *executionPlan); +char *gen_normquery(const char *query) noexcept; +StringInfo gen_normplan(const char *executionPlan) noexcept; char *get_rg_name_for_id(Oid group_id); void insert_log(const yagpcc::SetQueryReq &req, bool utility);