From c8f7046b3e2ac9606ddf3910e36f145eb160c73d Mon Sep 17 00:00:00 2001 From: Guillaume Outters Date: Fri, 12 Mar 2021 00:16:18 +0100 Subject: [PATCH 1/6] pgsqlSetNoticeCallback Allows a callback to be triggered on every notice sent by PostgreSQL. Such notices can be sent with a RAISE NOTICE in PL/pgSQL; in a long running stored procedure, they prove useful as realtime checkpoint indicators. --- ext/pdo_pgsql/pgsql_driver.c | 78 +++++++++++++++++++++++++++- ext/pdo_pgsql/pgsql_driver.stub.php | 3 ++ ext/pdo_pgsql/pgsql_driver_arginfo.h | 8 ++- ext/pdo_pgsql/php_pdo_pgsql_int.h | 6 +++ 4 files changed, 92 insertions(+), 3 deletions(-) diff --git a/ext/pdo_pgsql/pgsql_driver.c b/ext/pdo_pgsql/pgsql_driver.c index 525f9cd1c15f7..3833abc2b4d57 100644 --- a/ext/pdo_pgsql/pgsql_driver.c +++ b/ext/pdo_pgsql/pgsql_driver.c @@ -104,7 +104,23 @@ int _pdo_pgsql_error(pdo_dbh_t *dbh, pdo_stmt_t *stmt, int errcode, const char * static void _pdo_pgsql_notice(pdo_dbh_t *dbh, const char *message) /* {{{ */ { -/* pdo_pgsql_db_handle *H = (pdo_pgsql_db_handle *)dbh->driver_data; */ + int ret; + zval zarg; + zval retval; + pdo_pgsql_fci * fc; + if ((fc = ((pdo_pgsql_db_handle *)dbh->driver_data)->notice_callback)) { + ZVAL_STRINGL(&zarg, (char *) message, strlen(message)); + fc->fci.param_count = 1; + fc->fci.params = &zarg; + fc->fci.retval = &retval; + if ((ret = zend_call_function(&fc->fci, &fc->fcc)) != FAILURE) { + zval_ptr_dtor(&retval); + } + zval_ptr_dtor(&zarg); + if (ret == FAILURE) { + pdo_raise_impl_error(dbh, NULL, "HY000", "could not call user-supplied function"); + } + } } /* }}} */ @@ -125,6 +141,16 @@ static void pdo_pgsql_fetch_error_func(pdo_dbh_t *dbh, pdo_stmt_t *stmt, zval *i } /* }}} */ +static void pdo_pgsql_cleanup_notice_callback(pdo_pgsql_db_handle *H) /* {{{ */ +{ + if (H->notice_callback) { + zval_ptr_dtor(&H->notice_callback->fci.function_name); + efree(H->notice_callback); + H->notice_callback = NULL; + } +} +/* }}} */ + /* {{{ pdo_pgsql_create_lob_stream */ static ssize_t pgsql_lob_write(php_stream *stream, const char *buf, size_t count) { @@ -229,6 +255,7 @@ static void pgsql_handle_closer(pdo_dbh_t *dbh) /* {{{ */ pefree(H->lob_streams, dbh->is_persistent); H->lob_streams = NULL; } + pdo_pgsql_cleanup_notice_callback(H); if (H->server) { PQfinish(H->server); H->server = NULL; @@ -1224,6 +1251,53 @@ PHP_METHOD(PDO_PGSql_Ext, pgsqlGetPid) } /* }}} */ +/* {{{ proto bool PDO::pgsqlSetNoticeCallback(mixed callback) + Sets a callback to receive DB notices (after client_min_messages has been set) */ +PHP_METHOD(PDO_PGSql_Ext, pgsqlSetNoticeCallback) +{ + zval *callback; + zend_string *cbname; + pdo_dbh_t *dbh; + pdo_pgsql_db_handle *H; + pdo_pgsql_fci *fc; + + if (FAILURE == zend_parse_parameters(ZEND_NUM_ARGS(), "z", &callback)) { + RETURN_FALSE; + } + + dbh = Z_PDO_DBH_P(getThis()); + PDO_CONSTRUCT_CHECK; + + H = (pdo_pgsql_db_handle *)dbh->driver_data; + + if (Z_TYPE_P(callback) == IS_NULL) { + pdo_pgsql_cleanup_notice_callback(H); + RETURN_TRUE; + } else { + if (!(fc = H->notice_callback)) { + fc = (pdo_pgsql_fci*)ecalloc(1, sizeof(pdo_pgsql_fci)); + } else { + zval_ptr_dtor(&fc->fci.function_name); + memcpy(&fc->fcc, &empty_fcall_info_cache, sizeof(fc->fcc)); + } + + if (FAILURE == zend_fcall_info_init(callback, 0, &fc->fci, &fc->fcc, &cbname, NULL)) { + php_error_docref(NULL, E_WARNING, "function '%s' is not callable", ZSTR_VAL(cbname)); + zend_string_release_ex(cbname, 0); + efree(fc); + H->notice_callback = NULL; + RETURN_FALSE; + } + Z_TRY_ADDREF_P(&fc->fci.function_name); + zend_string_release_ex(cbname, 0); + + H->notice_callback = fc; + + RETURN_TRUE; + } +} +/* }}} */ + static const zend_function_entry *pdo_pgsql_get_driver_methods(pdo_dbh_t *dbh, int kind) { switch (kind) { @@ -1341,7 +1415,7 @@ static int pdo_pgsql_handle_factory(pdo_dbh_t *dbh, zval *driver_options) /* {{{ goto cleanup; } - PQsetNoticeProcessor(H->server, (void(*)(void*,const char*))_pdo_pgsql_notice, (void *)&dbh); + PQsetNoticeProcessor(H->server, (void(*)(void*,const char*))_pdo_pgsql_notice, (void *)dbh); H->attached = 1; H->pgoid = -1; diff --git a/ext/pdo_pgsql/pgsql_driver.stub.php b/ext/pdo_pgsql/pgsql_driver.stub.php index cf3504fbbfb83..8c989be88927b 100644 --- a/ext/pdo_pgsql/pgsql_driver.stub.php +++ b/ext/pdo_pgsql/pgsql_driver.stub.php @@ -33,4 +33,7 @@ public function pgsqlGetNotify(int $fetchMode = PDO::FETCH_DEFAULT, int $timeout /** @tentative-return-type */ public function pgsqlGetPid(): int {} + + /** @tentative-return-type */ + public function pgsqlSetNoticeCallback(?callable $callback): bool {} } diff --git a/ext/pdo_pgsql/pgsql_driver_arginfo.h b/ext/pdo_pgsql/pgsql_driver_arginfo.h index 2b858e0a1ad1c..7f5f276f52b1b 100644 --- a/ext/pdo_pgsql/pgsql_driver_arginfo.h +++ b/ext/pdo_pgsql/pgsql_driver_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 9bb79af98dbb7c171fd9533aeabece4937a06cd2 */ + * Stub hash: f1bbbb97912bd83638aaf6a1d843ead2181894a0 */ ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX(arginfo_class_PDO_PGSql_Ext_pgsqlCopyFromArray, 0, 2, _IS_BOOL, 0) ZEND_ARG_TYPE_INFO(0, tableName, IS_STRING, 0) @@ -46,6 +46,10 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX(arginfo_class_PDO_PGSql_Ext_pgsqlGetPid, 0, 0, IS_LONG, 0) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX(arginfo_class_PDO_PGSql_Ext_pgsqlSetNoticeCallback, 0, 1, _IS_BOOL, 0) + ZEND_ARG_TYPE_INFO(0, callback, IS_CALLABLE, 1) +ZEND_END_ARG_INFO() + ZEND_METHOD(PDO_PGSql_Ext, pgsqlCopyFromArray); ZEND_METHOD(PDO_PGSql_Ext, pgsqlCopyFromFile); ZEND_METHOD(PDO_PGSql_Ext, pgsqlCopyToArray); @@ -55,6 +59,7 @@ ZEND_METHOD(PDO_PGSql_Ext, pgsqlLOBOpen); ZEND_METHOD(PDO_PGSql_Ext, pgsqlLOBUnlink); ZEND_METHOD(PDO_PGSql_Ext, pgsqlGetNotify); ZEND_METHOD(PDO_PGSql_Ext, pgsqlGetPid); +ZEND_METHOD(PDO_PGSql_Ext, pgsqlSetNoticeCallback); static const zend_function_entry class_PDO_PGSql_Ext_methods[] = { ZEND_ME(PDO_PGSql_Ext, pgsqlCopyFromArray, arginfo_class_PDO_PGSql_Ext_pgsqlCopyFromArray, ZEND_ACC_PUBLIC) @@ -66,5 +71,6 @@ static const zend_function_entry class_PDO_PGSql_Ext_methods[] = { ZEND_ME(PDO_PGSql_Ext, pgsqlLOBUnlink, arginfo_class_PDO_PGSql_Ext_pgsqlLOBUnlink, ZEND_ACC_PUBLIC) ZEND_ME(PDO_PGSql_Ext, pgsqlGetNotify, arginfo_class_PDO_PGSql_Ext_pgsqlGetNotify, ZEND_ACC_PUBLIC) ZEND_ME(PDO_PGSql_Ext, pgsqlGetPid, arginfo_class_PDO_PGSql_Ext_pgsqlGetPid, ZEND_ACC_PUBLIC) + ZEND_ME(PDO_PGSql_Ext, pgsqlSetNoticeCallback, arginfo_class_PDO_PGSql_Ext_pgsqlSetNoticeCallback, ZEND_ACC_PUBLIC) ZEND_FE_END }; diff --git a/ext/pdo_pgsql/php_pdo_pgsql_int.h b/ext/pdo_pgsql/php_pdo_pgsql_int.h index 303aff6006058..b50b38ba658b4 100644 --- a/ext/pdo_pgsql/php_pdo_pgsql_int.h +++ b/ext/pdo_pgsql/php_pdo_pgsql_int.h @@ -32,6 +32,11 @@ typedef struct { char *errmsg; } pdo_pgsql_error_info; +typedef struct { + zend_fcall_info fci; + zend_fcall_info_cache fcc; +} pdo_pgsql_fci; + /* stuff we use in a pgsql database handle */ typedef struct { PGconn *server; @@ -46,6 +51,7 @@ typedef struct { bool disable_native_prepares; /* deprecated since 5.6 */ bool disable_prepares; HashTable *lob_streams; + pdo_pgsql_fci * notice_callback; } pdo_pgsql_db_handle; typedef struct { From bcf15f4c685c8c7cd06c62916bcd8ba7499e8995 Mon Sep 17 00:00:00 2001 From: Guillaume Outters Date: Fri, 3 Jan 2020 14:27:50 +0100 Subject: [PATCH 2/6] pgsqlSetNoticeCallback: test case Add a test for a standard pgsqlSetNoticeCallback use. --- ext/pdo_pgsql/tests/issue78621.inc | 15 +++++++++++++++ ext/pdo_pgsql/tests/issue78621.phpt | 28 ++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 ext/pdo_pgsql/tests/issue78621.inc create mode 100644 ext/pdo_pgsql/tests/issue78621.phpt diff --git a/ext/pdo_pgsql/tests/issue78621.inc b/ext/pdo_pgsql/tests/issue78621.inc new file mode 100644 index 0000000000000..b1cdc990fd1ec --- /dev/null +++ b/ext/pdo_pgsql/tests/issue78621.inc @@ -0,0 +1,15 @@ +beginTransaction(); +$db->exec("create temporary table t (a varchar(3))"); +$db->exec("create function hey() returns trigger as \$\$ begin new.a := 'oh'; raise notice 'I tampered your data, did you know?'; return new; end; \$\$ language plpgsql"); +$db->exec("create trigger hop before insert on t for each row execute procedure hey()"); +$db->exec("insert into t values ('ah')"); +var_dump($db->query("select * from t")->fetchAll(PDO::FETCH_ASSOC)); +echo "Done\n"; +?> diff --git a/ext/pdo_pgsql/tests/issue78621.phpt b/ext/pdo_pgsql/tests/issue78621.phpt new file mode 100644 index 0000000000000..89213dd38472a --- /dev/null +++ b/ext/pdo_pgsql/tests/issue78621.phpt @@ -0,0 +1,28 @@ +--TEST-- +pgsqlSetNoticeCallback catches Postgres "raise notice". +--SKIPIF-- + +--FILE-- +pgsqlSetNoticeCallback('disp'); +} +require dirname(__FILE__) . '/issue78621.inc'; +?> +--EXPECT-- +NOTICE: I tampered your data, did you know? +array(1) { + [0]=> + array(1) { + ["a"]=> + string(2) "oh" + } +} +Done From 3862ac2bec5b0c4fce9da1c8b6cbd7d62d3ce7b9 Mon Sep 17 00:00:00 2001 From: Guillaume Outters Date: Fri, 3 Jan 2020 14:44:13 +0100 Subject: [PATCH 3/6] pgsqlSetNoticeCallback: test case for closure callback https://github.com/php/php-src/pull/4823#discussion_r360669537 Closure cannot be used as a callback if not persisted. --- ext/pdo_pgsql/tests/issue78621_closure.phpt | 30 +++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 ext/pdo_pgsql/tests/issue78621_closure.phpt diff --git a/ext/pdo_pgsql/tests/issue78621_closure.phpt b/ext/pdo_pgsql/tests/issue78621_closure.phpt new file mode 100644 index 0000000000000..c116cde305d95 --- /dev/null +++ b/ext/pdo_pgsql/tests/issue78621_closure.phpt @@ -0,0 +1,30 @@ +--TEST-- +pgsqlSetNoticeCallback catches Postgres "raise notice". +--SKIPIF-- + +--FILE-- +pgsqlSetNoticeCallback(function($message) { echo trim($message)."\n"; }); + // https://github.com/php/php-src/pull/4823#pullrequestreview-335623806 + $eraseCallbackMemoryHere = (object)[1]; +} +require dirname(__FILE__) . '/issue78621.inc'; +?> +--EXPECT-- +NOTICE: I tampered your data, did you know? +array(1) { + [0]=> + array(1) { + ["a"]=> + string(2) "oh" + } +} +Done From 3011850ee0f5fcf3f1563a30c8488b1e8a2a03b3 Mon Sep 17 00:00:00 2001 From: Guillaume Outters Date: Fri, 3 Jan 2020 17:36:21 +0100 Subject: [PATCH 4/6] pgsqlSetNoticeCallback: tests: ensure where are "multiple calls"-proof --- ext/pdo_pgsql/tests/issue78621.inc | 6 ++++++ ext/pdo_pgsql/tests/issue78621.phpt | 6 ++++-- ext/pdo_pgsql/tests/issue78621_closure.phpt | 5 +++-- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/ext/pdo_pgsql/tests/issue78621.inc b/ext/pdo_pgsql/tests/issue78621.inc index b1cdc990fd1ec..42236d803163d 100644 --- a/ext/pdo_pgsql/tests/issue78621.inc +++ b/ext/pdo_pgsql/tests/issue78621.inc @@ -10,6 +10,12 @@ $db->exec("create temporary table t (a varchar(3))"); $db->exec("create function hey() returns trigger as \$\$ begin new.a := 'oh'; raise notice 'I tampered your data, did you know?'; return new; end; \$\$ language plpgsql"); $db->exec("create trigger hop before insert on t for each row execute procedure hey()"); $db->exec("insert into t values ('ah')"); +attach($db, 'Re'); +$db->exec("delete from t"); +$db->exec("insert into t values ('ah')"); +$db->pgsqlSetNoticeCallback(null); +$db->exec("delete from t"); +$db->exec("insert into t values ('ah')"); var_dump($db->query("select * from t")->fetchAll(PDO::FETCH_ASSOC)); echo "Done\n"; ?> diff --git a/ext/pdo_pgsql/tests/issue78621.phpt b/ext/pdo_pgsql/tests/issue78621.phpt index 89213dd38472a..ee474a116cbdb 100644 --- a/ext/pdo_pgsql/tests/issue78621.phpt +++ b/ext/pdo_pgsql/tests/issue78621.phpt @@ -10,14 +10,16 @@ PDOTest::skip(); --FILE-- pgsqlSetNoticeCallback('disp'); + $db->pgsqlSetNoticeCallback('disp'.$prefix); } require dirname(__FILE__) . '/issue78621.inc'; ?> --EXPECT-- NOTICE: I tampered your data, did you know? +ReNOTICE: I tampered your data, did you know? array(1) { [0]=> array(1) { diff --git a/ext/pdo_pgsql/tests/issue78621_closure.phpt b/ext/pdo_pgsql/tests/issue78621_closure.phpt index c116cde305d95..f041f85f8e066 100644 --- a/ext/pdo_pgsql/tests/issue78621_closure.phpt +++ b/ext/pdo_pgsql/tests/issue78621_closure.phpt @@ -10,9 +10,9 @@ PDOTest::skip(); --FILE-- pgsqlSetNoticeCallback(function($message) { echo trim($message)."\n"; }); + $db->pgsqlSetNoticeCallback(function($message) use($prefix) { echo $prefix.trim($message)."\n"; }); // https://github.com/php/php-src/pull/4823#pullrequestreview-335623806 $eraseCallbackMemoryHere = (object)[1]; } @@ -20,6 +20,7 @@ require dirname(__FILE__) . '/issue78621.inc'; ?> --EXPECT-- NOTICE: I tampered your data, did you know? +ReNOTICE: I tampered your data, did you know? array(1) { [0]=> array(1) { From 1c8851a63772de4b752cbbf2f655c2ce1dd8549d Mon Sep 17 00:00:00 2001 From: Guillaume Outters Date: Fri, 3 Jan 2020 21:09:25 +0100 Subject: [PATCH 5/6] pgsqlSetNoticeCallback: test case for method callback --- ext/pdo_pgsql/tests/issue78621_method.phpt | 35 ++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 ext/pdo_pgsql/tests/issue78621_method.phpt diff --git a/ext/pdo_pgsql/tests/issue78621_method.phpt b/ext/pdo_pgsql/tests/issue78621_method.phpt new file mode 100644 index 0000000000000..749788b667bfd --- /dev/null +++ b/ext/pdo_pgsql/tests/issue78621_method.phpt @@ -0,0 +1,35 @@ +--TEST-- +pgsqlSetNoticeCallback catches Postgres "raise notice". +--SKIPIF-- + +--FILE-- +pgsqlSetNoticeCallback(array($logger, 'disp'.$prefix)); +} +require dirname(__FILE__) . '/issue78621.inc'; +?> +--EXPECT-- +NOTICE: I tampered your data, did you know? +ReNOTICE: I tampered your data, did you know? +array(1) { + [0]=> + array(1) { + ["a"]=> + string(2) "oh" + } +} +Done From c89544eae0b9741cc2c47b1db13ed64a7c8f9492 Mon Sep 17 00:00:00 2001 From: Guillaume Outters Date: Thu, 9 Jan 2020 07:20:36 +0100 Subject: [PATCH 6/6] pgsqlSetNoticeCallback: tests: set client_min_messages --- ext/pdo_pgsql/tests/issue78621.inc | 1 + 1 file changed, 1 insertion(+) diff --git a/ext/pdo_pgsql/tests/issue78621.inc b/ext/pdo_pgsql/tests/issue78621.inc index 42236d803163d..76acd201c1794 100644 --- a/ext/pdo_pgsql/tests/issue78621.inc +++ b/ext/pdo_pgsql/tests/issue78621.inc @@ -6,6 +6,7 @@ $db = PDOTest::test_factory(dirname(__FILE__) . '/common.phpt'); attach($db); $db->beginTransaction(); +$db->exec("set client_min_messages to notice"); $db->exec("create temporary table t (a varchar(3))"); $db->exec("create function hey() returns trigger as \$\$ begin new.a := 'oh'; raise notice 'I tampered your data, did you know?'; return new; end; \$\$ language plpgsql"); $db->exec("create trigger hop before insert on t for each row execute procedure hey()");