From 63ccf6aaa0ce1e7dc81957287da315af31ed8100 Mon Sep 17 00:00:00 2001 From: Mats Kindahl Date: Fri, 14 Feb 2025 10:21:33 +0100 Subject: [PATCH] Load hypercore handler eagerly If a crossmodule wrapper function is called as part of a query on a fresh backend, it will generate a license error even if the license is set correctly. This happens because the module load happens very late (in `post_parse_analyze_hook`) but the function call is quite early. This is fixed by eagerly loading the TSL module in the hypercore handler function and hypercore index proxy handler function. --- .unreleased/pr_7711 | 2 ++ src/cross_module_fn.c | 29 ++++++++++++++-- src/init.c | 9 +++++ tsl/test/expected/hypercore.out | 60 +++++++++++++++++++++++++++++++++ tsl/test/sql/CMakeLists.txt | 1 + tsl/test/sql/hypercore.sql | 41 ++++++++++++++++++++++ 6 files changed, 139 insertions(+), 3 deletions(-) create mode 100644 .unreleased/pr_7711 diff --git a/.unreleased/pr_7711 b/.unreleased/pr_7711 new file mode 100644 index 00000000000..7ac0412a0f0 --- /dev/null +++ b/.unreleased/pr_7711 @@ -0,0 +1,2 @@ +Fixes: #7711 License error when using hypercore handler +Thanks: @jflambert for reporting an bug on license error show in autovacuum diff --git a/src/cross_module_fn.c b/src/cross_module_fn.c index 881a929d33b..c1fba55dac3 100644 --- a/src/cross_module_fn.c +++ b/src/cross_module_fn.c @@ -135,8 +135,13 @@ error_hypercore_proxy_index_options(Datum reloptions, bool validate) * parsing index options instead. */ static Datum -error_pg_community_hypercore_proxy_handler(PG_FUNCTION_ARGS) +process_hypercore_proxy_handler(PG_FUNCTION_ARGS) { + ts_license_enable_module_loading(); + + if (ts_cm_functions->hypercore_proxy_handler != process_hypercore_proxy_handler) + return ts_cm_functions->hypercore_proxy_handler(fcinfo); + IndexAmRoutine *amroutine = makeNode(IndexAmRoutine); amroutine->amstrategies = 0; @@ -248,6 +253,24 @@ process_cagg_try_repair(PG_FUNCTION_ARGS) pg_unreachable(); } +/* + * Ensure that the TSL library is loaded before trying to use the handler. + * + * As for the functions above, the TSL library might not be loaded when this + * function is called, so we try to load this function, but fall back on the + * Apache error message if not possible. + */ +static Datum +process_hypercore_handler(PG_FUNCTION_ARGS) +{ + ts_license_enable_module_loading(); + if (ts_cm_functions->hypercore_handler != process_hypercore_handler) + return ts_cm_functions->hypercore_handler(fcinfo); + + error_no_default_fn_pg_community(fcinfo); + pg_unreachable(); +} + static DDLResult process_cagg_viewstmt_default(Node *stmt, const char *query_string, void *pstmt, WithClauseResult *with_clause_options) @@ -396,8 +419,8 @@ TSDLLEXPORT CrossModuleFunctions ts_cm_functions_default = { .dictionary_compressor_finish = error_no_default_fn_pg_community, .array_compressor_append = error_no_default_fn_pg_community, .array_compressor_finish = error_no_default_fn_pg_community, - .hypercore_handler = error_no_default_fn_pg_community, - .hypercore_proxy_handler = error_pg_community_hypercore_proxy_handler, + .hypercore_handler = process_hypercore_handler, + .hypercore_proxy_handler = process_hypercore_proxy_handler, .is_compressed_tid = error_no_default_fn_pg_community, .show_chunk = error_no_default_fn_pg_community, diff --git a/src/init.c b/src/init.c index 878832f3ad8..8fff4ad9b6f 100644 --- a/src/init.c +++ b/src/init.c @@ -91,6 +91,8 @@ cleanup_on_pg_proc_exit(int code, Datum arg) void _PG_init(void) { + static bool init_done = false; + /* * Check extension_is loaded to catch certain errors such as calls to * functions defined on the wrong extension version @@ -99,6 +101,12 @@ _PG_init(void) ts_extension_check_server_version(); ts_bgw_check_loader_api_version(); + /* We can call _PG_init() several times if we do an eager load, so abort + * init if we do. */ + + if (init_done) + return; + _cache_init(); _hypertable_cache_init(); _cache_invalidate_init(); @@ -118,6 +126,7 @@ _PG_init(void) /* Register a cleanup function to be called when the backend exits */ on_proc_exit(cleanup_on_pg_proc_exit, 0); + init_done = true; } TSDLLEXPORT Datum diff --git a/tsl/test/expected/hypercore.out b/tsl/test/expected/hypercore.out index ebc43a78180..328141793fc 100644 --- a/tsl/test/expected/hypercore.out +++ b/tsl/test/expected/hypercore.out @@ -784,3 +784,63 @@ select test_hypercore(:'rescan_chunk'); (1 row) +-- Simple test that loading the hypercore handler works on a fresh +-- backend. +-- +-- Since TSL library is loaded late (after a query has been executed), +-- this can generate a license error if an attempt is made to use a +-- crossmodule function inside a query, and the hypertable handler is +-- used through GetTableAmRoutine(). +-- +-- To test this, we create a hypercore table, start a fresh +-- connection, and run a query on it. +\c :TEST_DBNAME :ROLE_SUPERUSER +create table crossmodule_test( + time timestamptz unique, + location int +); +select hypertable_id + from create_hypertable('crossmodule_test', by_range('time', '1d'::interval)) \gset +NOTICE: adding not-null constraint to column "time" +select setseed(1); + setseed +--------- + +(1 row) + +insert into crossmodule_test select t, ceil(random()*10) +from generate_series('2022-06-01'::timestamptz, '2022-06-10'::timestamptz, '1h') t; +alter table crossmodule_test + set access method hypercore, + set (timescaledb.compress_orderby = 'time'); +WARNING: there was some uncertainty picking the default segment by for the hypertable: You do not have any indexes on columns that can be used for segment_by and thus we are not using segment_by for compression. Please make sure you are not missing any indexes +NOTICE: default segment by for hypertable "crossmodule_test" is set to "" +WARNING: there was some uncertainty picking the default segment by for the hypertable: You do not have any indexes on columns that can be used for segment_by and thus we are not using segment_by for compression. Please make sure you are not missing any indexes +NOTICE: default segment by for hypertable "crossmodule_test" is set to "" +\c :TEST_DBNAME :ROLE_SUPERUSER +select count(*) from crossmodule_test; + count +------- + 217 +(1 row) + +-- Verify that vacuum with fail without the correct license. +alter system set timescaledb.license to 'apache'; +select pg_reload_conf(); + pg_reload_conf +---------------- + t +(1 row) + +\c :TEST_DBNAME :ROLE_SUPERUSER +\set ON_ERROR_STOP 0 +vacuum crossmodule_test; +ERROR: function "ts_hypercore_handler" is not supported under the current "apache" license +\set ON_ERROR_STOP 1 +alter system reset timescaledb.license; +select pg_reload_conf(); + pg_reload_conf +---------------- + t +(1 row) + diff --git a/tsl/test/sql/CMakeLists.txt b/tsl/test/sql/CMakeLists.txt index 89954d78b00..35484c5a921 100644 --- a/tsl/test/sql/CMakeLists.txt +++ b/tsl/test/sql/CMakeLists.txt @@ -207,6 +207,7 @@ set(SOLO_TESTS cagg_ddl-${PG_VERSION_MAJOR} cagg_dump cagg_invalidation + hypercore move reorder telemetry_stats) diff --git a/tsl/test/sql/hypercore.sql b/tsl/test/sql/hypercore.sql index 32679b5eeed..f33fa5f305e 100644 --- a/tsl/test/sql/hypercore.sql +++ b/tsl/test/sql/hypercore.sql @@ -427,3 +427,44 @@ returns void as :TSL_MODULE_PATHNAME, 'ts_test_hypercore' language c; set role :ROLE_DEFAULT_PERM_USER; select test_hypercore(:'rescan_chunk'); + +-- Simple test that loading the hypercore handler works on a fresh +-- backend. +-- +-- Since TSL library is loaded late (after a query has been executed), +-- this can generate a license error if an attempt is made to use a +-- crossmodule function inside a query, and the hypertable handler is +-- used through GetTableAmRoutine(). +-- +-- To test this, we create a hypercore table, start a fresh +-- connection, and run a query on it. +\c :TEST_DBNAME :ROLE_SUPERUSER +create table crossmodule_test( + time timestamptz unique, + location int +); + +select hypertable_id + from create_hypertable('crossmodule_test', by_range('time', '1d'::interval)) \gset + +select setseed(1); + +insert into crossmodule_test select t, ceil(random()*10) +from generate_series('2022-06-01'::timestamptz, '2022-06-10'::timestamptz, '1h') t; + +alter table crossmodule_test + set access method hypercore, + set (timescaledb.compress_orderby = 'time'); + +\c :TEST_DBNAME :ROLE_SUPERUSER +select count(*) from crossmodule_test; + +-- Verify that vacuum with fail without the correct license. +alter system set timescaledb.license to 'apache'; +select pg_reload_conf(); +\c :TEST_DBNAME :ROLE_SUPERUSER +\set ON_ERROR_STOP 0 +vacuum crossmodule_test; +\set ON_ERROR_STOP 1 +alter system reset timescaledb.license; +select pg_reload_conf();