Throw error when changing a duckdb GUC after DuckDB is initialized #743
Conversation
ef3967e to
c5c246a
Compare
src/pgduckdb.cpp
Outdated
| @@ -122,53 +135,54 @@ DuckdbInitGUC() { | |||
| &duckdb_log_pg_explain); | |||
|
|
|||
| DefineCustomVariable("duckdb.enable_external_access", "Allow the DuckDB to access external state.", | |||
| &duckdb_enable_external_access, PGC_SUSET); | |||
| &duckdb_enable_external_access, PGC_SUSET, 0, GucCheckDuckDBNotInitdHook); | |||
There was a problem hiding this comment.
nit: does it make sense to add another template without the flags argument, so we don't need to specify 0 soo many times.
There was a problem hiding this comment.
Ah I went back and forth on that. I thought I remembered that you created those with all the possible args, so didn't want to add toe many helpers.
But maybe we do want a DefineCustomDuckDBVariable that would do exactly that. I'll give it a shot so we can compare
src/pgduckdb.cpp
Outdated
| GucCheckDuckDBNotInitdHook(T *, void **, GucSource) { | ||
| if (pgduckdb::DuckDBManager::IsInitialized()) { | ||
| GUC_check_errmsg( | ||
| "Cannot set this variable after DuckDB has been initialized. Use `duckdb.recycle_ddb()` to reset " |
There was a problem hiding this comment.
| "Cannot set this variable after DuckDB has been initialized. Use `duckdb.recycle_ddb()` to reset " | |
| "Cannot set this variable after DuckDB has been initialized. Reconnect to Postgres or use `duckdb.recycle_ddb()` to reset " |
| GucIntCheckHook check_hook, | ||
| GucIntAssignHook assign_hook, | ||
| GucTypeCheckHook<T> check_hook, | ||
| GucTypeAssignHook<T> assign_hook, |
There was a problem hiding this comment.
This was a "bug" - using a float type would have failed to compile
| using GucTypeAssignHook = void (*)(T, void *); | ||
|
|
||
| template <typename T> | ||
| void |
There was a problem hiding this comment.
Let's put static back so it doesn't conflict with symbols from possibly other libraries.
| void | |
| static void |
There was a problem hiding this comment.
So these are templates, so it won't conflict with anything, and I've put all the concrete implementation in the anonymous ns:
Line 66 in f1fc8f4
Fixes #736