Subject: Re: SPI_connect, SPI_connect_ext return type

From: Tom Lane <[email-masked]>
Date: Sat, 10 Aug 2024 10:12:04 -0400

Stepan <[email-masked]> writes:
> Hi, hackers! If you look at the code in the src/backend/executor/spi.c file,
> you will see the SPI_connect function familiar to many there, which
> internally simply calls SPI_connect_ext. The return type is int, at the end
> it says return SPI_OK_CONNECT;
> It confuses me that nothing but OK, judging by the code, can return.(I
> understand that earlier, before 1833f1a1, it could also return
> SPI_ERROR_CONNECT). Therefore, I suggest making the returned value void
> instead of int and not checking the returned value. What do you think about
> this?

That would break a lot of code (much of it not under our control) to
little purpose; it would also foreclose the option to return to using
SPI_ERROR_CONNECT someday.

We go to a lot of effort to keep the SPI API as stable as we can
across major versions, so I don't see why we'd just randomly make
an API-breaking change like this.

			regards, tom lane

---

From: "David G. Johnston" <[email-masked]>
Date: Sat, 10 Aug 2024 08:18:31 -0700

> SPI_ERROR_CONNECT someday.

I suggest we document it as deprecated and insist any future attempt to
implement a return-on-error connection function define a completely new
function.

David J.

---

From: Tom Lane <[email-masked]>
Date: Sat, 10 Aug 2024 12:29:04 -0400

> implement a return-on-error connection function define a completely new

True; we're kind of in an intermediate place right now where certain
call sites aren't bothering to check the return code, but it's hard
to argue that they're really wrong --- and more to the point,
re-introducing use of SPI_ERROR_CONNECT would break them.  I don't
know if that usage pattern has propagated outside Postgres core,
but it might've.  Perhaps it would be better to update the docs to
say that the only return value is SPI_OK_CONNECT and all failure
cases are reported via elog/ereport.

			regards, tom lane

---

From: "David G. Johnston" <[email-masked]>
Date: Sat, 10 Aug 2024 10:25:29 -0700

On Sat, Aug 10, 2024 at 9:29 AM Tom Lane <[email-masked]> wrote:

> I don't
> but it might've.

I'd give it decent odds since our example usage doesn't include the test.

https://www.postgresql.org/docs/current/spi-examples.html

    /* Convert given text object to a C string */
    command = text_to_cstring(PG_GETARG_TEXT_PP(0));
    cnt = PG_GETARG_INT32(1);

    SPI_connect();

    ret = SPI_exec(command, cnt);

    proc = SPI_processed;

David J.

---

From: Stepan Neretin <[email-masked]>
Date: Sat, 10 Aug 2024 21:34:43 +0300

> https://www.postgresql.org/docs/current/spi-examples.html

https://www.postgresql.org/docs/devel/trigger-example.html
<https://www.postgresql.org/docs/current/spi-examples.html>

> /* connect to SPI manager */

>    if ((ret = SPI_connect()) < 0)
>        elog(ERROR, "trigf (fired %s): SPI_connect returned %d", when, ret);

in this page check include in the example.
I think we need to give examples of one kind. If there is no void in
the code, then there should be checks everywhere (at least in the
documentation).

 What do you think about the attached patch?

Best Regards, Stepan Neretin.
From 4c8d6d9ae6f8843bc6d1ad203d629df09657b039 Mon Sep 17 00:00:00 2001
From: Stepan Neretin <[email-masked]>
Date: Sat, 10 Aug 2024 21:30:37 +0300
Subject: [PATCH v1] Fix SPI Documentation - Standardized the example in the
 execq function to ensure consistent error handling. Now, the error from
 SPI_connect() is checked and reported uniformly. - Removed the
 SPI_ERROR_CONNECT return entry, reflecting that there is now only one return
 value for SPI_connect_ext.

---
 doc/src/sgml/spi.sgml | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/doc/src/sgml/spi.sgml b/doc/src/sgml/spi.sgml
index 7d154914b9..fdd31c3fdf 100644
--- a/doc/src/sgml/spi.sgml
+++ b/doc/src/sgml/spi.sgml
@@ -126,16 +126,10 @@ int SPI_connect_ext(int <parameter>options</parameter>)
      </para>
     </listitem>
    </varlistentry>
-
-   <varlistentry>
-    <term><symbol>SPI_ERROR_CONNECT</symbol></term>
-    <listitem>
-     <para>
-      on error
-     </para>
-    </listitem>
-   </varlistentry>
   </variablelist>
+  <para>
+    All failure cases are reported via <literal>elog/ereport</literal>.
+  </para>
  </refsect1>
 </refentry>
 
@@ -5281,7 +5275,8 @@ execq(PG_FUNCTION_ARGS)
     command = text_to_cstring(PG_GETARG_TEXT_PP(0));
     cnt = PG_GETARG_INT32(1);
 
-    SPI_connect();
+    if ((ret = SPI_connect()) < 0)
+        elog(ERROR, "SPI_connect returned %d");

-- 
2.43.0

---

From: Peter Eisentraut <[email-masked]>
Date: Mon, 12 Aug 2024 14:39:04 +0200

> an API-breaking change like this.

Here is a previous discussion: 
https://www.postgresql.org/message-id/flat/1356682025.20017.4.camel%40vanquo.pezone.net

I like the idea that we would keep the API but convert most errors to 
exceptions.

---

From: Tom Lane <[email-masked]>
Date: Thu, 05 Sep 2024 17:13:50 -0400

> I like the idea that we would keep the API but convert most errors to 

After thinking about this for awhile, one reason that it's practical
to change it today for SPI_connect is that SPI_connect has not
returned anything except SPI_OK_CONNECT since v10.  So if we tell
extension authors they don't need to check the result, it's unlikely
that that will cause any new code they write to get used with PG
versions where it would be wrong.  I fear that we'd need a similar
multi-year journey to get to a place where we could deprecate checking
the result of any other SPI function.

Nonetheless, there seems no reason not to do it now for SPI_connect.
So attached is a patch that documents the result value as vestigial
and removes the calling-code checks in our own code, but doesn't
touch SPI_connect[_ext] itself.  This combines portions of Stepan's
two patches with some additional work (mostly, that he'd missed fixing
any of contrib/).

			regards, tom lane

diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 755293456f..c1c82eb4dd 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -2377,9 +2377,7 @@ get_tuple_of_interest(Relation rel, int *pkattnums, int pknumatts, char **src_pk
 	/*
 	 * Connect to SPI manager
 	 */
-	if ((ret = SPI_connect()) < 0)
-		/* internal error */
-		elog(ERROR, "SPI connect failure - returned %d", ret);
+	SPI_connect();
 
 	initStringInfo(&buf);
 
diff --git a/contrib/spi/refint.c b/contrib/spi/refint.c
index 18062eb1cf..e1aef7cd2a 100644
--- a/contrib/spi/refint.c
+++ b/contrib/spi/refint.c
@@ -108,9 +108,7 @@ check_primary_key(PG_FUNCTION_ARGS)
 	tupdesc = rel->rd_att;
 
 	/* Connect to SPI manager */
-	if ((ret = SPI_connect()) < 0)
-		/* internal error */
-		elog(ERROR, "check_primary_key: SPI_connect returned %d", ret);
+	SPI_connect();
 
 	/*
 	 * We use SPI plan preparation feature, so allocate space to place key
@@ -328,9 +326,7 @@ check_foreign_key(PG_FUNCTION_ARGS)
 	tupdesc = rel->rd_att;
 
 	/* Connect to SPI manager */
-	if ((ret = SPI_connect()) < 0)
-		/* internal error */
-		elog(ERROR, "check_foreign_key: SPI_connect returned %d", ret);
+	SPI_connect();
 
 	/*
 	 * We use SPI plan preparation feature, so allocate space to place key
diff --git a/contrib/tablefunc/tablefunc.c b/contrib/tablefunc/tablefunc.c
index 7d1b5f5143..2a25607a2a 100644
--- a/contrib/tablefunc/tablefunc.c
+++ b/contrib/tablefunc/tablefunc.c
@@ -385,9 +385,7 @@ crosstab(PG_FUNCTION_ARGS)
 	per_query_ctx = rsinfo->econtext->ecxt_per_query_memory;
 
 	/* Connect to SPI manager */
-	if ((ret = SPI_connect()) < 0)
-		/* internal error */
-		elog(ERROR, "crosstab: SPI_connect returned %d", ret);
+	SPI_connect();
 
 	/* Retrieve the desired rows */
 	ret = SPI_execute(sql, true, 0);
@@ -724,9 +722,7 @@ load_categories_hash(char *cats_sql, MemoryContext per_query_ctx)
 								HASH_ELEM | HASH_STRINGS | HASH_CONTEXT);
 
 	/* Connect to SPI manager */
-	if ((ret = SPI_connect()) < 0)
-		/* internal error */
-		elog(ERROR, "load_categories_hash: SPI_connect returned %d", ret);
+	SPI_connect();
 
 	/* Retrieve the category name rows */
 	ret = SPI_execute(cats_sql, true, 0);
@@ -806,9 +802,7 @@ get_crosstab_tuplestore(char *sql,
 	tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
 
 	/* Connect to SPI manager */
-	if ((ret = SPI_connect()) < 0)
-		/* internal error */
-		elog(ERROR, "get_crosstab_tuplestore: SPI_connect returned %d", ret);
+	SPI_connect();
 
 	/* Now retrieve the crosstab source rows */
 	ret = SPI_execute(sql, true, 0);
@@ -1151,15 +1145,11 @@ connectby(char *relname,
 		  AttInMetadata *attinmeta)
 {
 	Tuplestorestate *tupstore = NULL;
-	int			ret;
 	MemoryContext oldcontext;
-
 	int			serial = 1;
 
 	/* Connect to SPI manager */
-	if ((ret = SPI_connect()) < 0)
-		/* internal error */
-		elog(ERROR, "connectby: SPI_connect returned %d", ret);
+	SPI_connect();
 
 	/* switch to longer term context to create the tuple store */
 	oldcontext = MemoryContextSwitchTo(per_query_ctx);
diff --git a/contrib/xml2/xpath.c b/contrib/xml2/xpath.c
index b999b1f706..f94b622d92 100644
--- a/contrib/xml2/xpath.c
+++ b/contrib/xml2/xpath.c
@@ -560,8 +560,7 @@ xpath_table(PG_FUNCTION_ARGS)
 					 relname,
 					 condition);
 
-	if ((ret = SPI_connect()) < 0)
-		elog(ERROR, "xpath_table: SPI_connect returned %d", ret);
+	SPI_connect();
 
 	if ((ret = SPI_exec(query_buf.data, 0)) != SPI_OK_SELECT)
 		elog(ERROR, "xpath_table: SPI execution failed for query %s",
diff --git a/doc/src/sgml/spi.sgml b/doc/src/sgml/spi.sgml
index 7d154914b9..7e2f2df965 100644
--- a/doc/src/sgml/spi.sgml
+++ b/doc/src/sgml/spi.sgml
@@ -126,16 +126,16 @@ int SPI_connect_ext(int <parameter>options</parameter>)
      </para>
     </listitem>
    </varlistentry>
-
-   <varlistentry>
-    <term><symbol>SPI_ERROR_CONNECT</symbol></term>
-    <listitem>
-     <para>
-      on error
-     </para>
-    </listitem>
-   </varlistentry>
   </variablelist>
+
+  <para>
+   The fact that these functions return <type>int</type>
+   not <type>void</type> is historical.  All failure cases are reported
+   via <function>ereport</function> or <function>elog</function>.
+   (In versions before <productname>PostgreSQL</productname> v10,
+   some but not all failures would be reported with a result value
+   of <symbol>SPI_ERROR_CONNECT</symbol>.)
+  </para>
  </refsect1>
 </refentry>
 
diff --git a/doc/src/sgml/trigger.sgml b/doc/src/sgml/trigger.sgml
index 31626536a2..49382d07fa 100644
--- a/doc/src/sgml/trigger.sgml
+++ b/doc/src/sgml/trigger.sgml
@@ -915,8 +915,7 @@ trigf(PG_FUNCTION_ARGS)
     tupdesc = trigdata->tg_relation->rd_att;
 
     /* connect to SPI manager */
-    if ((ret = SPI_connect()) < 0)
-        elog(ERROR, "trigf (fired %s): SPI_connect returned %d", when, ret);
+    SPI_connect();
 
     /* get number of rows in table */
     ret = SPI_exec("SELECT count(*) FROM ttest", 0);
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index b2457f121a..010097873d 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -639,8 +639,7 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
 	relnatts = RelationGetNumberOfAttributes(matviewRel);
 
 	/* Open SPI context. */
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/* Analyze the temp table with the new contents. */
 	appendStringInfo(&querybuf, "ANALYZE %s", tempname);
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 62601a6d80..25931f397f 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -340,8 +340,7 @@ RI_FKey_check(TriggerData *trigdata)
 			break;
 	}
 
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/* Fetch or prepare a saved plan for the real check */
 	ri_BuildQueryKey(&qkey, riinfo, RI_PLAN_CHECK_LOOKUPPK);
@@ -469,8 +468,7 @@ ri_Check_Pk_Match(Relation pk_rel, Relation fk_rel,
 	/* Only called for non-null rows */
 	Assert(ri_NullCheck(RelationGetDescr(pk_rel), oldslot, riinfo, true) == RI_KEYS_NONE_NULL);
 
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/*
 	 * Fetch or prepare a saved plan for checking PK table with values coming
@@ -656,8 +654,7 @@ ri_restrict(TriggerData *trigdata, bool is_no_action)
 		return PointerGetDatum(NULL);
 	}
 
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/*
 	 * Fetch or prepare a saved plan for the restrict lookup (it's the same
@@ -766,8 +763,7 @@ RI_FKey_cascade_del(PG_FUNCTION_ARGS)
 	pk_rel = trigdata->tg_relation;
 	oldslot = trigdata->tg_trigslot;
 
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/* Fetch or prepare a saved plan for the cascaded delete */
 	ri_BuildQueryKey(&qkey, riinfo, RI_PLAN_CASCADE_ONDELETE);
@@ -875,8 +871,7 @@ RI_FKey_cascade_upd(PG_FUNCTION_ARGS)
 	newslot = trigdata->tg_newslot;
 	oldslot = trigdata->tg_trigslot;
 
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/* Fetch or prepare a saved plan for the cascaded update */
 	ri_BuildQueryKey(&qkey, riinfo, RI_PLAN_CASCADE_ONUPDATE);
@@ -1051,8 +1046,7 @@ ri_set(TriggerData *trigdata, bool is_set_null, int tgkind)
 	pk_rel = trigdata->tg_relation;
 	oldslot = trigdata->tg_trigslot;
 
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/*
 	 * Fetch or prepare a saved plan for the trigger.
@@ -1547,8 +1541,7 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
 							 PGC_USERSET, PGC_S_SESSION,
 							 GUC_ACTION_SAVE, true, 0, false);
 
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/*
 	 * Generate the plan.  We don't need to cache it, and there are no
@@ -1787,8 +1780,7 @@ RI_PartitionRemove_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
 							 PGC_USERSET, PGC_S_SESSION,
 							 GUC_ACTION_SAVE, true, 0, false);
 
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/*
 	 * Generate the plan.  We don't need to cache it, and there are no
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index b31be31321..53ea224c72 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -589,8 +589,7 @@ pg_get_ruledef_worker(Oid ruleoid, int prettyFlags)
 	/*
 	 * Connect to SPI manager
 	 */
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/*
 	 * On the first call prepare the plan to lookup pg_rewrite. We read
@@ -782,8 +781,7 @@ pg_get_viewdef_worker(Oid viewoid, int prettyFlags, int wrapColumn)
 	/*
 	 * Connect to SPI manager
 	 */
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/*
 	 * On the first call prepare the plan to lookup pg_rewrite. We read
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index d68ad7be34..fe719935c6 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -1947,8 +1947,7 @@ plperl_inline_handler(PG_FUNCTION_ARGS)
 
 		current_call_data = &this_call_data;
 
-		if (SPI_connect_ext(codeblock->atomic ? 0 : SPI_OPT_NONATOMIC) != SPI_OK_CONNECT)
-			elog(ERROR, "could not connect to SPI manager");
+		SPI_connect_ext(codeblock->atomic ? 0 : SPI_OPT_NONATOMIC);
 
 		select_perl_context(desc.lanpltrusted);
 
@@ -2412,8 +2411,7 @@ plperl_func_handler(PG_FUNCTION_ARGS)
 		IsA(fcinfo->context, CallContext) &&
 		!castNode(CallContext, fcinfo->context)->atomic;
 
-	if (SPI_connect_ext(nonatomic ? SPI_OPT_NONATOMIC : 0) != SPI_OK_CONNECT)
-		elog(ERROR, "could not connect to SPI manager");
+	SPI_connect_ext(nonatomic ? SPI_OPT_NONATOMIC : 0);
 
 	prodesc = compile_plperl_function(fcinfo->flinfo->fn_oid, false, false);
 	current_call_data->prodesc = prodesc;
@@ -2530,8 +2528,7 @@ plperl_trigger_handler(PG_FUNCTION_ARGS)
 	int			rc PG_USED_FOR_ASSERTS_ONLY;
 
 	/* Connect to SPI manager */
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "could not connect to SPI manager");
+	SPI_connect();
 
 	/* Make transition tables visible to this SPI connection */
 	tdata = (TriggerData *) fcinfo->context;
@@ -2638,8 +2635,7 @@ plperl_event_trigger_handler(PG_FUNCTION_ARGS)
 	ErrorContextCallback pl_error_context;
 
 	/* Connect to SPI manager */
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "could not connect to SPI manager");
+	SPI_connect();
 
 	/* Find or compile the function */
 	prodesc = compile_plperl_function(fcinfo->flinfo->fn_oid, false, true);
diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
index 980f0961bc..adfbbc8a7b 100644
--- a/src/pl/plpgsql/src/pl_handler.c
+++ b/src/pl/plpgsql/src/pl_handler.c
@@ -235,8 +235,7 @@ plpgsql_call_handler(PG_FUNCTION_ARGS)
 	/*
 	 * Connect to SPI manager
 	 */
-	if ((rc = SPI_connect_ext(nonatomic ? SPI_OPT_NONATOMIC : 0)) != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed: %s", SPI_result_code_string(rc));
+	SPI_connect_ext(nonatomic ? SPI_OPT_NONATOMIC : 0);
 
 	/* Find or compile the function */
 	func = plpgsql_compile(fcinfo, false);
@@ -326,8 +325,7 @@ plpgsql_inline_handler(PG_FUNCTION_ARGS)
 	/*
 	 * Connect to SPI manager
 	 */
-	if ((rc = SPI_connect_ext(codeblock->atomic ? 0 : SPI_OPT_NONATOMIC)) != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed: %s", SPI_result_code_string(rc));
+	SPI_connect_ext(codeblock->atomic ? 0 : SPI_OPT_NONATOMIC);
 
 	/* Compile the anonymous code block */
 	func = plpgsql_compile_inline(codeblock->source_text);
@@ -510,8 +508,7 @@ plpgsql_validator(PG_FUNCTION_ARGS)
 		/*
 		 * Connect to SPI manager (is this needed for compilation?)
 		 */
-		if ((rc = SPI_connect()) != SPI_OK_CONNECT)
-			elog(ERROR, "SPI_connect failed: %s", SPI_result_code_string(rc));
+		SPI_connect();
 
 		/*
 		 * Set up a fake fcinfo with just enough info to satisfy
diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c
index 010a97378c..8117e20efa 100644
--- a/src/pl/plpython/plpy_main.c
+++ b/src/pl/plpython/plpy_main.c
@@ -202,8 +202,7 @@ plpython3_call_handler(PG_FUNCTION_ARGS)
 		!castNode(CallContext, fcinfo->context)->atomic;
 
 	/* Note: SPI_finish() happens in plpy_exec.c, which is dubious design */
-	if (SPI_connect_ext(nonatomic ? SPI_OPT_NONATOMIC : 0) != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect_ext(nonatomic ? SPI_OPT_NONATOMIC : 0);
 
 	/*
 	 * Push execution context onto stack.  It is important that this get
@@ -272,8 +271,7 @@ plpython3_inline_handler(PG_FUNCTION_ARGS)
 	PLy_initialize();
 
 	/* Note: SPI_finish() happens in plpy_exec.c, which is dubious design */
-	if (SPI_connect_ext(codeblock->atomic ? 0 : SPI_OPT_NONATOMIC) != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect_ext(codeblock->atomic ? 0 : SPI_OPT_NONATOMIC);
 
 	MemSet(fcinfo, 0, SizeForFunctionCallInfo(0));
 	MemSet(&flinfo, 0, sizeof(flinfo));
diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c
index 21b2b04593..e2ccaa84f3 100644
--- a/src/pl/tcl/pltcl.c
+++ b/src/pl/tcl/pltcl.c
@@ -808,8 +808,7 @@ pltcl_func_handler(PG_FUNCTION_ARGS, pltcl_call_state *call_state,
 		!castNode(CallContext, fcinfo->context)->atomic;
 
 	/* Connect to SPI manager */
-	if (SPI_connect_ext(nonatomic ? SPI_OPT_NONATOMIC : 0) != SPI_OK_CONNECT)
-		elog(ERROR, "could not connect to SPI manager");
+	SPI_connect_ext(nonatomic ? SPI_OPT_NONATOMIC : 0);
 
 	/* Find or compile the function */
 	prodesc = compile_pltcl_function(fcinfo->flinfo->fn_oid, InvalidOid,
@@ -1072,8 +1071,7 @@ pltcl_trigger_handler(PG_FUNCTION_ARGS, pltcl_call_state *call_state,
 	call_state->trigdata = trigdata;
 
 	/* Connect to SPI manager */
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "could not connect to SPI manager");
+	SPI_connect();
 
 	/* Make transition tables visible to this SPI connection */
 	rc = SPI_register_trigger_data(trigdata);
@@ -1321,8 +1319,7 @@ pltcl_event_trigger_handler(PG_FUNCTION_ARGS, pltcl_call_state *call_state,
 	int			tcl_rc;
 
 	/* Connect to SPI manager */
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "could not connect to SPI manager");
+	SPI_connect();
 
 	/* Find or compile the function */
 	prodesc = compile_pltcl_function(fcinfo->flinfo->fn_oid,
diff --git a/src/test/modules/plsample/plsample.c b/src/test/modules/plsample/plsample.c
index 40c462e84e..89ea166a67 100644
--- a/src/test/modules/plsample/plsample.c
+++ b/src/test/modules/plsample/plsample.c
@@ -220,8 +220,7 @@ plsample_trigger_handler(PG_FUNCTION_ARGS)
 		elog(ERROR, "not called by trigger manager");
 
 	/* Connect to the SPI manager */
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "could not connect to SPI manager");
+	SPI_connect();
 
 	rc = SPI_register_trigger_data(trigdata);
 	Assert(rc >= 0);
diff --git a/src/test/modules/test_predtest/test_predtest.c b/src/test/modules/test_predtest/test_predtest.c
index 27ac136ca5..678b13ca30 100644
--- a/src/test/modules/test_predtest/test_predtest.c
+++ b/src/test/modules/test_predtest/test_predtest.c
@@ -54,8 +54,7 @@ test_predtest(PG_FUNCTION_ARGS)
 	int			i;
 
 	/* We use SPI to parse, plan, and execute the test query */
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/*
 	 * First, plan and execute the query, and inspect the results.  To the
diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c
index 14aad5a0c6..9e81371be4 100644
--- a/src/test/regress/regress.c
+++ b/src/test/regress/regress.c
@@ -379,8 +379,7 @@ ttdummy(PG_FUNCTION_ARGS)
 	newoff = Int32GetDatum((int32) DatumGetInt64(newoff));
 
 	/* Connect to SPI manager */
-	if ((ret = SPI_connect()) < 0)
-		elog(ERROR, "ttdummy (%s): SPI_connect returned %d", relname, ret);
+	SPI_connect();
 
 	/* Fetch tuple values and nulls */
 	cvals = (Datum *) palloc(natts * sizeof(Datum));

---

From: Stepan Neretin <[email-masked]>
Date: Sun, 8 Sep 2024 13:35:00 +0700

> So if we tell extension authors they don't need to check the result, it's
unlikely
> versions where it would be wrong.
Yes, I concur.

> This combines portions of Stepan's
Thank you for the feedback! I believe the patch looks satisfactory. Should
we await a review? The changes seem straightforward to me.

---

From: Tom Lane <[email-masked]>
Date: Sun, 08 Sep 2024 03:31:23 -0400

> we await a review? The changes seem straightforward to me.

I too think it's good to go.  If no one complains or asks for
more time to review, I will push it Monday or so.

			regards, tom lane

---

From: Tom Lane <[email-masked]>
Date: Mon, 09 Sep 2024 12:19:58 -0400

> more time to review, I will push it Monday or so.

And done.

			regards, tom lane