Hackers,
As discussed with Tom in [1] and again with Pavel and Alvaro in [2],
here is a partial WIP refactoring of the SPI interface. The goal is to
remove as many of the SPI_ERROR_xxx codes as possible from the
interface, replacing them with elog(ERROR), without removing the ability
of callers to check meaningful return codes and make their own decisions
about what to do next. The crucial point here is that many of the error
codes in SPI are "can't happen" or "you made a programmatic mistake"
type errors that don't require run time remediation, but rather require
fixing the C code and recompiling. Those seem better handled as an
elog(ERROR) to avoid the need for tests against such error codes
sprinkled throughout the system.
One upshot of the refactoring is that some of the SPI functions that
previously returned an error code can be changed to return void. Tom
suggested a nice way to use macros to provide backward compatibility
with older third-party software, and I used his suggestion.
I need guidance with SPI_ERROR_ARGUMENT because it is used by functions
that don't return an integer error code, but rather return a SPIPlanPtr,
such as SPI_prepare. Those functions return NULL and set a global
variable named SPI_result to the error code. I'd be happy to just
convert this to throwing an error, too, but it is a greater API break
than anything implemented in the attached patches so far. How do others
feel about it?
If more places like this can be converted to use elog(ERROR), it may be
possible to convert more functions to return void.
[1] https://www.postgresql.org/message-id/13404.1558558354%40sss.pgh.pa.us
[2]
https://www.postgresql.org/message-id/20191106151112.GA12251%40alvherre.pgsql
--
Mark Dilger
>From 113d42772be2c2abd71fd142cde9240522f143d7 Mon Sep 17 00:00:00 2001
From: Mark Dilger <hornschnor...@gmail.com>
Date: Thu, 7 Nov 2019 07:51:06 -0800
Subject: [PATCH v1 1/5] Deprecating unused SPI error codes.
The SPI_ERROR_NOOUTFUNC and SPI_ERROR_CONNECT codes, defined in spi.h,
were no longer used anywhere in the sources except nominally in spi.c
where SPI_result_code_string(int code) was translating them to a cstring
for use in error messages. But since the system never returns these
error codes, it seems unlikely anybody still needs to be able to convert
them to a string.
Removing these from spi.c, from the docs, and from a code comment in
contrib. Leaving the definition in spi.h for backwards compatibility of
third-party applications.
---
contrib/spi/refint.c | 2 +-
doc/src/sgml/spi.sgml | 14 ++------------
src/backend/executor/spi.c | 4 ----
src/include/executor/spi.h | 4 ++--
4 files changed, 5 insertions(+), 19 deletions(-)
diff --git a/contrib/spi/refint.c b/contrib/spi/refint.c
index adf0490f85..00253a63e9 100644
--- a/contrib/spi/refint.c
+++ b/contrib/spi/refint.c
@@ -392,7 +392,7 @@ check_foreign_key(PG_FUNCTION_ARGS)
char *oldval = SPI_getvalue(trigtuple, tupdesc, fnumber);
char *newval;
- /* this shouldn't happen! SPI_ERROR_NOOUTFUNC ? */
+ /* this shouldn't happen! */
if (oldval == NULL)
/* internal error */
elog(ERROR, "check_foreign_key: SPI_getvalue returned %s", SPI_result_code_string(SPI_result));
diff --git a/doc/src/sgml/spi.sgml b/doc/src/sgml/spi.sgml
index 9e37fc3a14..d1e6582d96 100644
--- a/doc/src/sgml/spi.sgml
+++ b/doc/src/sgml/spi.sgml
@@ -128,14 +128,6 @@ int SPI_connect_ext(int <parameter>options</parameter>)
</listitem>
</varlistentry>
- <varlistentry>
- <term><symbol>SPI_ERROR_CONNECT</symbol></term>
- <listitem>
- <para>
- on error
- </para>
- </listitem>
- </varlistentry>
</variablelist>
</refsect1>
</refentry>
@@ -3231,12 +3223,10 @@ char * SPI_getvalue(HeapTuple <parameter>row</parameter>, TupleDesc <parameter>r
<title>Return Value</title>
<para>
- Column value, or <symbol>NULL</symbol> if the column is null,
+ Column value, or <symbol>NULL</symbol> if the column is null, or
<parameter>colnumber</parameter> is out of range
(<varname>SPI_result</varname> is set to
- <symbol>SPI_ERROR_NOATTRIBUTE</symbol>), or no output function is
- available (<varname>SPI_result</varname> is set to
- <symbol>SPI_ERROR_NOOUTFUNC</symbol>).
+ <symbol>SPI_ERROR_NOATTRIBUTE</symbol>).
</para>
</refsect1>
</refentry>
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 2c0ae395ba..2bb49e6919 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -1708,8 +1708,6 @@ SPI_result_code_string(int code)
switch (code)
{
- case SPI_ERROR_CONNECT:
- return "SPI_ERROR_CONNECT";
case SPI_ERROR_COPY:
return "SPI_ERROR_COPY";
case SPI_ERROR_OPUNKNOWN:
@@ -1724,8 +1722,6 @@ SPI_result_code_string(int code)
return "SPI_ERROR_TRANSACTION";
case SPI_ERROR_NOATTRIBUTE:
return "SPI_ERROR_NOATTRIBUTE";
- case SPI_ERROR_NOOUTFUNC:
- return "SPI_ERROR_NOOUTFUNC";
case SPI_ERROR_TYPUNKNOWN:
return "SPI_ERROR_TYPUNKNOWN";
case SPI_ERROR_REL_DUPLICATE:
diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h
index ad69a5ce43..a2ea6dda5f 100644
--- a/src/include/executor/spi.h
+++ b/src/include/executor/spi.h
@@ -36,7 +36,7 @@ typedef struct SPITupleTable
/* Plans are opaque structs for standard users of SPI */
typedef struct _SPI_plan *SPIPlanPtr;
-#define SPI_ERROR_CONNECT (-1)
+#define SPI_ERROR_CONNECT (-1) /* not used anymore */
#define SPI_ERROR_COPY (-2)
#define SPI_ERROR_OPUNKNOWN (-3)
#define SPI_ERROR_UNCONNECTED (-4)
@@ -45,7 +45,7 @@ typedef struct _SPI_plan *SPIPlanPtr;
#define SPI_ERROR_PARAM (-7)
#define SPI_ERROR_TRANSACTION (-8)
#define SPI_ERROR_NOATTRIBUTE (-9)
-#define SPI_ERROR_NOOUTFUNC (-10)
+#define SPI_ERROR_NOOUTFUNC (-10) /* not used anymore */
#define SPI_ERROR_TYPUNKNOWN (-11)
#define SPI_ERROR_REL_DUPLICATE (-12)
#define SPI_ERROR_REL_NOT_FOUND (-13)
base-commit: effa40281bb1f6c71e81c980f86852ab3be603c3
--
2.20.1
>From ac1036a40fd15745567ab3f5a4e90930a2f9a7b4 Mon Sep 17 00:00:00 2001
From: Mark Dilger <hornschnor...@gmail.com>
Date: Thu, 7 Nov 2019 08:29:07 -0800
Subject: [PATCH v1 2/5] Changing SPI_connect and SPI_connect_ext to return
void.
SPI_connect and SPI_connect_ext were always returning SPI_OK_CONNECT if they
returned at all. Numerous places in the code were needlessly checking the
return value of these two functions. I am changing the functions to return
void, and including two backward compatibility macros to avoid breaking
third-party code which either does not get updated, or which needs to work
across different versions.
This patch is by me, but some of these ideas and a sketch of the backward
compatibility macros provided by Tom Lane:
https://www.postgresql.org/message-id/14823.1558630129%40sss.pgh.pa.us
---
contrib/dblink/dblink.c | 4 +---
contrib/spi/refint.c | 8 ++-----
contrib/tablefunc/tablefunc.c | 17 ++++---------
src/backend/commands/matview.c | 3 +--
src/backend/executor/spi.c | 11 +++++----
src/backend/utils/adt/ri_triggers.c | 24 +++++++------------
src/backend/utils/adt/ruleutils.c | 6 ++---
src/include/executor/spi.h | 18 ++++++++++++--
src/pl/plpgsql/src/pl_handler.c | 9 +++----
.../modules/test_predtest/test_predtest.c | 3 +--
src/test/regress/regress.c | 3 +--
11 files changed, 45 insertions(+), 61 deletions(-)
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 7e225589a9..5587c57bf9 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -2379,9 +2379,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 00253a63e9..a71ef931ec 100644
--- a/contrib/spi/refint.c
+++ b/contrib/spi/refint.c
@@ -107,9 +107,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
@@ -326,9 +324,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 256d52fc62..8866bbf0ed 100644
--- a/contrib/tablefunc/tablefunc.c
+++ b/contrib/tablefunc/tablefunc.c
@@ -379,9 +379,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);
@@ -726,9 +724,7 @@ load_categories_hash(char *cats_sql, MemoryContext per_query_ctx)
HASH_ELEM | 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);
@@ -805,9 +801,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);
@@ -1157,15 +1151,12 @@ 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/src/backend/commands/matview.c b/src/backend/commands/matview.c
index 537d0e8cef..b11c98a093 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -605,8 +605,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 2bb49e6919..c3943ebbb9 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -82,16 +82,19 @@ static MemoryContext _SPI_execmem(void);
static MemoryContext _SPI_procmem(void);
static bool _SPI_checktuples(void);
+/* Defend against BACKWARDS_COMPATIBLE_SPI_CALLS wrapper macros */
+#undef SPI_connect
+#undef SPI_connect_ext
/* =================== 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;
@@ -168,8 +171,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 caf27f8bcb..1462f29e25 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -334,8 +334,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);
@@ -459,8 +458,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
@@ -646,8 +644,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
@@ -756,8 +753,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_DEL_DODELETE);
@@ -865,8 +861,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_UPD_DOUPDATE);
@@ -1040,8 +1035,7 @@ ri_set(TriggerData *trigdata, bool is_set_null)
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 set null/default operation (it's
@@ -1464,8 +1458,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
@@ -1699,8 +1692,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 3e64390d81..acf3f804af 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -534,8 +534,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
@@ -727,8 +726,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 a2ea6dda5f..2d505610ff 100644
--- a/src/include/executor/spi.h
+++ b/src/include/executor/spi.h
@@ -81,8 +81,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_plan(SPIPlanPtr plan, Datum *Values, const char *Nulls,
@@ -172,4 +172,18 @@ extern void AtEOXact_SPI(bool isCommit);
extern void AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid);
extern bool SPI_inside_nonatomic_context(void);
+/*
+ * Backward compatibility for third-party code which expects the older SPI
+ * function prototypes returning int rather than void.
+ *
+ * This switch should only be defined when compiling legacy third-party code
+ * which expects an integer return value, otherwise, at least under some
+ * compilers, compilation may draw warnings about the right-hand side value
+ * being ignored (-Wunused-value).
+ */
+#ifdef BACKWARDS_COMPATIBLE_SPI_CALLS
+#define SPI_connect() (SPI_connect(), SPI_OK_CONNECT)
+#define SPI_connect_ext(o) (SPI_connect_ext(o), SPI_OK_CONNECT)
+#endif
+
#endif /* SPI_H */
diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
index 1b592c8a52..3e41f36f61 100644
--- a/src/pl/plpgsql/src/pl_handler.c
+++ b/src/pl/plpgsql/src/pl_handler.c
@@ -234,8 +234,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);
@@ -303,8 +302,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);
@@ -468,8 +466,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/test/modules/test_predtest/test_predtest.c b/src/test/modules/test_predtest/test_predtest.c
index 10cd4f9ae5..d612e29198 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 2f66d76ab3..7d8086cd2c 100644
--- a/src/test/regress/regress.c
+++ b/src/test/regress/regress.c
@@ -368,8 +368,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.20.1
>From e89cb24c2f98cfd706fc053ac60703d374a3e83e Mon Sep 17 00:00:00 2001
From: Mark Dilger <hornschnor...@gmail.com>
Date: Thu, 7 Nov 2019 10:10:17 -0800
Subject: [PATCH v1 3/5] Refactoring SPI trigger related errors.
Deprecating SPI_ERROR_REL_DUPLICATE and SPI_ERROR_REL_NOT_FOUND error
codes in favor of throwing an error. These errors should not be
reachable from within the core code.
Specifically, only the function SPI_register_trigger_data is calling
SPI_register_relation, which was the only function returning
SPI_ERROR_REL_DUPLICATE. The call to SPI_register_relation is for the
purpose of registering the NEW and OLD trigger relations, and given how
the code is written, it cannot make the mistake of registering them more
than once. Hence, SPI_ERROR_REL_DUPLICATE was unreachable by a user,
except through poorly written third-party C code that had some other
usage pattern.
SPI_unregister_relation was the only function returning
SPI_ERROR_REL_NOT_FOUND. No calls are made to SPI_unregister_relation
from core, so once again, it should be unreachable by a user except
through poorly written third-party C code.
---
doc/src/sgml/spi.sgml | 27 ---------------------------
src/backend/executor/spi.c | 8 ++------
src/include/executor/spi.h | 4 ++--
3 files changed, 4 insertions(+), 35 deletions(-)
diff --git a/doc/src/sgml/spi.sgml b/doc/src/sgml/spi.sgml
index d1e6582d96..abe1475256 100644
--- a/doc/src/sgml/spi.sgml
+++ b/doc/src/sgml/spi.sgml
@@ -2756,15 +2756,6 @@ int SPI_register_relation(EphemeralNamedRelation <parameter>enr</parameter>)
</listitem>
</varlistentry>
- <varlistentry>
- <term><symbol>SPI_ERROR_REL_DUPLICATE</symbol></term>
- <listitem>
- <para>
- if the name specified in the <varname>name</varname> field of
- <parameter>enr</parameter> is already registered for this connection
- </para>
- </listitem>
- </varlistentry>
</variablelist>
</para>
</refsect1>
@@ -2861,15 +2852,6 @@ int SPI_unregister_relation(const char * <parameter>name</parameter>)
</listitem>
</varlistentry>
- <varlistentry>
- <term><symbol>SPI_ERROR_REL_NOT_FOUND</symbol></term>
- <listitem>
- <para>
- if <parameter>name</parameter> is not found in the registry for the
- current connection
- </para>
- </listitem>
- </varlistentry>
</variablelist>
</para>
</refsect1>
@@ -2976,15 +2958,6 @@ int SPI_register_trigger_data(TriggerData *<parameter>tdata</parameter>)
</listitem>
</varlistentry>
- <varlistentry>
- <term><symbol>SPI_ERROR_REL_DUPLICATE</symbol></term>
- <listitem>
- <para>
- if the name of any trigger data transient relation is already
- registered for this connection
- </para>
- </listitem>
- </varlistentry>
</variablelist>
</para>
</refsect1>
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index c3943ebbb9..7d227a11d2 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -1725,10 +1725,6 @@ SPI_result_code_string(int code)
return "SPI_ERROR_NOATTRIBUTE";
case SPI_ERROR_TYPUNKNOWN:
return "SPI_ERROR_TYPUNKNOWN";
- case SPI_ERROR_REL_DUPLICATE:
- return "SPI_ERROR_REL_DUPLICATE";
- case SPI_ERROR_REL_NOT_FOUND:
- return "SPI_ERROR_REL_NOT_FOUND";
case SPI_OK_CONNECT:
return "SPI_OK_CONNECT";
case SPI_OK_FINISH:
@@ -2875,7 +2871,7 @@ SPI_register_relation(EphemeralNamedRelation enr)
match = _SPI_find_ENR_by_name(enr->md.name);
if (match)
- res = SPI_ERROR_REL_DUPLICATE;
+ elog(ERROR, "duplicate relation registration");
else
{
if (_SPI_current->queryEnv == NULL)
@@ -2914,7 +2910,7 @@ SPI_unregister_relation(const char *name)
res = SPI_OK_REL_UNREGISTER;
}
else
- res = SPI_ERROR_REL_NOT_FOUND;
+ elog(ERROR, "unknown relation unregistration");
_SPI_end_call(false);
diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h
index 2d505610ff..7a43b9d21e 100644
--- a/src/include/executor/spi.h
+++ b/src/include/executor/spi.h
@@ -47,8 +47,8 @@ typedef struct _SPI_plan *SPIPlanPtr;
#define SPI_ERROR_NOATTRIBUTE (-9)
#define SPI_ERROR_NOOUTFUNC (-10) /* not used anymore */
#define SPI_ERROR_TYPUNKNOWN (-11)
-#define SPI_ERROR_REL_DUPLICATE (-12)
-#define SPI_ERROR_REL_NOT_FOUND (-13)
+#define SPI_ERROR_REL_DUPLICATE (-12) /* not used anymore */
+#define SPI_ERROR_REL_NOT_FOUND (-13) /* not used anymore */
#define SPI_OK_CONNECT 1
#define SPI_OK_FINISH 2
--
2.20.1
>From 6a3a28847b7089d7688d8a906b0046f57204bf83 Mon Sep 17 00:00:00 2001
From: Mark Dilger <hornschnor...@gmail.com>
Date: Thu, 7 Nov 2019 12:20:46 -0800
Subject: [PATCH v1 4/5] Refactoring SPI execute related errors.
Deprecating SPI_ERROR_PARAM in favor of throwing an error. This error
code was only being returned when a caller of SPI_execute_plan,
SPI_execute_snapshot, or SPI_execute_with_args passed in a plan which
has nargs > 0 but then negligently passed in NULL for the Values and/or
argtypes. This would be a C coding error, not a state of affairs that a
user could trigger.
---
doc/src/sgml/spi.sgml | 9 ---------
src/backend/executor/spi.c | 8 +++-----
src/include/executor/spi.h | 2 +-
3 files changed, 4 insertions(+), 15 deletions(-)
diff --git a/doc/src/sgml/spi.sgml b/doc/src/sgml/spi.sgml
index abe1475256..cca91f6ef6 100644
--- a/doc/src/sgml/spi.sgml
+++ b/doc/src/sgml/spi.sgml
@@ -1439,15 +1439,6 @@ int SPI_execute_plan(SPIPlanPtr <parameter>plan</parameter>, Datum * <parameter>
</listitem>
</varlistentry>
- <varlistentry>
- <term><symbol>SPI_ERROR_PARAM</symbol></term>
- <listitem>
- <para>
- if <parameter>values</parameter> is <symbol>NULL</symbol> and
- <parameter>plan</parameter> was prepared with some parameters
- </para>
- </listitem>
- </varlistentry>
</variablelist>
</para>
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 7d227a11d2..579e80a6a7 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -538,7 +538,7 @@ SPI_execute_plan(SPIPlanPtr plan, Datum *Values, const char *Nulls,
return SPI_ERROR_ARGUMENT;
if (plan->nargs > 0 && Values == NULL)
- return SPI_ERROR_PARAM;
+ elog(ERROR, "SPI_execute_plan passed invalid parameters");
res = _SPI_begin_call(true);
if (res < 0)
@@ -608,7 +608,7 @@ SPI_execute_snapshot(SPIPlanPtr plan,
return SPI_ERROR_ARGUMENT;
if (plan->nargs > 0 && Values == NULL)
- return SPI_ERROR_PARAM;
+ elog(ERROR, "SPI_execute_snapshot passed invalid parameters");
res = _SPI_begin_call(true);
if (res < 0)
@@ -644,7 +644,7 @@ SPI_execute_with_args(const char *src,
return SPI_ERROR_ARGUMENT;
if (nargs > 0 && (argtypes == NULL || Values == NULL))
- return SPI_ERROR_PARAM;
+ elog(ERROR, "SPI_execute_with_args passed invalid parameters");
res = _SPI_begin_call(true);
if (res < 0)
@@ -1717,8 +1717,6 @@ SPI_result_code_string(int code)
return "SPI_ERROR_UNCONNECTED";
case SPI_ERROR_ARGUMENT:
return "SPI_ERROR_ARGUMENT";
- case SPI_ERROR_PARAM:
- return "SPI_ERROR_PARAM";
case SPI_ERROR_TRANSACTION:
return "SPI_ERROR_TRANSACTION";
case SPI_ERROR_NOATTRIBUTE:
diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h
index 7a43b9d21e..575978737f 100644
--- a/src/include/executor/spi.h
+++ b/src/include/executor/spi.h
@@ -42,7 +42,7 @@ typedef struct _SPI_plan *SPIPlanPtr;
#define SPI_ERROR_UNCONNECTED (-4)
#define SPI_ERROR_CURSOR (-5) /* not used anymore */
#define SPI_ERROR_ARGUMENT (-6)
-#define SPI_ERROR_PARAM (-7)
+#define SPI_ERROR_PARAM (-7) /* not used anymore */
#define SPI_ERROR_TRANSACTION (-8)
#define SPI_ERROR_NOATTRIBUTE (-9)
#define SPI_ERROR_NOOUTFUNC (-10) /* not used anymore */
--
2.20.1
>From 15aa7a3ac17a13b35bcf335f6f8230ea12eda598 Mon Sep 17 00:00:00 2001
From: Mark Dilger <hornschnor...@gmail.com>
Date: Thu, 7 Nov 2019 14:38:06 -0800
Subject: [PATCH v1 5/5] Refactoring SPI related errors.
Deprecating SPI_ERROR_OPUNKNOWN error code in favor of throwing an
exception instead. This code was only returned by SPI_execute and
SPI_execute_plan in the event that they failed to handle all relevant
cases in a C switch statement.
---
doc/src/sgml/spi.sgml | 9 ---------
src/backend/executor/spi.c | 9 +++++----
src/include/executor/spi.h | 2 +-
3 files changed, 6 insertions(+), 14 deletions(-)
diff --git a/doc/src/sgml/spi.sgml b/doc/src/sgml/spi.sgml
index cca91f6ef6..af822cfbe7 100644
--- a/doc/src/sgml/spi.sgml
+++ b/doc/src/sgml/spi.sgml
@@ -520,15 +520,6 @@ typedef struct SPITupleTable
</listitem>
</varlistentry>
- <varlistentry>
- <term><symbol>SPI_ERROR_OPUNKNOWN</symbol></term>
- <listitem>
- <para>
- if the command type is unknown (shouldn't happen)
- </para>
- </listitem>
- </varlistentry>
-
<varlistentry>
<term><symbol>SPI_ERROR_UNCONNECTED</symbol></term>
<listitem>
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 579e80a6a7..f2a90f0d0d 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -1711,8 +1711,6 @@ SPI_result_code_string(int code)
{
case SPI_ERROR_COPY:
return "SPI_ERROR_COPY";
- case SPI_ERROR_OPUNKNOWN:
- return "SPI_ERROR_OPUNKNOWN";
case SPI_ERROR_UNCONNECTED:
return "SPI_ERROR_UNCONNECTED";
case SPI_ERROR_ARGUMENT:
@@ -2459,7 +2457,7 @@ _SPI_convert_params(int nargs, Oid *argtypes,
static int
_SPI_pquery(QueryDesc *queryDesc, bool fire_triggers, uint64 tcount)
{
- int operation = queryDesc->operation;
+ CmdType operation = queryDesc->operation;
int eflags;
int res;
@@ -2492,8 +2490,11 @@ _SPI_pquery(QueryDesc *queryDesc, bool fire_triggers, uint64 tcount)
else
res = SPI_OK_UPDATE;
break;
+ case CMD_UNKNOWN:
+ case CMD_UTILITY:
+ case CMD_NOTHING:
default:
- return SPI_ERROR_OPUNKNOWN;
+ elog(ERROR, "Unsupported command type %d encountered while executing query", (int)operation);
}
#ifdef SPI_EXECUTOR_STATS
diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h
index 575978737f..ac34bb3b5d 100644
--- a/src/include/executor/spi.h
+++ b/src/include/executor/spi.h
@@ -38,7 +38,7 @@ typedef struct _SPI_plan *SPIPlanPtr;
#define SPI_ERROR_CONNECT (-1) /* not used anymore */
#define SPI_ERROR_COPY (-2)
-#define SPI_ERROR_OPUNKNOWN (-3)
+#define SPI_ERROR_OPUNKNOWN (-3) /* not used anymore */
#define SPI_ERROR_UNCONNECTED (-4)
#define SPI_ERROR_CURSOR (-5) /* not used anymore */
#define SPI_ERROR_ARGUMENT (-6)
--
2.20.1
