[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10278?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16417184#comment-16417184
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10278:
---------------------------------------------

DaanHoogland closed pull request #2449: CLOUDSTACK-10278 idempotent column 
addition
URL: https://github.com/apache/cloudstack/pull/2449
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/engine/schema/resources/META-INF/db/schema-41000to41100.sql 
b/engine/schema/resources/META-INF/db/schema-41000to41100.sql
index 2e7f9e9fe9c..4dc11d448fc 100644
--- a/engine/schema/resources/META-INF/db/schema-41000to41100.sql
+++ b/engine/schema/resources/META-INF/db/schema-41000to41100.sql
@@ -19,8 +19,54 @@
 -- Schema upgrade from 4.10.0.0 to 4.11.0.0
 --;
 
+--;
+-- Stored procedure to do idempotent column add;
+--;
+DROP PROCEDURE IF EXISTS `cloud`.`IDEMPOTENT_ADD_COLUMN`;
+
+CREATE PROCEDURE `cloud`.`IDEMPOTENT_ADD_COLUMN` (
+               IN in_table_name VARCHAR(200)
+    , IN in_column_name VARCHAR(200)
+    , IN in_column_definition VARCHAR(1000)
+)
+BEGIN
+
+    DECLARE CONTINUE HANDLER FOR 1060 BEGIN END; SET @ddl = CONCAT('ALTER 
TABLE ', in_table_name); SET @ddl = CONCAT(@ddl, ' ', 'ADD COLUMN') ; SET @ddl 
= CONCAT(@ddl, ' ', in_column_name); SET @ddl = CONCAT(@ddl, ' ', 
in_column_definition); PREPARE stmt FROM @ddl; EXECUTE stmt; DEALLOCATE PREPARE 
stmt; END;
+
+DROP PROCEDURE IF EXISTS `cloud`.`IDEMPOTENT_DROP_FOREIGN_KEY`;
+
+CREATE PROCEDURE `cloud`.`IDEMPOTENT_DROP_FOREIGN_KEY` (
+               IN in_table_name VARCHAR(200)
+    , IN in_foreign_key_name VARCHAR(200)
+)
+BEGIN
+
+    DECLARE CONTINUE HANDLER FOR 1091 BEGIN END; SET @ddl = CONCAT('ALTER 
TABLE ', in_table_name); SET @ddl = CONCAT(@ddl, ' ', ' DROP FOREIGN KEY '); 
SET @ddl = CONCAT(@ddl, ' ', in_foreign_key_name); PREPARE stmt FROM @ddl; 
EXECUTE stmt; DEALLOCATE PREPARE stmt; END;
+
+DROP PROCEDURE IF EXISTS `cloud`.`IDEMPOTENT_DROP_INDEX`;
+
+CREATE PROCEDURE `cloud`.`IDEMPOTENT_DROP_INDEX` (
+               IN in_index_name VARCHAR(200)
+    , IN in_table_name VARCHAR(200)
+)
+BEGIN
+
+    DECLARE CONTINUE HANDLER FOR 1091 BEGIN END; SET @ddl = CONCAT('DROP INDEX 
', in_index_name); SET @ddl = CONCAT(@ddl, ' ', ' ON ') ; SET @ddl = 
CONCAT(@ddl, ' ', in_table_name); PREPARE stmt FROM @ddl; EXECUTE stmt; 
DEALLOCATE PREPARE stmt; END;
+
+DROP PROCEDURE IF EXISTS `cloud`.`IDEMPOTENT_CREATE_UNIQUE_INDEX`;
+
+CREATE PROCEDURE `cloud`.`IDEMPOTENT_CREATE_UNIQUE_INDEX` (
+               IN in_index_name VARCHAR(200)
+    , IN in_table_name VARCHAR(200)
+    , IN in_index_definition VARCHAR(1000)
+)
+BEGIN
+
+    DECLARE CONTINUE HANDLER FOR 1061 BEGIN END; SET @ddl = CONCAT('CREATE 
UNIQUE INDEX ', in_index_name); SET @ddl = CONCAT(@ddl, ' ', ' ON ') ; SET @ddl 
= CONCAT(@ddl, ' ', in_table_name); SET @ddl = CONCAT(@ddl, ' ', 
in_index_definition); PREPARE stmt FROM @ddl; EXECUTE stmt; DEALLOCATE PREPARE 
stmt; END;
+
 -- Add For VPC flag
-ALTER TABLE cloud.network_offerings ADD COLUMN for_vpc INT(1) NOT NULL DEFAULT 
0;
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.network_offerings','for_vpc', 
'INT(1) NOT NULL DEFAULT 0');
+
 UPDATE cloud.network_offerings o
 SET for_vpc = 1
 where
@@ -88,7 +134,7 @@ CREATE TABLE IF NOT EXISTS `cloud`.`annotations` (
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
 
 DROP VIEW IF EXISTS `cloud`.`last_annotation_view`;
-CREATE VIEW `last_annotation_view` AS
+CREATE VIEW `cloud`.`last_annotation_view` AS
     SELECT
         `annotations`.`uuid` AS `uuid`,
         `annotations`.`annotation` AS `annotation`,
@@ -405,21 +451,18 @@ UPDATE `cloud`.`monitoring_services` SET 
pidfile="/var/run/apache2/apache2.pid"
 UPDATE `cloud`.`vm_template` SET guest_os_id=99 WHERE id=8;
 
 -- Network External Ids
-ALTER TABLE `cloud`.`networks` ADD `external_id` varchar(255);
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.networks','external_id', 
'varchar(255)');
 
 -- Separate Subnet for CPVM and SSVM (system vms)
-ALTER TABLE `cloud`.`op_dc_ip_address_alloc`
-ADD COLUMN `forsystemvms` TINYINT(1) NOT NULL DEFAULT '0' COMMENT 'Indicates 
if IP is dedicated for CPVM or SSVM';
+CALL 
`cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.op_dc_ip_address_alloc','forsystemvms', 
'TINYINT(1) NOT NULL DEFAULT 0 COMMENT ''Indicates if IP is dedicated for CPVM 
or SSVM'' ');
 
-ALTER TABLE `cloud`.`op_dc_ip_address_alloc`
-ADD COLUMN `vlan` INT(10) UNSIGNED NULL COMMENT 'Vlan the management network 
range is on';
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.op_dc_ip_address_alloc','vlan', 
'INT(10) UNSIGNED NULL COMMENT ''Vlan the management network range is on'' ');
 
 -- CLOUDSTACK-4757: Support multidisk OVA
-ALTER TABLE `cloud`.`vm_template` ADD COLUMN `parent_template_id` bigint(20) 
unsigned DEFAULT NULL COMMENT 'If datadisk template, then id of the root 
template this template belongs to';
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.vm_template','parent_template_id', 
'bigint(20) unsigned DEFAULT NULL COMMENT ''If datadisk template, then id of 
the root template this template belongs to'' ');
 
 -- CLOUDSTACK-10146: Bypass Secondary Storage for KVM templates
-ALTER TABLE `cloud`.`vm_template`
-ADD COLUMN `direct_download` TINYINT(1) DEFAULT '0' COMMENT 'Indicates if 
Secondary Storage is bypassed and template is downloaded to Primary Storage';
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.vm_template','direct_download', 
'TINYINT(1) DEFAULT 0 COMMENT ''Indicates if Secondary Storage is bypassed and 
template is downloaded to Primary Storage'' ');
 
 -- Changes to template_view for both multidisk OVA and bypass secondary 
storage for KVM templates
 DROP VIEW IF EXISTS `cloud`.`template_view`;
@@ -528,8 +571,7 @@ CREATE VIEW `cloud`.`template_view` AS
              OR (`resource_tags`.`resource_type` = 'ISO')))));
 
 -- CLOUDSTACK-10109: Enable dedication of public IPs to SSVM and CPVM
-ALTER TABLE `cloud`.`user_ip_address`
-ADD COLUMN `forsystemvms` TINYINT(1) NOT NULL DEFAULT '0' COMMENT 'true if IP 
is set to system vms, false if not';
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.user_ip_address','forsystemvms', 
'TINYINT(1) NOT NULL DEFAULT 0 COMMENT ''true if IP is set to system vms, false 
if not'' ');
 
 -- ldap binding on domain level
 CREATE TABLE IF NOT EXISTS `cloud`.`domain_details` (
@@ -541,11 +583,11 @@ CREATE TABLE IF NOT EXISTS `cloud`.`domain_details` (
     CONSTRAINT `fk_domain_details__domain_id` FOREIGN KEY (`domain_id`) 
REFERENCES `domain`(`id`) ON DELETE CASCADE
 )ENGINE=InnoDB DEFAULT CHARSET=utf8;
 
-ALTER TABLE cloud.ldap_configuration ADD COLUMN domain_id BIGINT(20) DEFAULT 
NULL;
-ALTER TABLE cloud.ldap_trust_map ADD COLUMN account_id BIGINT(20) DEFAULT 0;
-ALTER TABLE cloud.ldap_trust_map DROP FOREIGN KEY fk_ldap_trust_map__domain_id;
-DROP INDEX uk_ldap_trust_map__domain_id ON cloud.ldap_trust_map;
-CREATE UNIQUE INDEX uk_ldap_trust_map__bind_location ON ldap_trust_map 
(domain_id, account_id);
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.ldap_configuration','domain_id', 
'BIGINT(20) DEFAULT NULL');
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.ldap_trust_map','account_id', 
'BIGINT(20) DEFAULT 0');
+CALL 
`cloud`.`IDEMPOTENT_DROP_FOREIGN_KEY`('cloud.ldap_trust_map','fk_ldap_trust_map__domain_id');
+CALL 
`cloud`.`IDEMPOTENT_DROP_INDEX`('uk_ldap_trust_map__domain_id','cloud.ldap_trust_map');
+CALL 
`cloud`.`IDEMPOTENT_CREATE_UNIQUE_INDEX`('uk_ldap_trust_map__bind_location','cloud.ldap_trust_map',
 '(domain_id, account_id)');
 
 CREATE TABLE IF NOT EXISTS `cloud`.`netscaler_servicepackages` (
   `id` bigint unsigned NOT NULL AUTO_INCREMENT COMMENT 'id',
@@ -565,5 +607,5 @@ CREATE TABLE IF NOT EXISTS 
`cloud`.`external_netscaler_controlcenter` (
   PRIMARY KEY  (`id`)
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
 
-ALTER TABLE `cloud`.`sslcerts` ADD COLUMN `name` varchar(255) NULL default 
NULL COMMENT 'Name of the Certificate';
-ALTER TABLE `cloud`.`network_offerings` ADD COLUMN `service_package_id` 
varchar(255) NULL default NULL COMMENT 'Netscaler ControlCenter Service 
Package';
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.sslcerts','name', 'varchar(255) 
NULL default NULL COMMENT ''Name of the Certificate'' ');
+CALL 
`cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.network_offerings','service_package_id', 
'varchar(255) NULL default NULL COMMENT ''Netscaler ControlCenter Service 
Package'' ');
\ No newline at end of file


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Adding a SQL table column is not Idempotent
> -------------------------------------------
>
>                 Key: CLOUDSTACK-10278
>                 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10278
>             Project: CloudStack
>          Issue Type: Bug
>      Security Level: Public(Anyone can view this level - this is the 
> default.) 
>          Components: Install and Setup
>    Affects Versions: 4.10.0.0, 4.11.0.0
>            Reporter: Ernie Janse van Rensburg
>            Assignee: Ernie Janse van Rensburg
>            Priority: Major
>   Original Estimate: 4h
>  Remaining Estimate: 4h
>
> The SQL code to add a new column to a table in the 
> META-INF/db/schema-41000to41100.sql script is not written in an idempotent 
> way. When the upgrade is re-run, the code above causes a SQL error as 
> reported on the user mailing list: 
> ERROR [c.c.u.d.ScriptRunner] (main:null) (logid:)
> Error executing: ALTER TABLE cloud.network_offerings ADD COLUMN for_vpc
> INT(1) NOT NULL DEFAULT 0
> This is a more generic problem for every version due to to the fact that it 
> is not idempotent
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to