diff --git a/CHANGELOG b/CHANGELOG index 93ec2de..1c273a9 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -11,6 +11,8 @@ done so far: - Add support for bulk_insert FDWs on PG14+ (https://github.com/pgsql-io/multicorn2/pull/45) - PG16: Fix compatibility issues w/ log_to_postgres and join query planning in PostgreSQL 16 (https://github.com/pgsql-io/multicorn2/pull/51) - Fix crashes in EXPLAIN with complex quals (https://github.com/pgsql-io/multicorn2/pull/54) + - Support Python 3.11 (https://github.com/pgsql-io/multicorn2/pull/59) + - Behavior change: When log_to_postgres with level ERROR or FATAL is invoked, a specialized Python exception will be thrown and the stack unwound, allowing `catch` and `finally` blocks, and other things like context handler exits, to be invoked in the FDW. (https://github.com/pgsql-io/multicorn2/pull/59) to do: - confirm support for Python 3.11 & 3.12 diff --git a/README.md b/README.md index 69cfa95..aefb2c1 100755 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ Multicorn2 ========== -Multicorn Python3 Foreign Data Wrapper (FDW) for Postgresql. Tested on Linux w/ Python 3.9-3.12 & Postgres 12-17. +Multicorn Python3 Foreign Data Wrapper (FDW) for Postgresql. Tested on Linux w/ Python 3.9-3.11 & Postgres 12-17. The Multicorn Foreign Data Wrapper allows you to fetch foreign data in Python in your PostgreSQL server. @@ -151,7 +151,7 @@ nix build .#testSuites.test_pg12_py39 ``` **Known issues:** -- The tests cover only the supported range of Python & PostgreSQL combinations; in particular, Python releases 3.11 and later are disabled due to failures that have not been addressed. +- The tests cover only the supported range of Python & PostgreSQL combinations; in particular, Python releases 3.12 and later are disabled due to failures that have not been addressed. ### Adding new Python or PostgreSQL versions to the test suite diff --git a/flake.nix b/flake.nix index 88ce4e1..6578702 100644 --- a/flake.nix +++ b/flake.nix @@ -24,7 +24,7 @@ testPythonVersions = with pkgs; [ python39 python310 - # python311 # tests are currently broken + python311 # python312 # tests are currently broken # python313 # tests are currently broken ]; @@ -127,6 +127,7 @@ ./Makefile ./test-3.9 ./test-3.10 + ./test-3.11 ./test-common ]; unpackPhase = '' diff --git a/python/multicorn/utils.py b/python/multicorn/utils.py index b6f2749..79365ab 100644 --- a/python/multicorn/utils.py +++ b/python/multicorn/utils.py @@ -20,9 +20,25 @@ def _log_to_postgres(message, level=0, hint=None, detail=None): } +class MulticornException(Exception): + def __init__(self, message, code, hint, detail): + self._is_multicorn_exception = True + self.message = message + self.code = code + self.hint = hint + self.detail = detail + + def log_to_postgres(message, level=INFO, hint=None, detail=None): code = REPORT_CODES.get(level, None) if code is None: raise KeyError("Not a valid log level") - _log_to_postgres(message, code, hint=hint, detail=detail) - + if level in (ERROR, CRITICAL): + # if we sent an ERROR or FATAL(=CRITICAL) message to _log_to_postgres, we would trigger the PostgreSQL C-level + # exception handling, which would prevent us from cleanly exiting whatever Python context we're currently in. + # To avoid this, these log levels are replaced with exceptions which are bubbled back to Multicorn's entry + # points, and those exceptions are translated into appropriate logging after we exit the method at the top of + # the multicorn stack. + raise MulticornException(message, code, hint, detail) + else: + _log_to_postgres(message, code, hint=hint, detail=detail) diff --git a/src/errors.c b/src/errors.c index 12ec64a..86f089b 100644 --- a/src/errors.c +++ b/src/errors.c @@ -17,6 +17,7 @@ void reportException(PyObject *pErrType, PyObject *pErrValue, PyObject *pErrTraceback); +void reportMulticornException(PyObject *pErrValue); PGDLLEXPORT void errorCheck() @@ -28,7 +29,22 @@ errorCheck() PyErr_Fetch(&pErrType, &pErrValue, &pErrTraceback); if (pErrType) { - reportException(pErrType, pErrValue, pErrTraceback); + // if the error value has a property _is_multicorn_exception and a boolean value True, then we don't report the + // error as a generic exception with a stack trace -- instead we just take the message, code(severity), hint, + // and detail, and log it to Postgres. These exceptions are generated in utils.py to intercept ERROR/FATAL log + // messages. So, first detect whether that's the case, and call a new reporting function... + PyObject *is_multicorn_exception = PyObject_GetAttrString(pErrValue, "_is_multicorn_exception"); + if (is_multicorn_exception != NULL && PyObject_IsTrue(is_multicorn_exception)) + { + Py_DECREF(is_multicorn_exception); + Py_DECREF(pErrType); + Py_DECREF(pErrTraceback); + reportMulticornException(pErrValue); + } + else + { + reportException(pErrType, pErrValue, pErrTraceback); + } } } @@ -92,3 +108,64 @@ reportException(PyObject *pErrType, PyObject *pErrValue, PyObject *pErrTraceback errfinish(0); #endif } + +void reportMulticornException(PyObject* pErrValue) +{ + int severity; + PyObject *message = PyObject_GetAttrString(pErrValue, "message"); + PyObject *hint = PyObject_GetAttrString(pErrValue, "hint"); + PyObject *detail = PyObject_GetAttrString(pErrValue, "detail"); + PyObject *code = PyObject_GetAttrString(pErrValue, "code"); + int level = PyLong_AsLong(code); + + // Matches up with REPORT_CODES in utils.py + switch (level) + { + case 3: + severity = ERROR; + break; + default: + case 4: + severity = FATAL; + break; + } + + PG_TRY(); + { + + #if PG_VERSION_NUM >= 130000 + if (errstart(severity, TEXTDOMAIN)) + #else + if (errstart(severity, __FILE__, __LINE__, PG_FUNCNAME_MACRO, TEXTDOMAIN)) + #endif + { + errmsg("%s", PyString_AsString(message)); + if (hint != NULL && hint != Py_None) + { + char* hintstr = PyString_AsString(hint); + errhint("%s", hintstr); + } + if (detail != NULL && detail != Py_None) + { + char* detailstr = PyString_AsString(detail); + errdetail("%s", detailstr); + } + #if PG_VERSION_NUM >= 130000 + errfinish(__FILE__, __LINE__, PG_FUNCNAME_MACRO); + #else + errfinish(0); + #endif + } + + } + PG_CATCH(); + { + Py_DECREF(message); + Py_DECREF(hint); + Py_DECREF(detail); + Py_DECREF(code); + Py_DECREF(pErrValue); + PG_RE_THROW(); + } + PG_END_TRY(); +} diff --git a/src/python.c b/src/python.c index b421fd5..f230afc 100644 --- a/src/python.c +++ b/src/python.c @@ -667,7 +667,6 @@ getCacheEntry(Oid foreigntableid) MemoryContextDelete(tempContext); } RelationClose(rel); - Py_INCREF(entry->value); /* * Start a new transaction or subtransaction if needed. @@ -685,7 +684,9 @@ getCacheEntry(Oid foreigntableid) PyObject * getInstance(Oid foreigntableid) { - return getCacheEntry(foreigntableid)->value; + PyObject* retval = getCacheEntry(foreigntableid)->value; + Py_INCREF(retval); + return retval; } diff --git a/test-3.11 b/test-3.11 new file mode 120000 index 0000000..e814a5a --- /dev/null +++ b/test-3.11 @@ -0,0 +1 @@ +test-3.9 \ No newline at end of file diff --git a/test-3.9/expected/write_test_3.out b/test-3.9/expected/write_test_3.out new file mode 100644 index 0000000..47687c3 --- /dev/null +++ b/test-3.9/expected/write_test_3.out @@ -0,0 +1,190 @@ +CREATE EXTENSION multicorn; +CREATE server multicorn_srv foreign data wrapper multicorn options ( + wrapper 'multicorn.testfdw.TestForeignDataWrapper' +); +CREATE user mapping FOR current_user server multicorn_srv options (usermapping 'test'); +CREATE foreign table testmulticorn ( + test1 character varying, + test2 character varying +) server multicorn_srv options ( + option1 'option1', + test_type 'nowrite', + tx_hook 'true' +); +insert into testmulticorn(test1, test2) VALUES ('test', 'test2'); +NOTICE: [('option1', 'option1'), ('test_type', 'nowrite'), ('tx_hook', 'true'), ('usermapping', 'test')] +NOTICE: [('test1', 'character varying'), ('test2', 'character varying')] +NOTICE: BEGIN +ERROR: Error in python: NotImplementedError +DETAIL: This FDW does not support the writable API +NOTICE: ROLLBACK +update testmulticorn set test1 = 'test'; +NOTICE: BEGIN +NOTICE: [] +NOTICE: ['test1', 'test2'] +NOTICE: ROLLBACK +ERROR: Error in python: NotImplementedError +DETAIL: This FDW does not support the writable API +delete from testmulticorn where test2 = 'test2 2 0'; +NOTICE: BEGIN +NOTICE: [test2 = test2 2 0] +NOTICE: ['test1', 'test2'] +NOTICE: ROLLBACK +ERROR: Error in python: NotImplementedError +DETAIL: This FDW does not support the writable API +CREATE foreign table testmulticorn_write ( + test1 character varying, + test2 character varying +) server multicorn_srv options ( + option1 'option1', + row_id_column 'test1', + test_type 'returning', + tx_hook 'true' +); +insert into testmulticorn_write(test1, test2) VALUES ('test', 'test2'); +NOTICE: [('option1', 'option1'), ('row_id_column', 'test1'), ('test_type', 'returning'), ('tx_hook', 'true'), ('usermapping', 'test')] +NOTICE: [('test1', 'character varying'), ('test2', 'character varying')] +NOTICE: BEGIN +NOTICE: INSERTING: [('test1', 'test'), ('test2', 'test2')] +NOTICE: PRECOMMIT +NOTICE: COMMIT +update testmulticorn_write set test1 = 'test' where test1 ilike 'test1 3%'; +NOTICE: BEGIN +NOTICE: [test1 ~~* test1 3%] +NOTICE: ['test1', 'test2'] +NOTICE: UPDATING: test1 3 1 with [('test1', 'test'), ('test2', 'test2 1 1')] +NOTICE: UPDATING: test1 3 4 with [('test1', 'test'), ('test2', 'test2 1 4')] +NOTICE: UPDATING: test1 3 7 with [('test1', 'test'), ('test2', 'test2 1 7')] +NOTICE: UPDATING: test1 3 10 with [('test1', 'test'), ('test2', 'test2 1 10')] +NOTICE: UPDATING: test1 3 13 with [('test1', 'test'), ('test2', 'test2 1 13')] +NOTICE: UPDATING: test1 3 16 with [('test1', 'test'), ('test2', 'test2 1 16')] +NOTICE: UPDATING: test1 3 19 with [('test1', 'test'), ('test2', 'test2 1 19')] +NOTICE: PRECOMMIT +NOTICE: COMMIT +delete from testmulticorn_write where test2 = 'test2 2 0'; +NOTICE: BEGIN +NOTICE: [test2 = test2 2 0] +NOTICE: ['test1', 'test2'] +NOTICE: DELETING: test1 1 0 +NOTICE: PRECOMMIT +NOTICE: COMMIT +-- Test returning +insert into testmulticorn_write(test1, test2) VALUES ('test', 'test2') RETURNING test1; +NOTICE: BEGIN +NOTICE: INSERTING: [('test1', 'test'), ('test2', 'test2')] +NOTICE: PRECOMMIT +NOTICE: COMMIT + test1 +---------------- + INSERTED: test +(1 row) + +update testmulticorn_write set test1 = 'test' where test1 ilike 'test1 3%' RETURNING test1; +NOTICE: BEGIN +NOTICE: [test1 ~~* test1 3%] +NOTICE: ['test1', 'test2'] +NOTICE: UPDATING: test1 3 1 with [('test1', 'test'), ('test2', 'test2 1 1')] +NOTICE: UPDATING: test1 3 4 with [('test1', 'test'), ('test2', 'test2 1 4')] +NOTICE: UPDATING: test1 3 7 with [('test1', 'test'), ('test2', 'test2 1 7')] +NOTICE: UPDATING: test1 3 10 with [('test1', 'test'), ('test2', 'test2 1 10')] +NOTICE: UPDATING: test1 3 13 with [('test1', 'test'), ('test2', 'test2 1 13')] +NOTICE: UPDATING: test1 3 16 with [('test1', 'test'), ('test2', 'test2 1 16')] +NOTICE: UPDATING: test1 3 19 with [('test1', 'test'), ('test2', 'test2 1 19')] +NOTICE: PRECOMMIT +NOTICE: COMMIT + test1 +--------------- + UPDATED: test + UPDATED: test + UPDATED: test + UPDATED: test + UPDATED: test + UPDATED: test + UPDATED: test +(7 rows) + +delete from testmulticorn_write where test1 = 'test1 1 0' returning test2, test1; +NOTICE: BEGIN +NOTICE: [test1 = test1 1 0] +NOTICE: ['test1', 'test2'] +NOTICE: DELETING: test1 1 0 +NOTICE: PRECOMMIT +NOTICE: COMMIT + test2 | test1 +-----------+----------- + test2 2 0 | test1 1 0 +(1 row) + +DROP foreign table testmulticorn_write; +-- Now test with another column +CREATE foreign table testmulticorn_write( + test1 character varying, + test2 character varying +) server multicorn_srv options ( + option1 'option1', + row_id_column 'test2' +); +insert into testmulticorn_write(test1, test2) VALUES ('test', 'test2'); +NOTICE: [('option1', 'option1'), ('row_id_column', 'test2'), ('usermapping', 'test')] +NOTICE: [('test1', 'character varying'), ('test2', 'character varying')] +NOTICE: INSERTING: [('test1', 'test'), ('test2', 'test2')] +update testmulticorn_write set test1 = 'test' where test1 ilike 'test1 3%'; +NOTICE: [test1 ~~* test1 3%] +NOTICE: ['test1', 'test2'] +NOTICE: UPDATING: test2 1 1 with [('test1', 'test'), ('test2', 'test2 1 1')] +NOTICE: UPDATING: test2 1 4 with [('test1', 'test'), ('test2', 'test2 1 4')] +NOTICE: UPDATING: test2 1 7 with [('test1', 'test'), ('test2', 'test2 1 7')] +NOTICE: UPDATING: test2 1 10 with [('test1', 'test'), ('test2', 'test2 1 10')] +NOTICE: UPDATING: test2 1 13 with [('test1', 'test'), ('test2', 'test2 1 13')] +NOTICE: UPDATING: test2 1 16 with [('test1', 'test'), ('test2', 'test2 1 16')] +NOTICE: UPDATING: test2 1 19 with [('test1', 'test'), ('test2', 'test2 1 19')] +delete from testmulticorn_write where test2 = 'test2 2 0'; +NOTICE: [test2 = test2 2 0] +NOTICE: ['test2'] +NOTICE: DELETING: test2 2 0 +update testmulticorn_write set test2 = 'test' where test2 = 'test2 1 1'; +NOTICE: [test2 = test2 1 1] +NOTICE: ['test1', 'test2'] +NOTICE: UPDATING: test2 1 1 with [('test1', 'test1 3 1'), ('test2', 'test')] +DROP foreign table testmulticorn_write; +-- Now test with other types +CREATE foreign table testmulticorn_write( + test1 date, + test2 timestamp +) server multicorn_srv options ( + option1 'option1', + row_id_column 'test2', + test_type 'date' +); +insert into testmulticorn_write(test1, test2) VALUES ('2012-01-01', '2012-01-01 00:00:00'); +NOTICE: [('option1', 'option1'), ('row_id_column', 'test2'), ('test_type', 'date'), ('usermapping', 'test')] +NOTICE: [('test1', 'date'), ('test2', 'timestamp without time zone')] +NOTICE: INSERTING: [('test1', datetime.date(2012, 1, 1)), ('test2', datetime.datetime(2012, 1, 1, 0, 0))] +delete from testmulticorn_write where test2 > '2011-12-03'; +NOTICE: [test2 > 2011-12-03 00:00:00] +NOTICE: ['test2'] +NOTICE: DELETING: 2011-12-03 14:30:25 +update testmulticorn_write set test1 = date_trunc('day', test1) where test2 = '2011-09-03 14:30:25'; +NOTICE: [test2 = 2011-09-03 14:30:25] +NOTICE: ['test1', 'test2'] +NOTICE: UPDATING: 2011-09-03 14:30:25 with [('test1', datetime.date(2011, 9, 2)), ('test2', datetime.datetime(2011, 9, 3, 14, 30, 25))] +DROP foreign table testmulticorn_write; +-- Test with unknown column +CREATE foreign table testmulticorn_write( + test1 date, + test2 timestamp +) server multicorn_srv options ( + option1 'option1', + row_id_column 'teststuff', + test_type 'date' +); +delete from testmulticorn_write; +NOTICE: [('option1', 'option1'), ('row_id_column', 'teststuff'), ('test_type', 'date'), ('usermapping', 'test')] +NOTICE: [('test1', 'date'), ('test2', 'timestamp without time zone')] +ERROR: The rowid attribute does not exist +DROP USER MAPPING FOR current_user SERVER multicorn_srv; +DROP EXTENSION multicorn cascade; +NOTICE: drop cascades to 3 other objects +DETAIL: drop cascades to server multicorn_srv +drop cascades to foreign table testmulticorn +drop cascades to foreign table testmulticorn_write