On Fri, Feb 22, 2019 at 1:12 PM Corey Huinker
wrote:
> On Fri, Feb 22, 2019 at 11:05 AM Peter Eisentraut <
> peter.eisentr...@2ndquadrant.com> wrote:
>
>> ri_triggers.c is endlessly long and repetitive. I want to clean it up a
>> bit (more).
>>
>
> Having just been down this road, I agree that a lot of cleanup is needed
> and possible.
>
>
>> I looked into all these switch cases for the unimplemented MATCH PARTIAL
>> option. I toyed around with how a MATCH PARTIAL implementation would
>> actually look like, and it likely wouldn't use the existing code
>> structure anyway, so let's just simplify this for now.
>>
>
> +1
>
>
>
>> Attached are some patches.
>
>
> I intend to look this over in much greater detail, but I did skim the code
> and it seems like you left the SET DEFAULT and SET NULL paths separate. In
> my attempt at statement level triggers I realized that they only differed
> by the one literal value, and parameterized the function.
>
>
I've looked it over more closely now and I think that it's a nice
improvement.
As I suspected, the code for SET NULL and SET DEFAULT are highly similar
(see .diff), the major difference being two constants, the order of some
variable declarations, and the recheck in the set-default case.
The changes were so simple that I felt remiss not adding the patch for you
(see .patch).
Passes make check.
diff --git a/set_null.c b/set_default.c
index bc323ec..b2dd91d 100644
--- a/set_null.c
+++ b/set_default.c
@@ -1,10 +1,10 @@
/*
- * ri_setnull -
+ * ri_setdefault -
*
- * Common code for ON DELETE SET NULL and ON UPDATE SET NULL
+ * Common code for ON DELETE SET DEFAULT and ON UPDATE SET DEFAULT
*/
static Datum
-ri_setnull(TriggerData *trigdata)
+ri_setdefault(TriggerData *trigdata)
{
const RI_ConstraintInfo *riinfo;
Relationfk_rel;
@@ -30,10 +30,10 @@ ri_setnull(TriggerData *trigdata)
elog(ERROR, "SPI_connect failed");
/*
- * Fetch or prepare a saved plan for the set null operation (it's
- * the same query for delete and update cases)
+ * Fetch or prepare a saved plan for the set default operation
+ * (it's the same query for delete and update cases)
*/
-ri_BuildQueryKey(, riinfo, RI_PLAN_SETNULL_DOUPDATE);
+ri_BuildQueryKey(, riinfo, RI_PLAN_SETDEFAULT_DOUPDATE);
if ((qplan = ri_FetchPreparedPlan()) == NULL)
{
@@ -44,12 +44,12 @@ ri_setnull(TriggerData *trigdata)
charparamname[16];
const char *querysep;
const char *qualsep;
-const char *fk_only;
Oid queryoids[RI_MAX_NUMKEYS];
+const char *fk_only;
/* --
* The query string built is
- * UPDATE [ONLY] SET fkatt1 = NULL [, ...]
+ * UPDATE [ONLY] SET fkatt1 = DEFAULT [, ...]
* WHERE $1 = fkatt1 [AND ...]
* The type id's for the $ parameters are those of the
* corresponding PK attributes.
@@ -57,9 +57,9 @@ ri_setnull(TriggerData *trigdata)
*/
initStringInfo();
initStringInfo();
+quoteRelationName(fkrelname, fk_rel);
fk_only = fk_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ?
"" : "ONLY ";
-quoteRelationName(fkrelname, fk_rel);
appendStringInfo(, "UPDATE %s%s SET",
fk_only, fkrelname);
querysep = "";
@@ -72,9 +72,10 @@ ri_setnull(TriggerData *trigdata)
quoteOneName(attname,
RIAttName(fk_rel, riinfo->fk_attnums[i]));
appendStringInfo(,
- "%s %s = NULL",
+ "%s %s = DEFAULT",
querysep, attname);
sprintf(paramname, "$%d", i + 1);
+sprintf(paramname, "$%d", i + 1);
ri_GenerateQual(, qualsep,
paramname, pk_type,
riinfo->pf_eq_oprs[i],
@@ -104,5 +105,20 @@ ri_setnull(TriggerData *trigdata)
table_close(fk_rel, RowExclusiveLock);
-return PointerGetDatum(NULL);
+/*
+ * If we just deleted or updated the PK row whose key was equal to
+ * the FK columns' default values, and a referencing row exists in
+ * the FK table, we would have updated that row to the same values
+ * it already had --- and RI_FKey_fk_upd_check_required would
+ * hence believe no check is necessary. So we need to do another
+ * lookup now and in case a reference still exists, abort the
+ * operation. That is already implemented in the NO ACTION
+ * trigger, so just run it. (This recheck is only needed in the
+ * SET DEFAULT case, since CASCADE would remove such rows in case
+ * of a DELETE operation or would change the FK key values in case
+ * of an UPDATE, while SET NULL is certain to result in rows that
+ * satisfy the FK constraint.)
+ */
+return ri_restrict(trigdata, true);
}
+
From
