Front-ported pull request #8957 from FirebirdSQL/work/UpdateOverwriteMode

New setting AllowUpdateOverwrite : defines how UPDATE should handle the case when a record was updated by trigger.

Author: hvlad

builds/install/misc/firebird.conf

@@ -399,6 +399,31 @@
 #OptimizeForFirstRows = false
 
 
+# ----------------------------
+# Defines how UPDATE should handle the case when a record was already updated by 
+# trigger or by the another positional UPDATE in the same cursor.
+#
+# It is possible that BEFORE or AFTER UPDATE trigger was fired previously by the
+# current UPDATE statement for the other record and has already changed the
+# current record being updated. Due to cursor stability, UPDATE should not see
+# such changes, and could silently overwrite them.
+#
+# Setting AllowUpdateOverwrite manage how engine should handle such cases:
+# - if set to true (default), prior changes will be overwritten,
+# - if set to false, error "UPDATE will overwrite changes made by the trigger or 
+#	by the another UPDATE in the same cursor" will be raised.
+#
+# CAUTION!
+# There is no guarantee that this setting will be available in future Firebird
+# versions.
+#
+# Per-database configurable.
+#
+# Type: integer
+#
+#AllowUpdateOverwrite = true
+
+
 # ============================
 # Plugin settings
 # ============================

src/common/config/config.h

@@ -186,6 +186,7 @@ enum ConfigKey
 	KEY_PARALLEL_WORKERS,
 	KEY_MAX_PARALLEL_WORKERS,
 	KEY_OPTIMIZE_FOR_FIRST_ROWS,
+	KEY_ALLOW_UPDATE_OVERWRITE,
 	MAX_CONFIG_KEY		// keep it last
 };
 
@@ -298,7 +299,8 @@ inline constexpr ConfigEntry entries[MAX_CONFIG_KEY] =
 	{TYPE_INTEGER,	"MaxStatementCacheSize",	false,	2 * 1048576},	// bytes
 	{TYPE_INTEGER,	"ParallelWorkers",			true,	1},
 	{TYPE_INTEGER,	"MaxParallelWorkers",		true,	1},
-	{TYPE_BOOLEAN,	"OptimizeForFirstRows",		false,	false}
+	{TYPE_BOOLEAN,	"OptimizeForFirstRows",		false,	false},
+	{TYPE_BOOLEAN,	"AllowUpdateOverwrite",		false,	true}
 };
 
 
@@ -619,6 +621,8 @@ class Config : public RefCounted, public GlobalStorage
 	CONFIG_GET_GLOBAL_INT(getMaxParallelWorkers, KEY_MAX_PARALLEL_WORKERS);
 
 	CONFIG_GET_PER_DB_BOOL(getOptimizeForFirstRows, KEY_OPTIMIZE_FOR_FIRST_ROWS);
+
+	CONFIG_GET_PER_DB_BOOL(getAllowUpdateOverwrite, KEY_ALLOW_UPDATE_OVERWRITE);
 };
 
 // Implementation of interface to access master configuration file

src/include/firebird/impl/msg/jrd.h

@@ -1004,3 +1004,4 @@ FB_IMPL_MSG(JRD, 1001, sysf_argmustbe_range_inc0_1, -833, "42", "000", "Argument
 FB_IMPL_MSG(JRD, 1002, argmustbe_numeric_function, -833, "42", "000", "Argument for @1 function must be numeric types")
 FB_IMPL_MSG(JRD, 1003, percetile_only_one_sort_item, -833, "42", "000", "The PERCENTILE_DISC and PERENTILE_CONT functions support only one sort item in WITHIN GROUP")
 FB_IMPL_MSG(JRD, 1004, argmustbe_const_within_group, -833, "42", "000", "Argument for @1 function must be constant within each group")
+FB_IMPL_MSG(JRD, 1005, update_overwrite, -901, "27", "000", "UPDATE will overwrite changes made by the trigger or by the another UPDATE in the same cursor")

src/jrd/vio.cpp

@@ -178,7 +178,7 @@ static void protect_system_table_delupd(thread_db* tdbb, const jrd_rel* relation
 	bool force_flag = false);
 static void purge(thread_db*, record_param*);
 static void replace_record(thread_db*, record_param*, PageStack*, const jrd_tra*);
-static void refresh_fk_fields(thread_db*, Record*, record_param*, record_param*);
+static void refresh_changed_fields(thread_db*, Record*, record_param*, record_param*);
 static SSHORT set_metadata_id(thread_db*, Record*, USHORT, drq_type_t, const char*);
 static void set_nbackup_id(thread_db*, Record*, USHORT, drq_type_t, const char*);
 static void set_owner_name(thread_db*, Record*, USHORT);
@@ -3317,7 +3317,7 @@ bool VIO_modify(thread_db* tdbb, record_param* org_rpb, record_param* new_rpb, j
 		fb_assert(!(org_rpb->rpb_runtime_flags & RPB_undo_read));
 
 		if (undo_read)
-			refresh_fk_fields(tdbb, old_record, org_rpb, new_rpb);
+			refresh_changed_fields(tdbb, old_record, org_rpb, new_rpb);
 	}
 
 	// If we're the system transaction, modify stuff in place.  This saves
@@ -7014,83 +7014,118 @@ static void replace_record(thread_db*		tdbb,
 }
 
 
-static void refresh_fk_fields(thread_db* tdbb, Record* old_rec, record_param* cur_rpb,
+static void refresh_changed_fields(thread_db* tdbb, Record* old_rec, record_param* cur_rpb,
 	record_param* new_rpb)
 {
 /**************************************
  *
- *	r e f r e s h _ f k _ f i e l d s
+ *	r e f r e s h _ c h a n g e d _ f i e l d s
  *
  **************************************
  *
  * Functional description
  *	Update new_rpb with foreign key fields values changed by cascade triggers.
  *  Consider self-referenced foreign keys only.
+ *  Also, if AllowUpdateOverwrite is set to false, raise error when non
+ *  self-referenced foreign key fields were changed by user triggers.
  *
  *  old_rec - old record before modify
- *  cur_rpb - just read record with possibly changed FK fields
+ *  cur_rpb - just read record with possibly changed fields
  *  new_rpb - new record evaluated by modify statement and before-triggers
  *
  **************************************/
-	auto* relation = cur_rpb->rpb_relation->getPermanent();
+	const Database* dbb = tdbb->getDatabase();
+	const auto allowOverwrite = dbb->dbb_config->getAllowUpdateOverwrite();
 
-	relation->scanPartners(tdbb);
+	jrd_rel* relation = cur_rpb->rpb_relation;
+	auto* relPerm = relation->getPermanent();
 
-	const auto* frgn = relation->rel_foreign_refs;
-	if (!frgn)
-		return;
-
-	if (!frgn->getCount())
-		return;
-
-	RelationPages* relPages = cur_rpb->rpb_relation->getPages(tdbb);
+	relPerm->scanPartners(tdbb);
 
-	// Collect all fields of all foreign keys
+	// Collect all fields of self-referenced foreign keys
 	SortedArray<int, InlineStorage<int, 16> > fields;
 
-	for (auto& dep : *frgn)
+	if (const auto* frgn = relPerm->rel_foreign_refs)
 	{
-		// We need self-referenced FK's only
-		if (dep.dep_relation == relation->getId())
+		RelationPages* relPages = relation->getPages(tdbb);
+		for (auto& dep : *frgn)
 		{
-			index_desc idx;
-			idx.idx_id = idx_invalid;
-
-			if (BTR_lookup(tdbb, relation, dep.dep_reference_id, &idx, relPages))
+			if (dep.dep_relation == relPerm->getId())
 			{
-				fb_assert(idx.idx_flags & idx_foreign);
+				index_desc idx;
+				idx.idx_id = idx_invalid;
 
-				for (int fld = 0; fld < idx.idx_count; fld++)
+				if (BTR_lookup(tdbb, relPerm, dep.dep_reference_id, &idx, relPages))
 				{
-					const int fldNum = idx.idx_rpt[fld].idx_field;
-					if (!fields.exist(fldNum))
-						fields.add(fldNum);
+					fb_assert(idx.idx_flags & idx_foreign);
+
+					for (int fld = 0; fld < idx.idx_count; fld++)
+					{
+						const int fldNum = idx.idx_rpt[fld].idx_field;
+						if (!fields.exist(fldNum))
+							fields.add(fldNum);
+					}
 				}
 			}
 		}
 	}
 
 	if (fields.isEmpty())
-		return;
+	{
+		if (allowOverwrite)
+			return;
+
+		if (cur_rpb->rpb_record->getFormat()->fmt_version == old_rec->getFormat()->fmt_version)
+		{
+			if (memcmp(cur_rpb->rpb_address, old_rec->getData(), cur_rpb->rpb_length) == 0)
+				return;
 
-	DSC desc1, desc2;
-	for (FB_SIZE_T idx = 0; idx < fields.getCount(); idx++)
+			ERR_post(Arg::Gds(isc_update_overwrite));		// UPDATE will overwrite changes made by the trigger or by another UPDATE in the same cursor
+		}
+		// Else compare field-by-field
+	}
+
+	for (FB_SIZE_T fld = 0, frn = 0; fld < relation->rel_current_format->fmt_count; fld++)
 	{
-		// Detect if user changed FK field by himself.
-		const int fld = fields[idx];
-		const bool flag_old = EVL_field(cur_rpb->rpb_relation, old_rec, fld, &desc1);
-		const bool flag_new = EVL_field(cur_rpb->rpb_relation, new_rpb->rpb_record, fld, &desc2);
+		dsc dsc_old;
+		const bool flag_old = EVL_field(relation, old_rec, fld, &dsc_old);
 
-		// If field was not changed by user - pick up possible modification by
-		// system cascade trigger
-		if (flag_old == flag_new &&
-			(!flag_old || (flag_old && !MOV_compare(tdbb, &desc1, &desc2))))
+		const bool is_fk = (frn < fields.getCount() && fields[frn] == fld);
+		if (!is_fk)
 		{
-			const bool flag_tmp = EVL_field(cur_rpb->rpb_relation, cur_rpb->rpb_record, fld, &desc1);
-			if (flag_tmp)
-				MOV_move(tdbb, &desc1, &desc2);
-			else
-				new_rpb->rpb_record->setNull(fld);
+			if (allowOverwrite)
+				continue;
+
+			dsc dsc_cur;
+			const bool flag_cur = EVL_field(relation, cur_rpb->rpb_record, fld, &dsc_cur);
+
+			// Check if current record differs from old record
+			if ((flag_cur != flag_old) ||
+				(flag_cur && flag_old && MOV_compare(tdbb, &dsc_old, &dsc_cur) != 0))
+			{
+				// Record was modified by trigger.
+				ERR_post(Arg::Gds(isc_update_overwrite));		// UPDATE will overwrite changes made by the trigger or by another UPDATE in the same cursor
+			}
+		}
+		else
+		{
+			dsc dsc_new;
+			const bool flag_new = EVL_field(relation, new_rpb->rpb_record, fld, &dsc_new);
+
+			// If field was not changed by user - pick up possible modification by
+			// system cascade trigger
+			if (flag_old == flag_new &&
+				(!flag_old || (flag_old && !MOV_compare(tdbb, &dsc_old, &dsc_new))))
+			{
+				dsc dsc_cur;
+				const bool flag_cur = EVL_field(relation, cur_rpb->rpb_record, fld, &dsc_cur);
+				if (flag_cur)
+					MOV_move(tdbb, &dsc_cur, &dsc_new);
+				else
+					new_rpb->rpb_record->setNull(fld);
+			}
+
+			frn++;
 		}
 	}
 }