Merge #152874 #152879

152874: workload: add test to activerecord ignore list r=spilchen a=spilchen

The test test_disconnect_and_clear_reloadable_connections_are_able_to_preempt_other_waiting_threads has been observed to fail intermittently due to timing-related issues.

Closes #152653

Release note: none
Epic: none

152879: workload/schemachange: handle triggers in NULL constraint validation r=spilchen a=spilchen

Previously, the schemachange workload would expect a NOT NULL violation error when inserting NULL values into NOT NULL columns. However, if a table has a BEFORE INSERT trigger that returns NULL, the insertion is silently cancelled without raising any error, causing test failures.

This change detects when a table has BEFORE INSERT triggers and treats NOT NULL violations as potential errors rather than expected errors in such cases.

Fixes #152815

Release note: None
Epic: None

Co-authored-by: Matt Spilchen <[email protected]>

Author: craig[bot]

pkg/cmd/roachtest/tests/activerecord_blocklist.go

@@ -26,6 +26,7 @@ var activeRecordIgnoreList = blocklist{
 	`ActiveRecord::CockroachDBStructureDumpTest#test_structure_dump`:                                                                                           "flaky",
 	`ActiveRecord::ConnectionAdapters::ConnectionPoolThreadTest#test_checkout_fairness`:                                                                        "flaky",
 	`ActiveRecord::ConnectionAdapters::ConnectionPoolThreadTest#test_checkout_fairness_by_group`:                                                               "flaky",
+	`ActiveRecord::ConnectionAdapters::ConnectionPoolThreadTest#test_disconnect_and_clear_reloadable_connections_are_able_to_preempt_other_waiting_threads`:    "flaky",
 	`ActiveRecord::ConnectionAdapters::PostgreSQLAdapterTest#test_translate_no_connection_exception_to_not_established`:                                        "pg_terminate_backend not implemented",
 	`ActiveRecord::Encryption::EncryptableRecordTest#test_by_default,_it's_case_sensitive`:                                                                     "flaky",
 	`ActiveRecord::Encryption::EncryptableRecordTest#test_forced_encoding_for_deterministic_attributes_will_replace_invalid_characters`:                        "flaky",

pkg/workload/schemachange/error_screening.go

@@ -494,6 +494,28 @@ func isValidGenerationError(code string) bool {
 	return getValidGenerationErrors().contains(pgCode)
 }
 
+// tableHasBeforeInsertTrigger checks if the table has any BEFORE INSERT triggers
+// that could affect the insertion behavior.
+func (og *operationGenerator) tableHasBeforeInsertTrigger(
+	ctx context.Context, tx pgx.Tx, tableName *tree.TableName,
+) (bool, error) {
+	query := `
+	SELECT EXISTS (
+		SELECT 1 FROM pg_trigger t
+		JOIN pg_class c ON t.tgrelid = c.oid
+		JOIN pg_namespace n ON c.relnamespace = n.oid
+		WHERE n.nspname = $1 
+		AND c.relname = $2
+		AND t.tgenabled != 'D'
+		AND (t.tgtype & 2) = 2  -- BEFORE trigger
+		AND (t.tgtype & 4) = 4  -- INSERT trigger
+	)`
+
+	var hasTrigger bool
+	err := tx.QueryRow(ctx, query, tableName.Schema(), tableName.Object()).Scan(&hasTrigger)
+	return hasTrigger, err
+}
+
 // validateGeneratedExpressionsForInsert goes through generated expressions and
 // detects if a valid value can be generated with a given insert row.
 func (og *operationGenerator) validateGeneratedExpressionsForInsert(
@@ -514,6 +536,12 @@ func (og *operationGenerator) validateGeneratedExpressionsForInsert(
 			isInvalidInsert = true
 		}
 	}()
+
+	hasBeforeInsertTrigger, err := og.tableHasBeforeInsertTrigger(ctx, tx, tableName)
+	if err != nil {
+		return false, nil, nil, err
+	}
+
 	// Put values to be inserted into a column name to value map to simplify lookups.
 	columnsToValues := map[tree.Name]string{}
 	for i := 0; i < len(nonGeneratedColNames); i++ {
@@ -601,7 +629,14 @@ func (og *operationGenerator) validateGeneratedExpressionsForInsert(
 		}
 		if isNull && !isNullable && !nullViolationAdded {
 			nullViolationAdded = true
-			expectedErrCodes = expectedErrCodes.append(pgcode.NotNullViolation)
+			// If there is a trigger, then we cannot be sure if the NULL value
+			// will actually be inserted. It may be replaced or the row itself could
+			// be cleared.
+			if hasBeforeInsertTrigger {
+				potentialErrCodes = potentialErrCodes.append(pgcode.NotNullViolation)
+			} else {
+				expectedErrCodes = expectedErrCodes.append(pgcode.NotNullViolation)
+			}
 		}
 		// Re-run the another variant in case we have NULL values in arithmetic
 		// of expression, the evaluation order can differ depending on how variables