A lot of semi-internal code just prints out numeric SPI error codes,
which is not very helpful. We already have an API function
SPI_result_code_string() to convert the codes to a string, so here is a
patch to make more use of that and also document it for external use.
Also included are two patches to clarify that some RI error handling
code that I encountered at the same time.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From fa2783e7c263addd68ced77bcfbad0a10a3117ef Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Wed, 30 Aug 2017 22:16:50 -0400
Subject: [PATCH 1/3] Move SPI error reporting out of ri_ReportViolation()
These are two completely unrelated code paths, so it doesn't make sense
to pack them into one function.
---
src/backend/utils/adt/ri_triggers.c | 29 +++++++++++------------------
1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/src/backend/utils/adt/ri_triggers.c
b/src/backend/utils/adt/ri_triggers.c
index c2891e6fa1..014b204439 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -242,7 +242,7 @@ static void ri_ExtractValues(Relation rel, HeapTuple tup,
static void ri_ReportViolation(const RI_ConstraintInfo *riinfo,
Relation pk_rel, Relation fk_rel,
HeapTuple violator, TupleDesc tupdesc,
- int queryno, bool spi_err);
+ int queryno);
/* ----------
@@ -2499,7 +2499,7 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel,
Relation pk_rel)
ri_ReportViolation(&fake_riinfo,
pk_rel, fk_rel,
tuple, tupdesc,
- RI_PLAN_CHECK_LOOKUPPK,
false);
+ RI_PLAN_CHECK_LOOKUPPK);
}
if (SPI_finish() != SPI_OK_FINISH)
@@ -3147,11 +3147,13 @@ ri_PerformCheck(const RI_ConstraintInfo *riinfo,
elog(ERROR, "SPI_execute_snapshot returned %d", spi_result);
if (expect_OK >= 0 && spi_result != expect_OK)
- ri_ReportViolation(riinfo,
- pk_rel, fk_rel,
- new_tuple ? new_tuple :
old_tuple,
- NULL,
- qkey->constr_queryno, true);
+ ereport(ERROR,
+ (errcode(ERRCODE_INTERNAL_ERROR),
+ errmsg("referential integrity query on \"%s\"
from constraint \"%s\" on \"%s\" gave unexpected result",
+ RelationGetRelationName(pk_rel),
+ NameStr(riinfo->conname),
+
RelationGetRelationName(fk_rel)),
+ errhint("This is most likely due to a rule
having rewritten the query.")));
/* XXX wouldn't it be clearer to do this part at the caller? */
if (qkey->constr_queryno != RI_PLAN_CHECK_LOOKUPPK_FROM_PK &&
@@ -3161,7 +3163,7 @@ ri_PerformCheck(const RI_ConstraintInfo *riinfo,
pk_rel, fk_rel,
new_tuple ? new_tuple :
old_tuple,
NULL,
- qkey->constr_queryno, false);
+ qkey->constr_queryno);
return SPI_processed != 0;
}
@@ -3205,7 +3207,7 @@ static void
ri_ReportViolation(const RI_ConstraintInfo *riinfo,
Relation pk_rel, Relation fk_rel,
HeapTuple violator, TupleDesc tupdesc,
- int queryno, bool spi_err)
+ int queryno)
{
StringInfoData key_names;
StringInfoData key_values;
@@ -3216,15 +3218,6 @@ ri_ReportViolation(const RI_ConstraintInfo *riinfo,
AclResult aclresult;
bool has_perm = true;
- if (spi_err)
- ereport(ERROR,
- (errcode(ERRCODE_INTERNAL_ERROR),
- errmsg("referential integrity query on \"%s\"
from constraint \"%s\" on \"%s\" gave unexpected result",
- RelationGetRelationName(pk_rel),
- NameStr(riinfo->conname),
-
RelationGetRelationName(fk_rel)),
- errhint("This is most likely due to a rule
having rewritten the query.")));
-
/*
* Determine which relation to complain about. If tupdesc wasn't passed
* by caller, assume the violator tuple came from there.
base-commit: 04e9678614ec64ad9043174ac99a25b1dc45233a
--
2.14.1
From 88b79567b53efe7b8394a802f54b4c133161f141 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Wed, 30 Aug 2017 22:16:50 -0400
Subject: [PATCH 2/3] Add attribute noreturn
---
src/backend/utils/adt/ri_triggers.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/utils/adt/ri_triggers.c
b/src/backend/utils/adt/ri_triggers.c
index 014b204439..b63a7775b7 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -242,7 +242,7 @@ static void ri_ExtractValues(Relation rel, HeapTuple tup,
static void ri_ReportViolation(const RI_ConstraintInfo *riinfo,
Relation pk_rel, Relation fk_rel,
HeapTuple violator, TupleDesc tupdesc,
- int queryno);
+ int queryno) pg_attribute_noreturn();
/* ----------
--
2.14.1
From fb6b1d8c409f9a6cfd8876f90e4b1929c45882cb Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Wed, 30 Aug 2017 22:16:50 -0400
Subject: [PATCH 3/3] Document and use SPI_result_code_string()
A lot of semi-internal code just prints out numeric SPI error codes,
which is not very helpful. We already have an API function to convert
the codes to a string, so let's make more use of that.
---
contrib/spi/refint.c | 6 ++---
contrib/spi/timetravel.c | 2 +-
doc/src/sgml/spi.sgml | 53 +++++++++++++++++++++++++++++++++++++
src/backend/utils/adt/ri_triggers.c | 10 +++----
src/test/regress/regress.c | 2 +-
5 files changed, 63 insertions(+), 10 deletions(-)
diff --git a/contrib/spi/refint.c b/contrib/spi/refint.c
index 46205c7613..85cadda8a3 100644
--- a/contrib/spi/refint.c
+++ b/contrib/spi/refint.c
@@ -182,7 +182,7 @@ check_primary_key(PG_FUNCTION_ARGS)
pplan = SPI_prepare(sql, nkeys, argtypes);
if (pplan == NULL)
/* internal error */
- elog(ERROR, "check_primary_key: SPI_prepare returned
%d", SPI_result);
+ elog(ERROR, "check_primary_key: SPI_prepare returned
%s", SPI_result_code_string(SPI_result));
/*
* Remember that SPI_prepare places plan in current memory
context -
@@ -395,7 +395,7 @@ check_foreign_key(PG_FUNCTION_ARGS)
/* this shouldn't happen! SPI_ERROR_NOOUTFUNC ? */
if (oldval == NULL)
/* internal error */
- elog(ERROR, "check_foreign_key: SPI_getvalue
returned %d", SPI_result);
+ elog(ERROR, "check_foreign_key: SPI_getvalue
returned %s", SPI_result_code_string(SPI_result));
newval = SPI_getvalue(newtuple, tupdesc, fnumber);
if (newval == NULL || strcmp(oldval, newval) != 0)
isequal = false;
@@ -529,7 +529,7 @@ check_foreign_key(PG_FUNCTION_ARGS)
pplan = SPI_prepare(sql, nkeys, argtypes);
if (pplan == NULL)
/* internal error */
- elog(ERROR, "check_foreign_key: SPI_prepare
returned %d", SPI_result);
+ elog(ERROR, "check_foreign_key: SPI_prepare
returned %s", SPI_result_code_string(SPI_result));
/*
* Remember that SPI_prepare places plan in current
memory context
diff --git a/contrib/spi/timetravel.c b/contrib/spi/timetravel.c
index 2c66d888df..04c79273df 100644
--- a/contrib/spi/timetravel.c
+++ b/contrib/spi/timetravel.c
@@ -341,7 +341,7 @@ timetravel(PG_FUNCTION_ARGS)
/* Prepare plan for query */
pplan = SPI_prepare(sql, natts, ctypes);
if (pplan == NULL)
- elog(ERROR, "timetravel (%s): SPI_prepare returned %d",
relname, SPI_result);
+ elog(ERROR, "timetravel (%s): SPI_prepare returned %s",
relname, SPI_result_code_string(SPI_result));
/*
* Remember that SPI_prepare places plan in current memory
context -
diff --git a/doc/src/sgml/spi.sgml b/doc/src/sgml/spi.sgml
index 86be87c0fd..b9e2270514 100644
--- a/doc/src/sgml/spi.sgml
+++ b/doc/src/sgml/spi.sgml
@@ -3546,6 +3546,59 @@ <title>Return Value</title>
</refsect1>
</refentry>
+<refentry id="spi-spi-result-code-string">
+ <indexterm><primary>SPI_result_code_string</primary></indexterm>
+
+ <refmeta>
+ <refentrytitle>SPI_result_code_string</refentrytitle>
+ <manvolnum>3</manvolnum>
+ </refmeta>
+
+ <refnamediv>
+ <refname>SPI_result_code_string</refname>
+ <refpurpose>return error code as string</refpurpose>
+ </refnamediv>
+
+ <refsynopsisdiv>
+<synopsis>
+const char * SPI_result_code_string(int <parameter>code</parameter>);
+</synopsis>
+ </refsynopsisdiv>
+
+ <refsect1>
+ <title>Description</title>
+
+ <para>
+ <function>SPI_result_code_string</function> returns a string representation
+ of the result code returned by various SPI functions or stored
+ in <varname>SPI_result</varname>.
+ </para>
+ </refsect1>
+
+ <refsect1>
+ <title>Arguments</title>
+
+ <variablelist>
+ <varlistentry>
+ <term><literal>int <parameter>code</parameter></literal></term>
+ <listitem>
+ <para>
+ result code
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </refsect1>
+
+ <refsect1>
+ <title>Return Value</title>
+
+ <para>
+ A string representation of the result code.
+ </para>
+ </refsect1>
+</refentry>
+
</sect1>
<sect1 id="spi-memory">
diff --git a/src/backend/utils/adt/ri_triggers.c
b/src/backend/utils/adt/ri_triggers.c
index b63a7775b7..ba0e3ad87d 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -2435,8 +2435,8 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel,
Relation pk_rel)
qplan = SPI_prepare(querybuf.data, 0, NULL);
if (qplan == NULL)
- elog(ERROR, "SPI_prepare returned %d for %s",
- SPI_result, querybuf.data);
+ elog(ERROR, "SPI_prepare returned %s for %s",
+ SPI_result_code_string(SPI_result), querybuf.data);
/*
* Run the plan. For safety we force a current snapshot to be used. (In
@@ -2453,7 +2453,7 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel,
Relation pk_rel)
/* Check result */
if (spi_result != SPI_OK_SELECT)
- elog(ERROR, "SPI_execute_snapshot returned %d", spi_result);
+ elog(ERROR, "SPI_execute_snapshot returned %s",
SPI_result_code_string(spi_result));
/* Did we find a tuple violating the constraint? */
if (SPI_processed > 0)
@@ -3016,7 +3016,7 @@ ri_PlanCheck(const char *querystr, int nargs, Oid
*argtypes,
qplan = SPI_prepare(querystr, nargs, argtypes);
if (qplan == NULL)
- elog(ERROR, "SPI_prepare returned %d for %s", SPI_result,
querystr);
+ elog(ERROR, "SPI_prepare returned %s for %s",
SPI_result_code_string(SPI_result), querystr);
/* Restore UID and security context */
SetUserIdAndSecContext(save_userid, save_sec_context);
@@ -3144,7 +3144,7 @@ ri_PerformCheck(const RI_ConstraintInfo *riinfo,
/* Check result */
if (spi_result < 0)
- elog(ERROR, "SPI_execute_snapshot returned %d", spi_result);
+ elog(ERROR, "SPI_execute_snapshot returned %s",
SPI_result_code_string(spi_result));
if (expect_OK >= 0 && spi_result != expect_OK)
ereport(ERROR,
diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c
index 3d33b36e66..7793165d24 100644
--- a/src/test/regress/regress.c
+++ b/src/test/regress/regress.c
@@ -614,7 +614,7 @@ ttdummy(PG_FUNCTION_ARGS)
/* Prepare plan for query */
pplan = SPI_prepare(query, natts, ctypes);
if (pplan == NULL)
- elog(ERROR, "ttdummy (%s): SPI_prepare returned %d",
relname, SPI_result);
+ elog(ERROR, "ttdummy (%s): SPI_prepare returned %s",
relname, SPI_result_code_string(SPI_result));
if (SPI_keepplan(pplan))
elog(ERROR, "ttdummy (%s): SPI_keepplan failed",
relname);
--
2.14.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
