From af3d2789414440a14d1c02d3712ec43840bd6844 Mon Sep 17 00:00:00 2001 From: "Y." Date: Mon, 2 Dec 2024 10:19:57 -0500 Subject: [PATCH] Move CPP functions wrapper to dedicated file (#452) Move CPP function wrapper to dedicated file. Next, will do the same for the PG wrapper. --- include/pgduckdb/logger.hpp | 2 + include/pgduckdb/pg/error_data.hpp | 9 ++++ include/pgduckdb/pgduckdb_types.hpp | 2 + include/pgduckdb/pgduckdb_utils.hpp | 56 +++++++++--------------- include/pgduckdb/utility/cpp_wrapper.hpp | 43 ++++++++++++++++++ src/pg/error_data.cpp | 11 +++++ src/pgduckdb_background_worker.cpp | 3 +- src/pgduckdb_ddl.cpp | 3 +- src/pgduckdb_detoast.cpp | 5 ++- src/pgduckdb_duckdb.cpp | 2 +- src/pgduckdb_filter.cpp | 2 +- src/pgduckdb_hooks.cpp | 3 +- src/pgduckdb_node.cpp | 4 +- src/pgduckdb_options.cpp | 5 ++- src/pgduckdb_planner.cpp | 4 +- src/pgduckdb_types.cpp | 3 +- src/pgduckdb_xact.cpp | 2 + src/scan/heap_reader.cpp | 4 +- src/scan/postgres_scan.cpp | 4 +- 19 files changed, 114 insertions(+), 53 deletions(-) create mode 100644 include/pgduckdb/pg/error_data.hpp create mode 100644 include/pgduckdb/utility/cpp_wrapper.hpp create mode 100644 src/pg/error_data.cpp diff --git a/include/pgduckdb/logger.hpp b/include/pgduckdb/logger.hpp index a4e38b79..f08829d8 100644 --- a/include/pgduckdb/logger.hpp +++ b/include/pgduckdb/logger.hpp @@ -2,6 +2,8 @@ #include "pgduckdb/pgduckdb_process_lock.hpp" +#include "pgduckdb/utility/cpp_only_file.hpp" // Must be last include. + extern "C" { bool errstart(int elevel, const char *domain); void errfinish(const char *filename, int lineno, const char *funcname); diff --git a/include/pgduckdb/pg/error_data.hpp b/include/pgduckdb/pg/error_data.hpp new file mode 100644 index 00000000..238a662d --- /dev/null +++ b/include/pgduckdb/pg/error_data.hpp @@ -0,0 +1,9 @@ +#pragma once + +extern "C" { +struct ErrorData; +} + +namespace pgduckdb::pg { +const char* GetErrorDataMessage(ErrorData* error_data); +} diff --git a/include/pgduckdb/pgduckdb_types.hpp b/include/pgduckdb/pgduckdb_types.hpp index ebb54c9d..db02ef36 100644 --- a/include/pgduckdb/pgduckdb_types.hpp +++ b/include/pgduckdb/pgduckdb_types.hpp @@ -2,6 +2,8 @@ #include "pgduckdb/pg/declarations.hpp" +#include "pgduckdb/utility/cpp_only_file.hpp" // Must be last include. + namespace pgduckdb { class PostgresScanGlobalState; diff --git a/include/pgduckdb/pgduckdb_utils.hpp b/include/pgduckdb/pgduckdb_utils.hpp index 345f8352..5405772a 100644 --- a/include/pgduckdb/pgduckdb_utils.hpp +++ b/include/pgduckdb/pgduckdb_utils.hpp @@ -3,9 +3,26 @@ #include "duckdb/common/exception.hpp" #include "duckdb/common/error_data.hpp" #include "pgduckdb/pgduckdb_duckdb.hpp" +#include "pgduckdb/pg/error_data.hpp" +#include "pgduckdb/logger.hpp" + +#include + +#include "pgduckdb/utility/cpp_only_file.hpp" // Must be last include. extern "C" { -#include "postgres.h" +// Note: these forward-declarations could live in a header under the `pg/` folder. +// But since they are (hopefully) only used in this file, we keep them here. +struct ErrorContextCallback; +struct MemoryContextData; + +typedef struct MemoryContextData *MemoryContext; + +extern sigjmp_buf *PG_exception_stack; +extern MemoryContext CurrentMemoryContext; +extern ErrorContextCallback *error_context_stack; +extern ErrorData * CopyErrorData(); +extern void FlushErrorState(); } namespace pgduckdb { @@ -44,52 +61,19 @@ __PostgresFunctionGuard__(const char *func_name, FuncArgs... args) { return func(std::forward(args)...); } else { g.RestoreStacks(); - MemoryContextSwitchTo(ctx); + CurrentMemoryContext = ctx; edata = CopyErrorData(); FlushErrorState(); } } // PG_END_TRY(); - auto message = duckdb::StringUtil::Format("(PGDuckDB/%s) %s", func_name, edata->message); + auto message = duckdb::StringUtil::Format("(PGDuckDB/%s) %s", func_name, pg::GetErrorDataMessage(edata)); throw duckdb::Exception(duckdb::ExceptionType::EXECUTOR, message); } #define PostgresFunctionGuard(FUNC, ...) \ pgduckdb::__PostgresFunctionGuard__(__func__, __VA_ARGS__) -template -typename std::invoke_result::type -__CPPFunctionGuard__(const char *func_name, FuncArgs... args) { - const char *error_message = nullptr; - try { - return func(args...); - } catch (duckdb::Exception &ex) { - duckdb::ErrorData edata(ex.what()); - error_message = pstrdup(edata.Message().c_str()); - } catch (std::exception &ex) { - const auto msg = ex.what(); - if (msg[0] == '{') { - duckdb::ErrorData edata(ex.what()); - error_message = pstrdup(edata.Message().c_str()); - } else { - error_message = pstrdup(ex.what()); - } - } - - elog(ERROR, "(PGDuckDB/%s) %s", func_name, error_message); -} - -#define InvokeCPPFunc(FUNC, ...) pgduckdb::__CPPFunctionGuard__(__FUNCTION__, __VA_ARGS__) - -// Wrappers - -#define DECLARE_PG_FUNCTION(func_name) \ - PG_FUNCTION_INFO_V1(func_name); \ - Datum func_name##_cpp(PG_FUNCTION_ARGS); \ - Datum func_name(PG_FUNCTION_ARGS) { \ - return InvokeCPPFunc(func_name##_cpp, fcinfo); \ - } \ - Datum func_name##_cpp(PG_FUNCTION_ARGS __attribute__((unused))) duckdb::unique_ptr DuckDBQueryOrThrow(duckdb::ClientContext &context, const std::string &query); diff --git a/include/pgduckdb/utility/cpp_wrapper.hpp b/include/pgduckdb/utility/cpp_wrapper.hpp new file mode 100644 index 00000000..40a0b947 --- /dev/null +++ b/include/pgduckdb/utility/cpp_wrapper.hpp @@ -0,0 +1,43 @@ +#pragma once + +extern "C" { +#include "postgres.h" +} + +namespace pgduckdb { + +template +typename std::invoke_result::type +__CPPFunctionGuard__(const char *func_name, FuncArgs... args) { + const char *error_message = nullptr; + try { + return func(args...); + } catch (duckdb::Exception &ex) { + duckdb::ErrorData edata(ex.what()); + error_message = pstrdup(edata.Message().c_str()); + } catch (std::exception &ex) { + const auto msg = ex.what(); + if (msg[0] == '{') { + duckdb::ErrorData edata(ex.what()); + error_message = pstrdup(edata.Message().c_str()); + } else { + error_message = pstrdup(ex.what()); + } + } + + elog(ERROR, "(PGDuckDB/%s) %s", func_name, error_message); +} + +} + +#define InvokeCPPFunc(FUNC, ...) pgduckdb::__CPPFunctionGuard__(__FUNCTION__, __VA_ARGS__) + +// Wrappers + +#define DECLARE_PG_FUNCTION(func_name) \ + PG_FUNCTION_INFO_V1(func_name); \ + Datum func_name##_cpp(PG_FUNCTION_ARGS); \ + Datum func_name(PG_FUNCTION_ARGS) { \ + return InvokeCPPFunc(func_name##_cpp, fcinfo); \ + } \ + Datum func_name##_cpp(PG_FUNCTION_ARGS __attribute__((unused))) diff --git a/src/pg/error_data.cpp b/src/pg/error_data.cpp new file mode 100644 index 00000000..6e5f1bb4 --- /dev/null +++ b/src/pg/error_data.cpp @@ -0,0 +1,11 @@ +#include "pgduckdb/pg/error_data.hpp" + +extern "C" { +#include "postgres.h" +} + +namespace pgduckdb::pg { +const char* GetErrorDataMessage(ErrorData* error_data) { + return error_data->message; +} +} // namespace pgduckdb diff --git a/src/pgduckdb_background_worker.cpp b/src/pgduckdb_background_worker.cpp index 042d84df..2aa026d0 100644 --- a/src/pgduckdb_background_worker.cpp +++ b/src/pgduckdb_background_worker.cpp @@ -13,6 +13,8 @@ #include "duckdb/storage/table_storage_info.hpp" #include "duckdb/main/attached_database.hpp" #include "pgduckdb/pgduckdb_types.hpp" +#include "pgduckdb/pgduckdb_utils.hpp" +#include "pgduckdb/utility/cpp_wrapper.hpp" #include #include @@ -50,7 +52,6 @@ extern "C" { #include "pgduckdb/pgduckdb_duckdb.hpp" #include "pgduckdb/pgduckdb_background_worker.hpp" #include "pgduckdb/pgduckdb_metadata_cache.hpp" -#include "pgduckdb/pgduckdb_utils.hpp" static bool is_background_worker = false; static std::unordered_map last_known_motherduck_catalog_versions; diff --git a/src/pgduckdb_ddl.cpp b/src/pgduckdb_ddl.cpp index 24d8934a..37693cc6 100644 --- a/src/pgduckdb_ddl.cpp +++ b/src/pgduckdb_ddl.cpp @@ -2,6 +2,7 @@ #include #include "pgduckdb/pgduckdb_planner.hpp" +#include "pgduckdb/pgduckdb_utils.hpp" extern "C" { #include "postgres.h" @@ -27,10 +28,10 @@ extern "C" { #include "pgduckdb/pgduckdb_ruleutils.h" } +#include "pgduckdb/utility/cpp_wrapper.hpp" #include "pgduckdb/pgduckdb_duckdb.hpp" #include "pgduckdb/pgduckdb_background_worker.hpp" #include "pgduckdb/pgduckdb_metadata_cache.hpp" -#include "pgduckdb/pgduckdb_utils.hpp" #include "pgduckdb/utility/copy.hpp" #include "pgduckdb/vendor/pg_list.hpp" #include diff --git a/src/pgduckdb_detoast.cpp b/src/pgduckdb_detoast.cpp index 431441b4..1597b8da 100644 --- a/src/pgduckdb_detoast.cpp +++ b/src/pgduckdb_detoast.cpp @@ -1,5 +1,8 @@ #include "duckdb.hpp" +#include "pgduckdb/pgduckdb_types.hpp" +#include "pgduckdb/pgduckdb_utils.hpp" + extern "C" { #include "postgres.h" #include "pg_config.h" @@ -20,9 +23,7 @@ extern "C" { } #include "pgduckdb/pgduckdb_process_lock.hpp" -#include "pgduckdb/pgduckdb_types.hpp" #include "pgduckdb/pgduckdb_detoast.hpp" -#include "pgduckdb/pgduckdb_utils.hpp" /* * Following functions are direct logic found in postgres code but for duckdb execution they are needed to be thread diff --git a/src/pgduckdb_duckdb.cpp b/src/pgduckdb_duckdb.cpp index 1d7fa59c..f18b43ec 100644 --- a/src/pgduckdb_duckdb.cpp +++ b/src/pgduckdb_duckdb.cpp @@ -10,6 +10,7 @@ #include "pgduckdb/scan/postgres_scan.hpp" #include "pgduckdb/scan/postgres_seq_scan.hpp" #include "pgduckdb/pg/transactions.hpp" +#include "pgduckdb/pgduckdb_utils.hpp" extern "C" { #include "postgres.h" @@ -23,7 +24,6 @@ extern "C" { #include "pgduckdb/pgduckdb_options.hpp" #include "pgduckdb/pgduckdb_xact.hpp" #include "pgduckdb/pgduckdb_metadata_cache.hpp" -#include "pgduckdb/pgduckdb_utils.hpp" #include #include diff --git a/src/pgduckdb_filter.cpp b/src/pgduckdb_filter.cpp index aa53ea04..6ea6e032 100644 --- a/src/pgduckdb_filter.cpp +++ b/src/pgduckdb_filter.cpp @@ -1,5 +1,6 @@ #include "duckdb.hpp" #include "duckdb/planner/filter/constant_filter.hpp" +#include "pgduckdb/pgduckdb_types.hpp" extern "C" { #include "postgres.h" @@ -14,7 +15,6 @@ extern "C" { #include "pgduckdb/pgduckdb_filter.hpp" #include "pgduckdb/pgduckdb_detoast.hpp" -#include "pgduckdb/pgduckdb_types.hpp" namespace pgduckdb { diff --git a/src/pgduckdb_hooks.cpp b/src/pgduckdb_hooks.cpp index 23b80b23..6936edbe 100644 --- a/src/pgduckdb_hooks.cpp +++ b/src/pgduckdb_hooks.cpp @@ -3,6 +3,7 @@ #include "pgduckdb/pgduckdb_planner.hpp" #include "pgduckdb/pg/transactions.hpp" #include "pgduckdb/pgduckdb_xact.hpp" +#include "pgduckdb/pgduckdb_utils.hpp" extern "C" { #include "postgres.h" @@ -29,8 +30,8 @@ extern "C" { #include "pgduckdb/utility/copy.hpp" #include "pgduckdb/vendor/pg_explain.hpp" #include "pgduckdb/vendor/pg_list.hpp" -#include "pgduckdb/pgduckdb_utils.hpp" #include "pgduckdb/pgduckdb_node.hpp" +#include "pgduckdb/utility/cpp_wrapper.hpp" static planner_hook_type prev_planner_hook = NULL; static ExecutorStart_hook_type prev_executor_start_hook = NULL; diff --git a/src/pgduckdb_node.cpp b/src/pgduckdb_node.cpp index c05ff3eb..42a5d2b8 100644 --- a/src/pgduckdb_node.cpp +++ b/src/pgduckdb_node.cpp @@ -4,6 +4,7 @@ #include "duckdb/common/exception.hpp" #include "pgduckdb/pgduckdb_planner.hpp" +#include "pgduckdb/pgduckdb_types.hpp" extern "C" { #include "postgres.h" @@ -14,9 +15,8 @@ extern "C" { } #include "pgduckdb/pgduckdb_node.hpp" -#include "pgduckdb/pgduckdb_types.hpp" #include "pgduckdb/pgduckdb_duckdb.hpp" -#include "pgduckdb/pgduckdb_utils.hpp" +#include "pgduckdb/utility/cpp_wrapper.hpp" /* global variables */ CustomScanMethods duckdb_scan_scan_methods; diff --git a/src/pgduckdb_options.cpp b/src/pgduckdb_options.cpp index a09a93a4..dc130d09 100644 --- a/src/pgduckdb_options.cpp +++ b/src/pgduckdb_options.cpp @@ -4,6 +4,9 @@ #include #include +#include "pgduckdb/pgduckdb_types.hpp" +#include "pgduckdb/pgduckdb_utils.hpp" + extern "C" { #include "postgres.h" #include "miscadmin.h" @@ -24,8 +27,8 @@ extern "C" { #include "pgduckdb/pgduckdb_options.hpp" #include "pgduckdb/pgduckdb_duckdb.hpp" #include "pgduckdb/pgduckdb_utils.hpp" -#include "pgduckdb/pgduckdb_types.hpp" #include "pgduckdb/pgduckdb_xact.hpp" +#include "pgduckdb/utility/cpp_wrapper.hpp" namespace pgduckdb { diff --git a/src/pgduckdb_planner.cpp b/src/pgduckdb_planner.cpp index c982c49e..703677d4 100644 --- a/src/pgduckdb_planner.cpp +++ b/src/pgduckdb_planner.cpp @@ -4,6 +4,7 @@ #include "pgduckdb/catalog/pgduckdb_transaction.hpp" #include "pgduckdb/scan/postgres_scan.hpp" +#include "pgduckdb/pgduckdb_types.hpp" extern "C" { #include "postgres.h" @@ -24,8 +25,7 @@ extern "C" { #include "pgduckdb/pgduckdb_duckdb.hpp" #include "pgduckdb/pgduckdb_node.hpp" -#include "pgduckdb/pgduckdb_types.hpp" -#include "pgduckdb/pgduckdb_utils.hpp" +#include "pgduckdb/utility/cpp_wrapper.hpp" bool duckdb_explain_analyze = false; diff --git a/src/pgduckdb_types.cpp b/src/pgduckdb_types.cpp index 65b2a570..0c950218 100644 --- a/src/pgduckdb_types.cpp +++ b/src/pgduckdb_types.cpp @@ -3,7 +3,9 @@ #include "duckdb/common/extra_type_info.hpp" #include "duckdb/common/types/uuid.hpp" +#include "pgduckdb/pgduckdb_types.hpp" #include "pgduckdb/scan/postgres_scan.hpp" +#include "pgduckdb/types/decimal.hpp" extern "C" { #include "postgres.h" @@ -26,7 +28,6 @@ extern "C" { #include "pgduckdb/types/decimal.hpp" #include "pgduckdb/pgduckdb_filter.hpp" #include "pgduckdb/pgduckdb_detoast.hpp" -#include "pgduckdb/pgduckdb_types.hpp" namespace pgduckdb { diff --git a/src/pgduckdb_xact.cpp b/src/pgduckdb_xact.cpp index e8d43fc8..9e807c2d 100644 --- a/src/pgduckdb_xact.cpp +++ b/src/pgduckdb_xact.cpp @@ -1,7 +1,9 @@ #include "duckdb/common/exception.hpp" #include "pgduckdb/pgduckdb_duckdb.hpp" #include "pgduckdb/pgduckdb_utils.hpp" + #include "pgduckdb/pg/transactions.hpp" +#include "pgduckdb/utility/cpp_wrapper.hpp" namespace pgduckdb { diff --git a/src/scan/heap_reader.cpp b/src/scan/heap_reader.cpp index 3d09f974..cc4f37e3 100644 --- a/src/scan/heap_reader.cpp +++ b/src/scan/heap_reader.cpp @@ -1,6 +1,8 @@ #include "duckdb.hpp" #include "pgduckdb/scan/heap_reader.hpp" +#include "pgduckdb/pgduckdb_types.hpp" +#include "pgduckdb/pgduckdb_utils.hpp" extern "C" { #include "postgres.h" @@ -12,8 +14,6 @@ extern "C" { } #include "pgduckdb/pgduckdb_process_lock.hpp" -#include "pgduckdb/pgduckdb_types.hpp" -#include "pgduckdb/pgduckdb_utils.hpp" #include diff --git a/src/scan/postgres_scan.cpp b/src/scan/postgres_scan.cpp index 151ff69f..ffa9603e 100644 --- a/src/scan/postgres_scan.cpp +++ b/src/scan/postgres_scan.cpp @@ -13,6 +13,8 @@ #include "duckdb/common/enums/expression_type.hpp" #include "pgduckdb/scan/postgres_scan.hpp" +#include "pgduckdb/pgduckdb_types.hpp" +#include "pgduckdb/pgduckdb_utils.hpp" extern "C" { #include "postgres.h" @@ -28,8 +30,6 @@ extern "C" { } #include "pgduckdb/pgduckdb_process_lock.hpp" -#include "pgduckdb/pgduckdb_types.hpp" -#include "pgduckdb/pgduckdb_utils.hpp" namespace pgduckdb {