Re: [HACKERS] Repetitive code in RI triggers

[Ildar Musin]

Hi Maksim,

On 26.09.2017 11:51, Maksim Milyutin wrote:

On 19.09.2017 11:09, Daniel Gustafsson wrote:


Removing reviewer Maksim Milyutin from patch entry due to inactivity and
community account email bouncing.  Maksim: if you are indeed reviewing
this
patch, then please update the community account and re-add to the
patch entry.

cheers ./daniel


Daniel, thanks for noticing. I have updated my account and re-added to
the patch entry.

Ildar, your patch is conflicting with the current HEAD of master branch,
please update it.



Thank you for checking the patch out. Yes, it seems that original code 
was reformatted and this led to merging conflicts. I've fixed that and 
also introduced some minor improvements. The new version is in attachment.


Thanks!

--
Ildar Musin
i.mu...@postgrespro.ru
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index c2891e6..25cdf7d 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -196,8 +196,9 @@ static int	ri_constraint_cache_valid_count = 0;
 static bool ri_Check_Pk_Match(Relation pk_rel, Relation fk_rel,
   HeapTuple old_row,
   const RI_ConstraintInfo *riinfo);
-static Datum ri_restrict_del(TriggerData *trigdata, bool is_no_action);
-static Datum ri_restrict_upd(TriggerData *trigdata, bool is_no_action);
+static Datum ri_restrict(TriggerData *trigdata, bool is_no_action);
+static Datum ri_setnull(TriggerData *trigdata);
+static Datum ri_setdefault(FunctionCallInfo fcinfo);
 static void quoteOneName(char *buffer, const char *name);
 static void quoteRelationName(char *buffer, Relation rel);
 static void ri_GenerateQual(StringInfo buf,
@@ -605,7 +606,7 @@ RI_FKey_noaction_del(PG_FUNCTION_ARGS)
 	/*
 	 * Share code with RESTRICT case.
 	 */
-	return ri_restrict_del((TriggerData *) fcinfo->context, true);
+	return ri_restrict((TriggerData *) fcinfo->context, true);
 }
 
 /* --
@@ -630,175 +631,10 @@ RI_FKey_restrict_del(PG_FUNCTION_ARGS)
 	/*
 	 * Share code with NO ACTION case.
 	 */
-	return ri_restrict_del((TriggerData *) fcinfo->context, false);
+	return ri_restrict((TriggerData *) fcinfo->context, false);
 }
 
 /* --
- * ri_restrict_del -
- *
- *	Common code for ON DELETE RESTRICT and ON DELETE NO ACTION.
- * --
- */
-static Datum
-ri_restrict_del(TriggerData *trigdata, bool is_no_action)
-{
-	const RI_ConstraintInfo *riinfo;
-	Relation	fk_rel;
-	Relation	pk_rel;
-	HeapTuple	old_row;
-	RI_QueryKey qkey;
-	SPIPlanPtr	qplan;
-	int			i;
-
-	/*
-	 * Get arguments.
-	 */
-	riinfo = ri_FetchConstraintInfo(trigdata->tg_trigger,
-	trigdata->tg_relation, true);
-
-	/*
-	 * Get the relation descriptors of the FK and PK tables and the old tuple.
-	 *
-	 * fk_rel is opened in RowShareLock mode since that's what our eventual
-	 * SELECT FOR KEY SHARE will get on it.
-	 */
-	fk_rel = heap_open(riinfo->fk_relid, RowShareLock);
-	pk_rel = trigdata->tg_relation;
-	old_row = trigdata->tg_trigtuple;
-
-	switch (riinfo->confmatchtype)
-	{
-			/* --
-			 * SQL:2008 15.17 
-			 *	General rules 9) a) iv):
-			 *		MATCH SIMPLE/FULL
-			 *			... ON DELETE RESTRICT
-			 * --
-			 */
-		case FKCONSTR_MATCH_SIMPLE:
-		case FKCONSTR_MATCH_FULL:
-			switch (ri_NullCheck(old_row, riinfo, true))
-			{
-case RI_KEYS_ALL_NULL:
-case RI_KEYS_SOME_NULL:
-
-	/*
-	 * No check needed - there cannot be any reference to old
-	 * key if it contains a NULL
-	 */
-	heap_close(fk_rel, RowShareLock);
-	return PointerGetDatum(NULL);
-
-case RI_KEYS_NONE_NULL:
-
-	/*
-	 * Have a full qualified key - continue below
-	 */
-	break;
-			}
-
-			/*
-			 * If another PK row now exists providing the old key values, we
-			 * should not do anything.  However, this check should only be
-			 * made in the NO ACTION case; in RESTRICT cases we don't wish to
-			 * allow another row to be substituted.
-			 */
-			if (is_no_action &&
-ri_Check_Pk_Match(pk_rel, fk_rel, old_row, riinfo))
-			{
-heap_close(fk_rel, RowShareLock);
-return PointerGetDatum(NULL);
-			}
-
-			if (SPI_connect() != SPI_OK_CONNECT)
-elog(ERROR, "SPI_connect failed");
-
-			/*
-			 * Fetch or prepare a saved plan for the restrict delete lookup
-			 */
-			ri_BuildQueryKey(, riinfo, RI_PLAN_RESTRICT_DEL_CHECKREF);
-
-			if ((qplan = ri_FetchPreparedPlan()) == NULL)
-			{
-StringInfoData querybuf;
-char		fkrelname[MAX_QUOTED_REL_NAME_LEN];
-char		attname[MAX_QUOTED_NAME_LEN];
-char		paramname[16];
-const char *querysep;
-Oid			queryoids[RI_MAX_NUMKEYS];
-
-/* --
- * The query string built is
- *	SELECT 1 FROM ONLY  x WHERE $1 = fkatt1 [AND ...]
- *		   FOR KEY SHARE OF x
- * The type id's for the $ parameters are those of the
- * corresponding PK attributes.
- * --
- */
-initStringInfo();
-quoteRelationName(fkrelname, fk_rel);
-appendStringInfo(, "SELECT 1 FROM ONLY %s x",
-	

Re: [HACKERS] Repetitive code in RI triggers

[Daniel Gustafsson]

> On 26 Sep 2017, at 10:51, Maksim Milyutin  wrote:
> 
> On 19.09.2017 11:09, Daniel Gustafsson wrote:
> 
>> Removing reviewer Maksim Milyutin from patch entry due to inactivity and
>> community account email bouncing.  Maksim: if you are indeed reviewing this
>> patch, then please update the community account and re-add to the patch 
>> entry.
>> 
>> cheers ./daniel
> 
> Daniel, thanks for noticing. I have updated my account and re-added to the 
> patch entry.

Great, thanks!

> Ildar, your patch is conflicting with the current HEAD of master branch, 
> please update it.

I’ve changed status to Waiting on Author based on this.

cheers ./daniel

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Repetitive code in RI triggers

[Maksim Milyutin]

On 19.09.2017 11:09, Daniel Gustafsson wrote:


Removing reviewer Maksim Milyutin from patch entry due to inactivity and
community account email bouncing.  Maksim: if you are indeed reviewing this
patch, then please update the community account and re-add to the patch entry.

cheers ./daniel


Daniel, thanks for noticing. I have updated my account and re-added to 
the patch entry.


Ildar, your patch is conflicting with the current HEAD of master branch, 
please update it.


--
Regards,
Maksim Milyutin



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Repetitive code in RI triggers

[Daniel Gustafsson]

> On 11 Apr 2017, at 03:41, Peter Eisentraut  
> wrote:
> 
> On 4/10/17 11:55, Ildar Musin wrote:
>> I was looking through the RI triggers code recently and noticed a few 
>> almost identical functions, e.g. ri_restrict_upd() and 
>> ri_restrict_del(). The following patch is an attempt to reduce some of 
>> repetitive code.
> 
> That looks like something worth pursuing.  Please add it to the next
> commit fest.

Removing reviewer Maksim Milyutin from patch entry due to inactivity and
community account email bouncing.  Maksim: if you are indeed reviewing this
patch, then please update the community account and re-add to the patch entry.

cheers ./daniel

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Repetitive code in RI triggers

[Peter Eisentraut]

On 4/10/17 11:55, Ildar Musin wrote:
> I was looking through the RI triggers code recently and noticed a few 
> almost identical functions, e.g. ri_restrict_upd() and 
> ri_restrict_del(). The following patch is an attempt to reduce some of 
> repetitive code.

That looks like something worth pursuing.  Please add it to the next
commit fest.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers