From dfcc817d04f874dca181c7176a13cfec3706a894 Mon Sep 17 00:00:00 2001
From: Patrick Figel <pfigel@greenpeace.org>
Date: Mon, 17 Jun 2019 23:44:58 +0200
Subject: [PATCH] Fix use of cached schema information in SchemaHandler

This fixes an issue where CRM_Core_BAO_SchemaHandler uses outdated
schema/column information from cache when rebuilding triggers. In
certain scenarios, for example when multiple custom fields are deleted
during a single script execution, this could cause generated SQL to
reference deleted columns, throwing an error.
---
 CRM/Core/BAO/SchemaHandler.php                |  4 +-
 .../CRM/Core/BAO/SchemaHandlerTest.php        | 39 +++++++++++++++++++
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/CRM/Core/BAO/SchemaHandler.php b/CRM/Core/BAO/SchemaHandler.php
index 5fe0666b15..0e21bd549b 100644
--- a/CRM/Core/BAO/SchemaHandler.php
+++ b/CRM/Core/BAO/SchemaHandler.php
@@ -77,7 +77,7 @@ class CRM_Core_BAO_SchemaHandler {
     }
 
     // always do a trigger rebuild for this table
-    CRM_Core_DAO::triggerRebuild($params['name']);
+    Civi::service('sql_triggers')->rebuild($params['name'], TRUE);
 
     return TRUE;
   }
@@ -347,7 +347,7 @@ ALTER TABLE {$tableName}
     }
 
     if ($triggerRebuild) {
-      CRM_Core_DAO::triggerRebuild($params['table_name']);
+      Civi::service('sql_triggers')->rebuild($params['table_name'], TRUE);
     }
 
     return TRUE;
diff --git a/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php b/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php
index ca5345dd51..a504f8f437 100644
--- a/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php
+++ b/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php
@@ -297,4 +297,43 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase {
     $this->assertEquals($indices['two']['sig'], 'my_table::0::title');
   }
 
+  /**
+   * Test that columns are dropped
+   */
+  public function testDropColumn() {
+    CRM_Core_DAO::executeQuery('DROP TABLE IF EXISTS `civicrm_test_drop_column`');
+    CRM_Core_DAO::executeQuery('CREATE TABLE `civicrm_test_drop_column` (`id` int(10), `col1` varchar(255), `col2` varchar(255))');
+
+    // test with logging enabled to ensure log triggers don't break anything
+    $schema = new CRM_Logging_Schema();
+    $schema->enableLogging();
+
+    $alterParams = [
+      'table_name' => 'civicrm_test_drop_column',
+      'operation'  => 'delete',
+      'name'       => 'col1',
+      'type'       => 'varchar(255)',
+      'required'   => FALSE,
+      'searchable' => FALSE,
+    ];
+
+    // drop col1
+    CRM_Core_BAO_SchemaHandler::alterFieldSQL($alterParams, FALSE, TRUE);
+
+    $create_table = CRM_Core_DAO::executeQuery("SHOW CREATE TABLE civicrm_test_drop_column");
+    while ($create_table->fetch()) {
+      $this->assertNotContains('col1', $create_table->Create_Table);
+      $this->assertContains('col2', $create_table->Create_Table);
+    }
+
+    // drop col2
+    $alterParams['name'] = 'col2';
+    CRM_Core_BAO_SchemaHandler::alterFieldSQL($alterParams, FALSE, TRUE);
+
+    $create_table = CRM_Core_DAO::executeQuery("SHOW CREATE TABLE civicrm_test_drop_column");
+    while ($create_table->fetch()) {
+      $this->assertNotContains('col2', $create_table->Create_Table);
+    }
+  }
+
 }
-- 
2.25.1