From 6d8dff293973edd1be9cc67be03aac400e85cb4c Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Fri, 1 May 2026 08:41:44 +0000 Subject: [PATCH] docs(chassis): mark query-hook ABI scaffold-state explicit + add TODO markers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The chassis ABI 2 query-hook surface (ProxySQL_PluginQueryHookPayload / Result / Action plus register_query_hook / dispatch_configured_plugin_ query_hook) is wired through ProxySQL_PluginManager and exercised end- to-end by unit tests. But the production data plane never calls proxysql_dispatch_configured_plugin_query_hook — neither MySQL_Session nor PgSQL_Session has the integration point. Net effect today: a plugin can register a query hook successfully and unit tests can drive dispatch through it, but a real client query arriving over MySQL or PgSQL will never consult the hook. The DENY contract from the public ABI ("DENY prevents a query from dispatching") is currently a promise the production path doesn't keep. # Why scaffold-state instead of immediate full integration Wiring the dispatch into MySQL_Session::handler___status_WAITING_ CLIENT_DATA___STATE_SLEEP___MYSQL_COM_QUERY_qpo (and the COM_STMT_ PREPARE / COM_STMT_EXECUTE entries that share the qpo machinery) is mechanical but touches a 95k-LOC hot-path file. A bug in the integration would manifest as a query-blocking regression that's hard to debug and easy to ship. Same for PgSQL_Session. The integration deserves its own focused PR with TAP coverage that exercises both ALLOW and DENY paths against a fake plugin — not a side-quest in this PR which is already large. # What this commit does 1. Updates the ABI doc block in include/ProxySQL_Plugin.h to call out the scaffold state explicitly. A plugin author reading the header today would reasonably assume the hook fires in production; that assumption is wrong, and the doc block now says so. References the precise grep target ("TODO(plugin-query-hook)") that points at the missing injection sites. 2. Adds two TODO(plugin-query-hook) markers to MySQL_Session.cpp: - line ~3387 (COM_STMT_PREPARE → COM_QUERY shared codepath) - line ~5403 (the main COM_QUERY execution path) Each immediately follows the GloMyQPro->process_query call, where CurrentQuery is populated and the dispatch payload would be ready. 3. Adds one TODO(plugin-query-hook) marker to PgSQL_Session.cpp at line ~2387, following the GloPgQPro->process_query call. # What this commit deliberately does NOT do The actual dispatch wiring. The next person picking this up has: - A precise injection site (3 in MySQL, 1+ in PgSQL). - Pre-built ABI surface (proxysql_has_configured_plugin_query_hook for the lock-free fast-path probe; proxysql_dispatch_configured_ plugin_query_hook for the actual hook invocation under shared lock). - Existing test_helpers/fake_plugin.cpp with a HOOK_DENY env toggle that exercises the dispatch path under unit test today — extensible to TAP integration tests once the production wiring lands. Caught by an external review pass, finding #5. Filing as a deferred fix with explicit scaffold acknowledgement rather than rushing the integration. --- include/ProxySQL_Plugin.h | 13 +++++++++++++ lib/MySQL_Session.cpp | 15 +++++++++++++++ lib/PgSQL_Session.cpp | 9 +++++++++ 3 files changed, 37 insertions(+) diff --git a/include/ProxySQL_Plugin.h b/include/ProxySQL_Plugin.h index d1d1e67d4..6bb9977e4 100644 --- a/include/ProxySQL_Plugin.h +++ b/include/ProxySQL_Plugin.h @@ -102,6 +102,19 @@ using proxysql_plugin_log_message_cb = #ifdef PROXYSQL40 // Pre-execution query hook (Step 2 ABI extension). // +// STATUS (chassis ABI 2 — initial baseline): the registration and +// dispatch path are wired through ProxySQL_PluginManager and the +// chassis fast-path probe (proxysql_has_configured_plugin_query_hook), +// and unit tests exercise the dispatch surface end-to-end. However, +// the production data plane (MySQL_Session::handler___status_WAITING_ +// CLIENT_DATA___STATE_SLEEP___MYSQL_COM_QUERY_qpo and the matching +// PgSQL_Session COM_QUERY entry) does NOT yet call the dispatch +// helper. Plugins that register a query hook today will see the +// callback invoked from unit tests but not from real client traffic. +// Wiring the production fast-path is a deliberate follow-up — search +// for "TODO(plugin-query-hook)" in lib/MySQL_Session.cpp and +// lib/PgSQL_Session.cpp for the precise injection points. +// // Wire protocol the hook is being invoked for. A plugin can register // independently for each protocol; one hook per protocol per plugin. enum class ProxySQL_PluginProtocol : uint8_t { diff --git a/lib/MySQL_Session.cpp b/lib/MySQL_Session.cpp index c1a36d248..62ff19e53 100644 --- a/lib/MySQL_Session.cpp +++ b/lib/MySQL_Session.cpp @@ -3383,6 +3383,16 @@ void MySQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C (begint.tv_sec*1000000000+begint.tv_nsec); } assert(qpo); // GloMyQPro->process_mysql_query() should always return a qpo + // TODO(plugin-query-hook): inject pre-execution hook dispatch + // here, after CurrentQuery is populated and before the query + // is committed for backend execution. The chassis ABI surface + // (proxysql_has_configured_plugin_query_hook + + // proxysql_dispatch_configured_plugin_query_hook in + // include/ProxySQL_PluginManager.h) is in place; this is the + // missing data-plane integration. See ProxySQL_Plugin.h's + // "STATUS (chassis ABI 2 — initial baseline)" comment for + // the full scope. Same injection pattern needed at line ~5398 + // (this same source file) and the PgSQL COM_QUERY entry. // setting 'prepared' to prevent fetching results from the cache if the digest matches rc_break=handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_COM_QUERY_qpo(&pkt, &lock_hostgroup, ps_type_prepare_stmt); if (rc_break==true) { @@ -5402,6 +5412,11 @@ __get_pkts_from_client: (endt.tv_sec*1000000000+endt.tv_nsec) - (begint.tv_sec*1000000000+begint.tv_nsec); } + // TODO(plugin-query-hook): inject pre-execution + // hook dispatch here for the COM_QUERY codepath, + // after CurrentQuery is populated. See + // ProxySQL_Plugin.h "STATUS (chassis ABI 2 — + // initial baseline)". assert(qpo); // GloMyQPro->process_mysql_query() should always return a qpo { diff --git a/lib/PgSQL_Session.cpp b/lib/PgSQL_Session.cpp index df3456327..c15eb0d63 100644 --- a/lib/PgSQL_Session.cpp +++ b/lib/PgSQL_Session.cpp @@ -2385,6 +2385,15 @@ __implicit_sync: (endt.tv_sec * 1000000000 + endt.tv_nsec) - (begint.tv_sec * 1000000000 + begint.tv_nsec); } + // TODO(plugin-query-hook): inject pre-execution + // hook dispatch here for the PgSQL COM_QUERY + // codepath, after CurrentQuery is populated. + // Use proxysql_dispatch_configured_plugin_ + // query_hook with ProxySQL_PluginProtocol::pgsql. + // See ProxySQL_Plugin.h "STATUS (chassis ABI 2 — + // initial baseline)" — the dispatch ABI is + // already in place; this is the data-plane + // integration that needs to land. assert(qpo); // GloPgQPro->process_mysql_query() should always return a qpo // =================================================== if (qpo->max_lag_ms >= 0) {