public inbox for [email protected]
help / color / mirror / Atom feedfix crash with Python 3.11
8+ messages / 3 participants
[nested] [flat]
* fix crash with Python 3.11
@ 2021-12-22 08:24 Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Peter Eisentraut @ 2021-12-22 08:24 UTC (permalink / raw)
To: pgsql-hackers
This patch needs another close pass and possibly some refactoring to
avoid copy-and-paste, but I'm putting this out here, since people are
already testing with Python 3.11 and will surely run into this problem.
The way plpy.commit() and plpy.rollback() handle errors is not ideal.
They end up just longjmping back to the main loop, without telling the
Python interpreter about it. This hasn't been a problem so far,
apparently, but with Python 3.11-to-be, this appears to cause corruption
in the state of the Python interpreter. This is readily reproducible
and will cause crashes in the plpython_transaction test.
The fix is that we need to catch the PostgreSQL error and turn it into a
Python exception, like we do for other places where plpy.* methods call
into PostgreSQL internals.
From 31bbd62f43ceb0542e0a311dc28704fd702caeb3 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 22 Dec 2021 09:01:24 +0100
Subject: [PATCH v1] Set Python exception after failed commit or rollback
When plpy.commit() or plpy.rollback() failed, the SPI code would
ereport(ERROR) and jump all the way back to the main loop, bypassing
PL/Python. By contrast, in plpy.execute(), we catch the ereport() and
generate a Python exception. This has the advantage that you can
catch the exception and you get better backtraces.
An additional problem is apparently that beginning in Python
3.11-to-be, longjmping out of the Python interpreter leaves its
internal state inconsistent, and subsequent invocations of PL/Python
code will crash. With previous Python versions, this has apparently
not been a problem.
To fix, adapt the logic in PLy_spi_subtransaction_abort() (which is
used by plpy.execute() and others) to catch the PostgreSQL error and
turn it into a Python exception. Move some code around to avoid
additional cross-file zigzag from this.
---
.../expected/plpython_transaction.out | 18 ++-
src/pl/plpython/plpy_plpymodule.c | 30 -----
src/pl/plpython/plpy_spi.c | 106 ++++++++++++++++++
src/pl/plpython/plpy_spi.h | 3 +
4 files changed, 121 insertions(+), 36 deletions(-)
diff --git a/src/pl/plpython/expected/plpython_transaction.out b/src/pl/plpython/expected/plpython_transaction.out
index 14152993c7..9fd9ef500e 100644
--- a/src/pl/plpython/expected/plpython_transaction.out
+++ b/src/pl/plpython/expected/plpython_transaction.out
@@ -55,8 +55,11 @@ for i in range(0, 10):
return 1
$$;
SELECT transaction_test2();
-ERROR: invalid transaction termination
-CONTEXT: PL/Python function "transaction_test2"
+ERROR: spiexceptions.InvalidTransactionTermination: invalid transaction termination
+CONTEXT: Traceback (most recent call last):
+ PL/Python function "transaction_test2", line 5, in <module>
+ plpy.commit()
+PL/Python function "transaction_test2"
SELECT * FROM test1;
a | b
---+---
@@ -70,7 +73,7 @@ plpy.execute("CALL transaction_test1()")
return 1
$$;
SELECT transaction_test3();
-ERROR: spiexceptions.InvalidTransactionTermination: invalid transaction termination
+ERROR: spiexceptions.InvalidTransactionTermination: spiexceptions.InvalidTransactionTermination: invalid transaction termination
CONTEXT: Traceback (most recent call last):
PL/Python function "transaction_test3", line 2, in <module>
plpy.execute("CALL transaction_test1()")
@@ -88,7 +91,7 @@ plpy.execute("DO LANGUAGE plpythonu $x$ plpy.commit() $x$")
return 1
$$;
SELECT transaction_test4();
-ERROR: spiexceptions.InvalidTransactionTermination: invalid transaction termination
+ERROR: spiexceptions.InvalidTransactionTermination: spiexceptions.InvalidTransactionTermination: invalid transaction termination
CONTEXT: Traceback (most recent call last):
PL/Python function "transaction_test4", line 2, in <module>
plpy.execute("DO LANGUAGE plpythonu $x$ plpy.commit() $x$")
@@ -100,8 +103,11 @@ s.enter()
plpy.commit()
$$;
WARNING: forcibly aborting a subtransaction that has not been exited
-ERROR: cannot commit while a subtransaction is active
-CONTEXT: PL/Python anonymous code block
+ERROR: spiexceptions.InvalidTransactionTermination: cannot commit while a subtransaction is active
+CONTEXT: Traceback (most recent call last):
+ PL/Python anonymous code block, line 4, in <module>
+ plpy.commit()
+PL/Python anonymous code block
-- commit inside cursor loop
CREATE TABLE test2 (x int);
INSERT INTO test2 VALUES (0), (1), (2), (3), (4);
diff --git a/src/pl/plpython/plpy_plpymodule.c b/src/pl/plpython/plpy_plpymodule.c
index 0365acc95b..907f89d153 100644
--- a/src/pl/plpython/plpy_plpymodule.c
+++ b/src/pl/plpython/plpy_plpymodule.c
@@ -40,8 +40,6 @@ static PyObject *PLy_fatal(PyObject *self, PyObject *args, PyObject *kw);
static PyObject *PLy_quote_literal(PyObject *self, PyObject *args);
static PyObject *PLy_quote_nullable(PyObject *self, PyObject *args);
static PyObject *PLy_quote_ident(PyObject *self, PyObject *args);
-static PyObject *PLy_commit(PyObject *self, PyObject *args);
-static PyObject *PLy_rollback(PyObject *self, PyObject *args);
/* A list of all known exceptions, generated from backend/utils/errcodes.txt */
@@ -577,31 +575,3 @@ PLy_output(volatile int level, PyObject *self, PyObject *args, PyObject *kw)
*/
Py_RETURN_NONE;
}
-
-static PyObject *
-PLy_commit(PyObject *self, PyObject *args)
-{
- PLyExecutionContext *exec_ctx = PLy_current_execution_context();
-
- SPI_commit();
- SPI_start_transaction();
-
- /* was cleared at transaction end, reset pointer */
- exec_ctx->scratch_ctx = NULL;
-
- Py_RETURN_NONE;
-}
-
-static PyObject *
-PLy_rollback(PyObject *self, PyObject *args)
-{
- PLyExecutionContext *exec_ctx = PLy_current_execution_context();
-
- SPI_rollback();
- SPI_start_transaction();
-
- /* was cleared at transaction end, reset pointer */
- exec_ctx->scratch_ctx = NULL;
-
- Py_RETURN_NONE;
-}
diff --git a/src/pl/plpython/plpy_spi.c b/src/pl/plpython/plpy_spi.c
index 99c1b4f28f..469a9b14ac 100644
--- a/src/pl/plpython/plpy_spi.c
+++ b/src/pl/plpython/plpy_spi.c
@@ -456,6 +456,112 @@ PLy_spi_execute_fetch_result(SPITupleTable *tuptable, uint64 rows, int status)
return (PyObject *) result;
}
+PyObject *
+PLy_commit(PyObject *self, PyObject *args)
+{
+ volatile MemoryContext oldcontext;
+ volatile ResourceOwner oldowner;
+
+ oldcontext = CurrentMemoryContext;
+ oldowner = CurrentResourceOwner;
+
+ PG_TRY();
+ {
+ PLyExecutionContext *exec_ctx = PLy_current_execution_context();
+
+ SPI_commit();
+ SPI_start_transaction();
+
+ /* was cleared at transaction end, reset pointer */
+ exec_ctx->scratch_ctx = NULL;
+ }
+ PG_CATCH();
+ {
+ ErrorData *edata;
+ PLyExceptionEntry *entry;
+ PyObject *exc;
+
+ /* Save error info */
+ MemoryContextSwitchTo(oldcontext);
+ edata = CopyErrorData();
+ FlushErrorState();
+
+ MemoryContextSwitchTo(oldcontext);
+ CurrentResourceOwner = oldowner;
+
+ /* Look up the correct exception */
+ entry = hash_search(PLy_spi_exceptions, &(edata->sqlerrcode),
+ HASH_FIND, NULL);
+
+ /*
+ * This could be a custom error code, if that's the case fallback to
+ * SPIError
+ */
+ exc = entry ? entry->exc : PLy_exc_spi_error;
+ /* Make Python raise the exception */
+ PLy_spi_exception_set(exc, edata);
+ FreeErrorData(edata);
+
+ return NULL;
+ }
+ PG_END_TRY();
+
+ Py_RETURN_NONE;
+}
+
+PyObject *
+PLy_rollback(PyObject *self, PyObject *args)
+{
+ volatile MemoryContext oldcontext;
+ volatile ResourceOwner oldowner;
+
+ oldcontext = CurrentMemoryContext;
+ oldowner = CurrentResourceOwner;
+
+ PG_TRY();
+ {
+ PLyExecutionContext *exec_ctx = PLy_current_execution_context();
+
+ SPI_rollback();
+ SPI_start_transaction();
+
+ /* was cleared at transaction end, reset pointer */
+ exec_ctx->scratch_ctx = NULL;
+ }
+ PG_CATCH();
+ {
+ ErrorData *edata;
+ PLyExceptionEntry *entry;
+ PyObject *exc;
+
+ /* Save error info */
+ MemoryContextSwitchTo(oldcontext);
+ edata = CopyErrorData();
+ FlushErrorState();
+
+ MemoryContextSwitchTo(oldcontext);
+ CurrentResourceOwner = oldowner;
+
+ /* Look up the correct exception */
+ entry = hash_search(PLy_spi_exceptions, &(edata->sqlerrcode),
+ HASH_FIND, NULL);
+
+ /*
+ * This could be a custom error code, if that's the case fallback to
+ * SPIError
+ */
+ exc = entry ? entry->exc : PLy_exc_spi_error;
+ /* Make Python raise the exception */
+ PLy_spi_exception_set(exc, edata);
+ FreeErrorData(edata);
+
+ return NULL;
+ }
+ PG_END_TRY();
+
+ Py_RETURN_NONE;
+}
+
/*
* Utilities for running SPI functions in subtransactions.
*
diff --git a/src/pl/plpython/plpy_spi.h b/src/pl/plpython/plpy_spi.h
index a5e2e60da7..98ccd21093 100644
--- a/src/pl/plpython/plpy_spi.h
+++ b/src/pl/plpython/plpy_spi.h
@@ -12,6 +12,9 @@ extern PyObject *PLy_spi_prepare(PyObject *self, PyObject *args);
extern PyObject *PLy_spi_execute(PyObject *self, PyObject *args);
extern PyObject *PLy_spi_execute_plan(PyObject *ob, PyObject *list, long limit);
+extern PyObject *PLy_commit(PyObject *self, PyObject *args);
+extern PyObject *PLy_rollback(PyObject *self, PyObject *args);
+
typedef struct PLyExceptionEntry
{
int sqlstate; /* hash key, must be first */
--
2.34.1
Attachments:
[text/plain] v1-0001-Set-Python-exception-after-failed-commit-or-rollb.patch (8.2K, ../../[email protected]/2-v1-0001-Set-Python-exception-after-failed-commit-or-rollb.patch)
download | inline diff:
From 31bbd62f43ceb0542e0a311dc28704fd702caeb3 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 22 Dec 2021 09:01:24 +0100
Subject: [PATCH v1] Set Python exception after failed commit or rollback
When plpy.commit() or plpy.rollback() failed, the SPI code would
ereport(ERROR) and jump all the way back to the main loop, bypassing
PL/Python. By contrast, in plpy.execute(), we catch the ereport() and
generate a Python exception. This has the advantage that you can
catch the exception and you get better backtraces.
An additional problem is apparently that beginning in Python
3.11-to-be, longjmping out of the Python interpreter leaves its
internal state inconsistent, and subsequent invocations of PL/Python
code will crash. With previous Python versions, this has apparently
not been a problem.
To fix, adapt the logic in PLy_spi_subtransaction_abort() (which is
used by plpy.execute() and others) to catch the PostgreSQL error and
turn it into a Python exception. Move some code around to avoid
additional cross-file zigzag from this.
---
.../expected/plpython_transaction.out | 18 ++-
src/pl/plpython/plpy_plpymodule.c | 30 -----
src/pl/plpython/plpy_spi.c | 106 ++++++++++++++++++
src/pl/plpython/plpy_spi.h | 3 +
4 files changed, 121 insertions(+), 36 deletions(-)
diff --git a/src/pl/plpython/expected/plpython_transaction.out b/src/pl/plpython/expected/plpython_transaction.out
index 14152993c7..9fd9ef500e 100644
--- a/src/pl/plpython/expected/plpython_transaction.out
+++ b/src/pl/plpython/expected/plpython_transaction.out
@@ -55,8 +55,11 @@ for i in range(0, 10):
return 1
$$;
SELECT transaction_test2();
-ERROR: invalid transaction termination
-CONTEXT: PL/Python function "transaction_test2"
+ERROR: spiexceptions.InvalidTransactionTermination: invalid transaction termination
+CONTEXT: Traceback (most recent call last):
+ PL/Python function "transaction_test2", line 5, in <module>
+ plpy.commit()
+PL/Python function "transaction_test2"
SELECT * FROM test1;
a | b
---+---
@@ -70,7 +73,7 @@ plpy.execute("CALL transaction_test1()")
return 1
$$;
SELECT transaction_test3();
-ERROR: spiexceptions.InvalidTransactionTermination: invalid transaction termination
+ERROR: spiexceptions.InvalidTransactionTermination: spiexceptions.InvalidTransactionTermination: invalid transaction termination
CONTEXT: Traceback (most recent call last):
PL/Python function "transaction_test3", line 2, in <module>
plpy.execute("CALL transaction_test1()")
@@ -88,7 +91,7 @@ plpy.execute("DO LANGUAGE plpythonu $x$ plpy.commit() $x$")
return 1
$$;
SELECT transaction_test4();
-ERROR: spiexceptions.InvalidTransactionTermination: invalid transaction termination
+ERROR: spiexceptions.InvalidTransactionTermination: spiexceptions.InvalidTransactionTermination: invalid transaction termination
CONTEXT: Traceback (most recent call last):
PL/Python function "transaction_test4", line 2, in <module>
plpy.execute("DO LANGUAGE plpythonu $x$ plpy.commit() $x$")
@@ -100,8 +103,11 @@ s.enter()
plpy.commit()
$$;
WARNING: forcibly aborting a subtransaction that has not been exited
-ERROR: cannot commit while a subtransaction is active
-CONTEXT: PL/Python anonymous code block
+ERROR: spiexceptions.InvalidTransactionTermination: cannot commit while a subtransaction is active
+CONTEXT: Traceback (most recent call last):
+ PL/Python anonymous code block, line 4, in <module>
+ plpy.commit()
+PL/Python anonymous code block
-- commit inside cursor loop
CREATE TABLE test2 (x int);
INSERT INTO test2 VALUES (0), (1), (2), (3), (4);
diff --git a/src/pl/plpython/plpy_plpymodule.c b/src/pl/plpython/plpy_plpymodule.c
index 0365acc95b..907f89d153 100644
--- a/src/pl/plpython/plpy_plpymodule.c
+++ b/src/pl/plpython/plpy_plpymodule.c
@@ -40,8 +40,6 @@ static PyObject *PLy_fatal(PyObject *self, PyObject *args, PyObject *kw);
static PyObject *PLy_quote_literal(PyObject *self, PyObject *args);
static PyObject *PLy_quote_nullable(PyObject *self, PyObject *args);
static PyObject *PLy_quote_ident(PyObject *self, PyObject *args);
-static PyObject *PLy_commit(PyObject *self, PyObject *args);
-static PyObject *PLy_rollback(PyObject *self, PyObject *args);
/* A list of all known exceptions, generated from backend/utils/errcodes.txt */
@@ -577,31 +575,3 @@ PLy_output(volatile int level, PyObject *self, PyObject *args, PyObject *kw)
*/
Py_RETURN_NONE;
}
-
-static PyObject *
-PLy_commit(PyObject *self, PyObject *args)
-{
- PLyExecutionContext *exec_ctx = PLy_current_execution_context();
-
- SPI_commit();
- SPI_start_transaction();
-
- /* was cleared at transaction end, reset pointer */
- exec_ctx->scratch_ctx = NULL;
-
- Py_RETURN_NONE;
-}
-
-static PyObject *
-PLy_rollback(PyObject *self, PyObject *args)
-{
- PLyExecutionContext *exec_ctx = PLy_current_execution_context();
-
- SPI_rollback();
- SPI_start_transaction();
-
- /* was cleared at transaction end, reset pointer */
- exec_ctx->scratch_ctx = NULL;
-
- Py_RETURN_NONE;
-}
diff --git a/src/pl/plpython/plpy_spi.c b/src/pl/plpython/plpy_spi.c
index 99c1b4f28f..469a9b14ac 100644
--- a/src/pl/plpython/plpy_spi.c
+++ b/src/pl/plpython/plpy_spi.c
@@ -456,6 +456,112 @@ PLy_spi_execute_fetch_result(SPITupleTable *tuptable, uint64 rows, int status)
return (PyObject *) result;
}
+PyObject *
+PLy_commit(PyObject *self, PyObject *args)
+{
+ volatile MemoryContext oldcontext;
+ volatile ResourceOwner oldowner;
+
+ oldcontext = CurrentMemoryContext;
+ oldowner = CurrentResourceOwner;
+
+ PG_TRY();
+ {
+ PLyExecutionContext *exec_ctx = PLy_current_execution_context();
+
+ SPI_commit();
+ SPI_start_transaction();
+
+ /* was cleared at transaction end, reset pointer */
+ exec_ctx->scratch_ctx = NULL;
+ }
+ PG_CATCH();
+ {
+ ErrorData *edata;
+ PLyExceptionEntry *entry;
+ PyObject *exc;
+
+ /* Save error info */
+ MemoryContextSwitchTo(oldcontext);
+ edata = CopyErrorData();
+ FlushErrorState();
+
+ MemoryContextSwitchTo(oldcontext);
+ CurrentResourceOwner = oldowner;
+
+ /* Look up the correct exception */
+ entry = hash_search(PLy_spi_exceptions, &(edata->sqlerrcode),
+ HASH_FIND, NULL);
+
+ /*
+ * This could be a custom error code, if that's the case fallback to
+ * SPIError
+ */
+ exc = entry ? entry->exc : PLy_exc_spi_error;
+ /* Make Python raise the exception */
+ PLy_spi_exception_set(exc, edata);
+ FreeErrorData(edata);
+
+ return NULL;
+ }
+ PG_END_TRY();
+
+ Py_RETURN_NONE;
+}
+
+PyObject *
+PLy_rollback(PyObject *self, PyObject *args)
+{
+ volatile MemoryContext oldcontext;
+ volatile ResourceOwner oldowner;
+
+ oldcontext = CurrentMemoryContext;
+ oldowner = CurrentResourceOwner;
+
+ PG_TRY();
+ {
+ PLyExecutionContext *exec_ctx = PLy_current_execution_context();
+
+ SPI_rollback();
+ SPI_start_transaction();
+
+ /* was cleared at transaction end, reset pointer */
+ exec_ctx->scratch_ctx = NULL;
+ }
+ PG_CATCH();
+ {
+ ErrorData *edata;
+ PLyExceptionEntry *entry;
+ PyObject *exc;
+
+ /* Save error info */
+ MemoryContextSwitchTo(oldcontext);
+ edata = CopyErrorData();
+ FlushErrorState();
+
+ MemoryContextSwitchTo(oldcontext);
+ CurrentResourceOwner = oldowner;
+
+ /* Look up the correct exception */
+ entry = hash_search(PLy_spi_exceptions, &(edata->sqlerrcode),
+ HASH_FIND, NULL);
+
+ /*
+ * This could be a custom error code, if that's the case fallback to
+ * SPIError
+ */
+ exc = entry ? entry->exc : PLy_exc_spi_error;
+ /* Make Python raise the exception */
+ PLy_spi_exception_set(exc, edata);
+ FreeErrorData(edata);
+
+ return NULL;
+ }
+ PG_END_TRY();
+
+ Py_RETURN_NONE;
+}
+
/*
* Utilities for running SPI functions in subtransactions.
*
diff --git a/src/pl/plpython/plpy_spi.h b/src/pl/plpython/plpy_spi.h
index a5e2e60da7..98ccd21093 100644
--- a/src/pl/plpython/plpy_spi.h
+++ b/src/pl/plpython/plpy_spi.h
@@ -12,6 +12,9 @@ extern PyObject *PLy_spi_prepare(PyObject *self, PyObject *args);
extern PyObject *PLy_spi_execute(PyObject *self, PyObject *args);
extern PyObject *PLy_spi_execute_plan(PyObject *ob, PyObject *list, long limit);
+extern PyObject *PLy_commit(PyObject *self, PyObject *args);
+extern PyObject *PLy_rollback(PyObject *self, PyObject *args);
+
typedef struct PLyExceptionEntry
{
int sqlstate; /* hash key, must be first */
--
2.34.1
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: fix crash with Python 3.11
@ 2022-06-21 16:33 Tom Lane <[email protected]>
parent: Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Tom Lane @ 2022-06-21 16:33 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: [email protected]; [email protected]
Is it time yet to back-patch 2e517818f ("Fix SPI's handling of errors
during transaction commit")? We know we're going to have to do it
before Python 3.11 ships, and it's been stable in HEAD for 3.5 months
now. Also, the Fedora guys absorbed the patch a couple weeks ago [1]
because they're already using 3.11 in rawhide, and I've not heard
complaints from that direction.
My inclination at this point is to not back-patch the second change
12d768e70 ("Don't use static storage for SaveTransactionCharacteristics").
It's not clear that the benefit would be worth even a small risk of
somebody being unhappy about the API break.
regards, tom lane
[1] https://www.postgresql.org/message-id/flat/CA%2BHKMWMY_e2otmTJDjKUAvC8Urh4rzSWOPZ%3DfszU5brkBP97ng%4...
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: fix crash with Python 3.11
@ 2022-06-23 07:41 Markus Wanner <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 2 replies; 8+ messages in thread
From: Markus Wanner @ 2022-06-23 07:41 UTC (permalink / raw)
To: Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; +Cc: [email protected]; [email protected]
On 6/21/22 18:33, Tom Lane wrote:
> My inclination at this point is to not back-patch the second change
> 12d768e70 ("Don't use static storage for SaveTransactionCharacteristics").
> It's not clear that the benefit would be worth even a small risk of
> somebody being unhappy about the API break.
Actually, the backport of 2e517818f ("Fix SPI's handling of errors")
already broke the API for code using SPICleanup, as that function had
been removed. Granted, it's not documented, but still exported.
I propose to re-introduce a no-op placeholder similar to what we have
for SPI_start_transaction, somewhat like the attached patch.
Regards
Markus
Attachments:
[text/x-patch] add-spicleanup-placeholder.diff (1.0K, ../../[email protected]/2-add-spicleanup-placeholder.diff)
download | inline diff:
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index dd5ef762707..f73c1e79e18 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -422,6 +422,16 @@ SPI_rollback_and_chain(void)
_SPI_rollback(true);
}
+/*
+ * SPICleanup is a no-op, kept for backwards compatibility. We rely on
+ * AtEOXact_SPI to cleanup. Extensions should not (need to) fiddle with the
+ * internal SPI state directly.
+ */
+void
+SPICleanup(void)
+{
+}
+
/*
* Clean up SPI state at transaction commit or abort.
*/
diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h
index 153eb5c7ad5..1e66a7d2ea0 100644
--- a/src/include/executor/spi.h
+++ b/src/include/executor/spi.h
@@ -205,6 +205,7 @@ extern void SPI_commit_and_chain(void);
extern void SPI_rollback(void);
extern void SPI_rollback_and_chain(void);
+extern void SPICleanup(void);
extern void AtEOXact_SPI(bool isCommit);
extern void AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid);
extern bool SPI_inside_nonatomic_context(void);
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: fix crash with Python 3.11
@ 2022-06-23 13:34 Tom Lane <[email protected]>
parent: Markus Wanner <[email protected]>
1 sibling, 1 reply; 8+ messages in thread
From: Tom Lane @ 2022-06-23 13:34 UTC (permalink / raw)
To: Markus Wanner <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; [email protected]; [email protected]
Markus Wanner <[email protected]> writes:
> Actually, the backport of 2e517818f ("Fix SPI's handling of errors")
> already broke the API for code using SPICleanup, as that function had
> been removed. Granted, it's not documented, but still exported.
Under what circumstances would it be OK for outside code to call
SPICleanup?
regards, tom lane
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: fix crash with Python 3.11
@ 2022-06-23 19:57 Markus Wanner <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Markus Wanner @ 2022-06-23 19:57 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; [email protected]; [email protected]
On 6/23/22 15:34, Tom Lane wrote:
> Under what circumstances would it be OK for outside code to call
> SPICleanup?
For the same reasons previous Postgres versions called SPICleanup: from
a sigsetjmp handler that duplicates most of what Postgres does in such a
situation.
However, I think that's the wrong question to ask for a stable branch.
Postgres did export this function in previous versions. Removing it
altogether constitutes an API change and makes extensions that link to
it fail to even load, which is a bad way to fail after a patch version
upgrade. Even if its original use was not sound in the first place.
Ofc my proposed patch is not meant for master, only for stable branches.
Best Regards
Markus
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: fix crash with Python 3.11
@ 2022-06-23 22:54 Tom Lane <[email protected]>
parent: Markus Wanner <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Tom Lane @ 2022-06-23 22:54 UTC (permalink / raw)
To: Markus Wanner <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; [email protected]; [email protected]
Markus Wanner <[email protected]> writes:
> On 6/23/22 15:34, Tom Lane wrote:
>> Under what circumstances would it be OK for outside code to call
>> SPICleanup?
> For the same reasons previous Postgres versions called SPICleanup: from
> a sigsetjmp handler that duplicates most of what Postgres does in such a
> situation.
Does such code exist? I don't see any other calls in Debian code search,
and I find it hard to believe that anyone would think such a thing is
maintainable.
regards, tom lane
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: fix crash with Python 3.11
@ 2022-06-24 12:04 Markus Wanner <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Markus Wanner @ 2022-06-24 12:04 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; [email protected]; [email protected]
On 6/24/22 00:54, Tom Lane wrote:
> Does such code exist? I don't see any other calls in Debian code search,
> and I find it hard to believe that anyone would think such a thing is
> maintainable.
Such a thing does exist within PGLogical and BDR, yes.
Thanks for your concern about maintainability. So far, that part was not
posing any trouble. Looking at e.g. postgres.c, the sigsetjmp handler
there didn't change all that much in recent years. Much of the code
there is from around 2004 written by you.
However, that shouldn't be your concern at all. Postgres refusing to
start after a minor upgrade probably should, especially when it's due to
an API change in a stable branch.
Regards
Markus
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: fix crash with Python 3.11
@ 2022-07-18 19:09 Peter Eisentraut <[email protected]>
parent: Markus Wanner <[email protected]>
1 sibling, 0 replies; 8+ messages in thread
From: Peter Eisentraut @ 2022-07-18 19:09 UTC (permalink / raw)
To: Markus Wanner <[email protected]>; Tom Lane <[email protected]>; +Cc: [email protected]; [email protected]
On 23.06.22 09:41, Markus Wanner wrote:
>
> On 6/21/22 18:33, Tom Lane wrote:
>> My inclination at this point is to not back-patch the second change
>> 12d768e70 ("Don't use static storage for
>> SaveTransactionCharacteristics").
>> It's not clear that the benefit would be worth even a small risk of
>> somebody being unhappy about the API break.
>
> Actually, the backport of 2e517818f ("Fix SPI's handling of errors")
> already broke the API for code using SPICleanup, as that function had
> been removed. Granted, it's not documented, but still exported.
>
> I propose to re-introduce a no-op placeholder similar to what we have
> for SPI_start_transaction, somewhat like the attached patch.
I have applied your patch to branches 11 through 14.
^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2022-07-18 19:09 UTC | newest]
Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-12-22 08:24 fix crash with Python 3.11 Peter Eisentraut <[email protected]>
2022-06-21 16:33 ` Tom Lane <[email protected]>
2022-06-23 07:41 ` Markus Wanner <[email protected]>
2022-06-23 13:34 ` Tom Lane <[email protected]>
2022-06-23 19:57 ` Markus Wanner <[email protected]>
2022-06-23 22:54 ` Tom Lane <[email protected]>
2022-06-24 12:04 ` Markus Wanner <[email protected]>
2022-07-18 19:09 ` Peter Eisentraut <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox