Skip to content

Commit

Permalink
Cache IsExtensionRegistered
Browse files Browse the repository at this point in the history
Doing a btree lookup in `pg_extension` for every query is not free, and
an extension introducing one can (based on my experience) impact
performance for single row lookups significantly.

This introduces a cache which tracks if the extension is registered. I
plan to use this cache in later PRs for more things, such as oids of our
DuckDB functions.

This cache is invalidated whenever an invalidation message is sent for
the "duckdb" schema, which just so happens to be whenever `CREATE/DROP
EXTENSION pg_duckdb` is executed. These invalidation messages are the
mechanism that Postgres uses to let other backends know that some DDL
has occured. We'll probably want to do something else for `ALTER
EXTENSION pg_duckdb UPDATE`, but for now this is good enough (since
there's no upgrades yet). It would have been best if we receive
invalidation messages for rows in `pg_extension`. But sadly this is not
possible. I submitted a patch to upstream Postgres to hopefully allow
for this in the future (but that will only be in PG18+):
https://www.postgresql.org/message-id/flat/CAGECzQTWm9sex719Hptbq4j56hBGUti7J9OWjeMobQ1ccRok9w%40mail.gmail.com
  • Loading branch information
JelteF committed Aug 13, 2024
1 parent cb4a243 commit 0f83fb7
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 7 deletions.
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ SRCS = src/scan/heap_reader.cpp \
src/scan/postgres_scan.cpp \
src/scan/postgres_seq_scan.cpp \
src/utility/copy.cpp \
src/pgduckdb_cache.cpp \
src/pgduckdb_detoast.cpp \
src/pgduckdb_duckdb.cpp \
src/pgduckdb_filter.cpp \
Expand Down
3 changes: 3 additions & 0 deletions include/pgduckdb/pgduckdb_cache.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
namespace pgduckdb {
bool IsExtensionRegistered();
}
45 changes: 45 additions & 0 deletions src/pgduckdb_cache.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
extern "C" {
#include "postgres.h"

#include "commands/extension.h"
#include "utils/inval.h"
#include "utils/syscache.h"
}

namespace pgduckdb {
struct {
bool valid;
bool installed;
} cache = {};

bool callback_is_configured = false;
uint32 schema_hash_value;

static void
InvalidateCaches(Datum arg, int cache_id, uint32 hash_value) {
if (hash_value != schema_hash_value) {
return;
}
cache.valid = false;
}

bool
IsExtensionRegistered() {
if (cache.valid) {
return cache.installed;
}

cache.installed = get_extension_oid("pg_duckdb", true) != InvalidOid;
cache.valid = true;

if (!callback_is_configured) {
callback_is_configured = true;
schema_hash_value = GetSysCacheHashValue1(NAMESPACENAME, CStringGetDatum("duckdb"));

CacheRegisterSyscacheCallback(NAMESPACENAME, InvalidateCaches, (Datum)0);
}

return cache.installed;
}

} // namespace pgduckdb
10 changes: 3 additions & 7 deletions src/pgduckdb_hooks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,14 @@ extern "C" {
}

#include "pgduckdb/pgduckdb.h"
#include "pgduckdb/pgduckdb_cache.hpp"
#include "pgduckdb/pgduckdb_planner.hpp"
#include "pgduckdb/utility/copy.hpp"
#include "pgduckdb/vendor/pg_list.hpp"

static planner_hook_type prev_planner_hook = NULL;
static ProcessUtility_hook_type prev_process_utility_hook = NULL;

static bool
IsDuckdbExtensionRegistered() {
return get_extension_oid("pg_duckdb", true) != InvalidOid;
}

static bool
IsCatalogTable(List *tables) {
foreach_node(RangeTblEntry, table, tables) {
Expand Down Expand Up @@ -57,7 +53,7 @@ IsAllowedStatement() {

static PlannedStmt *
DuckdbPlannerHook(Query *parse, const char *query_string, int cursor_options, ParamListInfo bound_params) {
if (duckdb_execution && IsAllowedStatement() && IsDuckdbExtensionRegistered() && parse->rtable &&
if (duckdb_execution && IsAllowedStatement() && pgduckdb::IsExtensionRegistered() && parse->rtable &&
!IsCatalogTable(parse->rtable) && parse->commandType == CMD_SELECT) {
PlannedStmt *duckdb_plan = DuckdbPlanNode(parse, query_string, cursor_options, bound_params);
if (duckdb_plan) {
Expand All @@ -76,7 +72,7 @@ static void
DuckdbUtilityHook(PlannedStmt *pstmt, const char *query_string, bool read_only_tree, ProcessUtilityContext context,
ParamListInfo params, struct QueryEnvironment *query_env, DestReceiver *dest, QueryCompletion *qc) {
Node *parsetree = pstmt->utilityStmt;
if (duckdb_execution && IsDuckdbExtensionRegistered() && IsA(parsetree, CopyStmt)) {
if (duckdb_execution && pgduckdb::IsExtensionRegistered() && IsA(parsetree, CopyStmt)) {
uint64 processed;
if (DuckdbCopy(pstmt, query_string, query_env, &processed)) {
if (qc) {
Expand Down

0 comments on commit 0f83fb7

Please sign in to comment.