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?
Best Regards, Stepan Neretin.
From 71affb48637159fc96d80a660e131d64d97cb6f9 Mon Sep 17 00:00:00 2001
From: Stepan Neretin <s.nere...@postgrespro.ru>
Date: Sat, 10 Aug 2024 16:51:24 +0300
Subject: [PATCH] Refactor SPI_connect and SPI_connect_ext functions to return
void
- Changed the return type of both `SPI_connect` and `SPI_connect_ext` from `int` to `void`, simplifying their interfaces by no longer returning a status code.
- Updated relevant code in all files where `SPI_connect` was used, removing checks for `SPI_OK_CONNECT` as they are no longer necessary.
- The return value in both functions was removed, as it was no longer needed to indicate connection status.
---
src/backend/commands/matview.c | 3 +--
src/backend/executor/spi.c | 8 +++----
src/backend/utils/adt/ri_triggers.c | 24 +++++++------------
src/backend/utils/adt/ruleutils.c | 6 ++---
src/include/executor/spi.h | 4 ++--
src/pl/plperl/plperl.c | 6 ++---
src/pl/plpgsql/src/pl_handler.c | 10 +++-----
src/pl/plpython/plpy_main.c | 6 ++---
src/pl/tcl/pltcl.c | 9 +++----
src/test/modules/plsample/plsample.c | 3 +--
.../modules/test_predtest/test_predtest.c | 3 +--
src/test/regress/regress.c | 3 +--
12 files changed, 29 insertions(+), 56 deletions(-)
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index 91f0fd6ea3..4fd433d410 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/executor/spi.c b/src/backend/executor/spi.c
index d6516b1bca..50026ba1d6 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -90,13 +90,13 @@ static bool _SPI_checktuples(void);
/* =================== interface functions =================== */
-int
+void
SPI_connect(void)
{
- return SPI_connect_ext(0);
+ SPI_connect_ext(0);
}
-int
+void
SPI_connect_ext(int options)
{
int newdepth;
@@ -174,8 +174,6 @@ SPI_connect_ext(int options)
SPI_processed = 0;
SPI_tuptable = NULL;
SPI_result = 0;
-
- return SPI_OK_CONNECT;
}
int
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 c656afc35a..0ac5c3089e 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -596,8 +596,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
@@ -789,8 +788,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/include/executor/spi.h b/src/include/executor/spi.h
index 48b87730ea..66aa526a71 100644
--- a/src/include/executor/spi.h
+++ b/src/include/executor/spi.h
@@ -105,8 +105,8 @@ extern PGDLLIMPORT uint64 SPI_processed;
extern PGDLLIMPORT SPITupleTable *SPI_tuptable;
extern PGDLLIMPORT int SPI_result;
-extern int SPI_connect(void);
-extern int SPI_connect_ext(int options);
+extern void SPI_connect(void);
+extern void SPI_connect_ext(int options);
extern int SPI_finish(void);
extern int SPI_execute(const char *src, bool read_only, long tcount);
extern int SPI_execute_extended(const char *src,
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index d68ad7be34..b1cae53b49 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -2530,8 +2530,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 +2637,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..ec7cbfb33c 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,9 +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 +507,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..3a8fa5dc09 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..71a0afd868 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));
--
2.43.0
