Harden DELETE/UPDATE trigger edge cases and expand test coverage

src/60createtable.js

@@ -540,7 +540,8 @@ yy.CreateTable.prototype.execute = function (databaseid, params, cb) {
 				alasql.executeTrigger(trigger, databaseid, r);
 			}
 		}
-		if (escape) return;
+		// Return false so callers keep the row (INSTEAD OF replaces the delete)
+		if (escape) return false;
 
 		if (this.pk) {
 			var pk = this.pk;
@@ -577,7 +578,8 @@ yy.CreateTable.prototype.execute = function (databaseid, params, cb) {
 
 	table.update = function (assignfn, i, params) {
 		// TODO: Analyze the speed
-		var r = cloneDeep(this.data[i]);
+		var oldRow = this.data[i];
+		var r = cloneDeep(oldRow);
 
 		var pk;
 		// PART 1 - PRECHECK
@@ -624,7 +626,7 @@ yy.CreateTable.prototype.execute = function (databaseid, params, cb) {
 		for (var tr in table.beforeupdate) {
 			var trigger = table.beforeupdate[tr];
 			if (trigger) {
-				if (alasql.executeTrigger(trigger, databaseid, this.data[i], r) === false) {
+				if (alasql.executeTrigger(trigger, databaseid, oldRow, r) === false) {
 					prevent = prevent || true;
 				}
 			}
@@ -637,7 +639,7 @@ yy.CreateTable.prototype.execute = function (databaseid, params, cb) {
 			escape = true;
 			var trigger = table.insteadofupdate[tr];
 			if (trigger) {
-				alasql.executeTrigger(trigger, databaseid, this.data[i], r);
+				alasql.executeTrigger(trigger, databaseid, oldRow, r);
 			}
 		}
 		if (escape) return;
@@ -688,11 +690,11 @@ yy.CreateTable.prototype.execute = function (databaseid, params, cb) {
 
 		this.data[i] = r;
 
-		// Trigger prevent functionality
+		// AFTER UPDATE: pass original old row and the new row
 		for (var tr in table.afterupdate) {
 			var trigger = table.afterupdate[tr];
 			if (trigger) {
-				alasql.executeTrigger(trigger, databaseid, this.data[i], r);
+				alasql.executeTrigger(trigger, databaseid, oldRow, r);
 			}
 		}
 	};

src/72delete.js

@@ -88,15 +88,19 @@ yy.Delete.prototype.compile = function (databaseid) {
 			var deletedRows = [];
 			for (var i = 0, ilen = table.data.length; i < ilen; i++) {
 				if (wherefn(table.data[i], params, alasql)) {
-					// Track deleted row for OUTPUT clause and AFTER DELETE trigger
-					if (self.output || table.afterdelete) {
-						deletedRows.push(cloneDeep(table.data[i]));
-					}
-					// Check for transaction - if it is not possible then return all back
+					// table.delete runs BEFORE/INSTEAD OF triggers and cleans indexes.
+					// false means prevent or INSTEAD OF — keep the row.
+					var deleted = true;
 					if (table.delete) {
-						table.delete(i, params, alasql);
+						deleted = table.delete(i, params, alasql) !== false;
+					}
+					if (deleted) {
+						// Track deleted row for OUTPUT clause and AFTER DELETE trigger
+						if (self.output || table.afterdelete) {
+							deletedRows.push(cloneDeep(table.data[i]));
+						}
 					} else {
-						// Simply do not push
+						newtable.push(table.data[i]);
 					}
 				} else {
 					newtable.push(table.data[i]);

test/test1119.js

@@ -153,4 +153,85 @@ describe('Test 1119 - Trigger callback parameter', function () {
 
 		delete alasql.fn.onchangeInsteadInsert;
 	});
+
+	it('F) AFTER UPDATE trigger callback should receive old and new row data', function () {
+		let triggerReceivedCorrectData = false;
+		let receivedOldValue = undefined;
+		let receivedNewValue = undefined;
+
+		alasql.fn.onchangeAfterUpdate = function (oldRow, newRow) {
+			receivedOldValue = oldRow;
+			receivedNewValue = newRow;
+			if (oldRow && oldRow.a === 10 && newRow && newRow.a === 20) {
+				triggerReceivedCorrectData = true;
+			}
+		};
+
+		alasql('CREATE TABLE six (a INT)');
+		alasql('INSERT INTO six VALUES (10)');
+		alasql('CREATE TRIGGER seven AFTER UPDATE ON six CALL onchangeAfterUpdate()');
+		alasql('UPDATE six SET a = 20 WHERE a = 10');
+
+		assert(
+			triggerReceivedCorrectData,
+			'AFTER UPDATE trigger function did not receive the expected data. Received old: ' +
+				JSON.stringify(receivedOldValue) +
+				', new: ' +
+				JSON.stringify(receivedNewValue)
+		);
+
+		delete alasql.fn.onchangeAfterUpdate;
+	});
+
+	it('G) BEFORE DELETE returning false should prevent deletion', function () {
+		alasql.fn.preventDelete = function (r) {
+			if (r && r.a === 1) return false;
+		};
+
+		alasql('CREATE TABLE seven (a INT)');
+		alasql('INSERT INTO seven VALUES (1), (2)');
+		alasql('CREATE TRIGGER eight BEFORE DELETE ON seven CALL preventDelete()');
+		alasql('DELETE FROM seven WHERE a = 1');
+
+		const res = alasql('SELECT * FROM seven ORDER BY a');
+		assert.deepStrictEqual(
+			res,
+			[{a: 1}, {a: 2}],
+			'Row should not be deleted when BEFORE DELETE returns false'
+		);
+
+		delete alasql.fn.preventDelete;
+	});
+
+	it('H) INSTEAD OF DELETE should keep the row and still receive it', function () {
+		let triggerReceivedCorrectData = false;
+		let receivedValue = undefined;
+
+		alasql.fn.onchangeInsteadDelete = function (r) {
+			receivedValue = r;
+			if (r && r.a === 3) {
+				triggerReceivedCorrectData = true;
+			}
+		};
+
+		alasql('CREATE TABLE eight (a INT)');
+		alasql('INSERT INTO eight VALUES (3), (4)');
+		alasql('CREATE TRIGGER nine INSTEAD OF DELETE ON eight CALL onchangeInsteadDelete()');
+		alasql('DELETE FROM eight WHERE a = 3');
+
+		assert(
+			triggerReceivedCorrectData,
+			'INSTEAD OF DELETE trigger function did not receive the expected data. Received: ' +
+				JSON.stringify(receivedValue)
+		);
+
+		const res = alasql('SELECT * FROM eight ORDER BY a');
+		assert.deepStrictEqual(
+			res,
+			[{a: 3}, {a: 4}],
+			'Data should not have been deleted with INSTEAD OF trigger'
+		);
+
+		delete alasql.fn.onchangeInsteadDelete;
+	});
 });