From a3f7117f8da41aa2e821544dd63a395b41021857 Mon Sep 17 00:00:00 2001 From: rillke Date: Tue, 27 May 2014 22:36:51 +0200 Subject: [PATCH] Name implicitly created CHECK constraints to ease future maintenance work. The pattern for catching the right CHECK constraint is admittedly vague but the order in which auto-created CHECK constraints of a table is removed doesn't really matter assuming the update is always completed before another update is applied. If updates are run through Update.php, that should be always the case. Furthermore eliminating a bug causing application of several patches each time when running Update.php Bug: 65757 Bug: 65813 Change-Id: Ic7fc3bd836241dce8f296237bbd80ed3e4d1ee0d --- RELEASE-NOTES-1.24 | 3 + includes/installer/DatabaseUpdater.php | 3 +- includes/installer/MssqlUpdater.php | 82 +++++++++++++++++++ .../mssql/archives/named_constraints.sql | 38 +++++++++ maintenance/mssql/tables.sql | 35 +++++--- 5 files changed, 148 insertions(+), 13 deletions(-) mode change 100644 => 100755 includes/installer/DatabaseUpdater.php mode change 100644 => 100755 includes/installer/MssqlUpdater.php create mode 100755 maintenance/mssql/archives/named_constraints.sql mode change 100644 => 100755 maintenance/mssql/tables.sql diff --git a/RELEASE-NOTES-1.24 b/RELEASE-NOTES-1.24 index 7cc8b130f1..2f41347c7d 100644 --- a/RELEASE-NOTES-1.24 +++ b/RELEASE-NOTES-1.24 @@ -124,6 +124,9 @@ production. however delete the redirect page. * (bug 22683) {{msgnw:}} and other uses of PPFrame::RECOVER_ORIG will correctly recover the original code of extension tags. +* (bug 65757) MSSQL: Update script drops unnamed constraints to be prepared + for future updates. Because it's doing so heuristically, it may fail or drop + wrong constraints. === Web API changes in 1.24 === * action=parse API now supports prop=modules, which provides the list of diff --git a/includes/installer/DatabaseUpdater.php b/includes/installer/DatabaseUpdater.php old mode 100644 new mode 100755 index 82a358e0bf..28dac46314 --- a/includes/installer/DatabaseUpdater.php +++ b/includes/installer/DatabaseUpdater.php @@ -476,7 +476,8 @@ abstract class DatabaseUpdater { public function updateRowExists( $key ) { $row = $this->db->selectRow( 'updatelog', - '1', + # Bug 65813 + '1 AS X', array( 'ul_key' => $key ), __METHOD__ ); diff --git a/includes/installer/MssqlUpdater.php b/includes/installer/MssqlUpdater.php old mode 100644 new mode 100755 index 0054573e1d..d590a709fc --- a/includes/installer/MssqlUpdater.php +++ b/includes/installer/MssqlUpdater.php @@ -42,6 +42,88 @@ class MssqlUpdater extends DatabaseUpdater { // 1.24 array( 'addField', 'page', 'page_lang', 'patch-page-page_lang.sql'), + // Constraint updates + array( 'updateConstraints', 'category_types', 'categorylinks', 'cl_type' ), + array( 'updateConstraints', 'major_mime', 'filearchive', 'fa_major_mime' ), + array( 'updateConstraints', 'media_type', 'filearchive', 'fa_media_type' ), + array( 'updateConstraints', 'major_mime', 'oldimage', 'oi_major_mime' ), + array( 'updateConstraints', 'media_type', 'oldimage', 'oi_media_type' ), + array( 'updateConstraints', 'major_mime', 'image', 'img_major_mime' ), + array( 'updateConstraints', 'media_type', 'image', 'img_media_type' ), + array( 'updateConstraints', 'media_type', 'uploadstash', 'us_media_type' ), + // END: Constraint updates ); } + + /** + * Drops unnamed and creates named constraints following the pattern + * _ckc + * + * @param string $constraintType + * @param string $table Name of the table to which the field belongs + * @param string $field Name of the field to modify + * @return bool False if patch is skipped. + */ + protected function updateConstraints( $constraintType, $table, $field ) { + if ( !$this->doTable( $table ) ) { + return true; + } + + $this->output( "...updating constraints on [$table].[$field] ..." ); + $updateKey = "$field-$constraintType-ck"; + if ( !$this->db->tableExists( $table, __METHOD__ ) ) { + $this->output( "...$table table does not exist, skipping modify field patch.\n" ); + return true; + } elseif ( !$this->db->fieldExists( $table, $field, __METHOD__ ) ) { + $this->output( "...$field field does not exist in $table table, " . + "skipping modify field patch.\n" ); + return true; + } elseif ( $this->updateRowExists( $updateKey ) ) { + $this->output( "...$field in table $table already patched.\n" ); + return true; + } + + # After all checks passed, start the update + $this->insertUpdateRow( $updateKey ); + $path = 'named_constraints.sql'; + $constraintMap = array( + 'category_types' => + "($field in('page', 'subcat', 'file'))", + 'major_mime' => + "($field in('unknown', 'application', 'audio', 'image', 'text', 'video'," . + " 'message', 'model', 'multipart'))", + 'media_type' => + "($field in('UNKNOWN', 'BITMAP', 'DRAWING', 'AUDIO', 'VIDEO', 'MULTIMEDIA'," . + "'OFFICE', 'TEXT', 'EXECUTABLE', 'ARCHIVE'))" + ); + $constraint = $constraintMap[$constraintType]; + + # and hack-in those variables that should be replaced + # in our template file right now + $this->db->setSchemaVars( array( + 'tableName' => $table, + 'fieldName' => $field, + 'checkConstraint' => $constraint, + 'wgDBname' => $wgDBname, + 'wgDBmwschema' => $wgDBmwschema, + ) ); + + # Full path from file name + $path = $this->db->patchPath( $path ); + + # No need for a cursor allowing result-iteration; just apply a patch + # store old value for re-setting later + $wasScrollable = $this->db->scrollableCursor( false ); + + # Apply patch + $this->db->sourceFile( $path ); + + # Reset DB instance to have original state + $this->db->setSchemaVars( false ); + $this->db->scrollableCursor( $wasScrollable ); + + $this->output( "done.\n" ); + + return true; + } } diff --git a/maintenance/mssql/archives/named_constraints.sql b/maintenance/mssql/archives/named_constraints.sql new file mode 100755 index 0000000000..94b77ea7cd --- /dev/null +++ b/maintenance/mssql/archives/named_constraints.sql @@ -0,0 +1,38 @@ +DECLARE @fullyQualifiedTableName nvarchar(max), +@tableName sysname, +@fieldName sysname, +@constr sysname, +@constrNew sysname, +@sqlcmd nvarchar(max), +@sqlcreate nvarchar(max) + +SET @fullyQualifiedTableName = '/*_*//*$tableName*/' +SET @tableName = '/*$tableName*/' +SET @fieldName = '/*$fieldName*/' + +SELECT @constr = CONSTRAINT_NAME +FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS +WHERE TABLE_NAME = @tableName +AND CONSTRAINT_CATALOG = '/*$wgDBname*/' +AND CONSTRAINT_SCHEMA = '/*$wgDBmwschema*/' +AND CONSTRAINT_TYPE = 'CHECK' +AND CONSTRAINT_NAME LIKE ('CK__' + left(@tableName,9) + '__' + left(@fieldName,5) + '%') + +SELECT @constrNew = CONSTRAINT_NAME +FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS +WHERE TABLE_NAME = @tableName +AND CONSTRAINT_CATALOG = '/*$wgDBname*/' +AND CONSTRAINT_SCHEMA = '/*$wgDBmwschema*/' +AND CONSTRAINT_TYPE = 'CHECK' +AND CONSTRAINT_NAME = (@fieldName + '_ckc') + +IF @constr IS NOT NULL +BEGIN + SET @sqlcmd = 'ALTER TABLE ' + @fullyQualifiedTableName + ' DROP CONSTRAINT [' + @constr + ']' + EXECUTE sp_executesql @sqlcmd +END +IF @constrNew IS NULL +BEGIN + SET @sqlcreate = 'ALTER TABLE ' + @fullyQualifiedTableName + ' WITH NOCHECK ADD CONSTRAINT ' + @fieldName + '_ckc CHECK /*$checkConstraint*/;' + EXECUTE sp_executesql @sqlcreate +END \ No newline at end of file diff --git a/maintenance/mssql/tables.sql b/maintenance/mssql/tables.sql old mode 100644 new mode 100755 index fa189ff16d..daaa81eeea --- a/maintenance/mssql/tables.sql +++ b/maintenance/mssql/tables.sql @@ -299,8 +299,9 @@ CREATE TABLE /*_*/categorylinks ( -- paginate the three categories separately. This never has to be updated -- after the page is created, since none of these page types can be moved to -- any other. + cl_type varchar(10) NOT NULL default 'page', -- SQL server doesn't have enums, so we approximate with this - cl_type varchar(10) NOT NULL default 'page' CHECK (cl_type IN('page', 'subcat', 'file')) + CONSTRAINT cl_type_ckc CHECK (cl_type IN('page', 'subcat', 'file')) ); CREATE UNIQUE INDEX /*i*/cl_from ON /*_*/categorylinks (cl_from,cl_to); @@ -567,11 +568,11 @@ CREATE TABLE /*_*/image ( img_bits int NOT NULL default 0, -- Media type as defined by the MEDIATYPE_xxx constants - img_media_type varchar(16) default null check (img_media_type in('UNKNOWN', 'BITMAP', 'DRAWING', 'AUDIO', 'VIDEO', 'MULTIMEDIA', 'OFFICE', 'TEXT', 'EXECUTABLE', 'ARCHIVE')), + img_media_type varchar(16) default null, -- major part of a MIME media type as defined by IANA -- see http://www.iana.org/assignments/media-types/ - img_major_mime varchar(16) not null default 'unknown' check (img_major_mime IN('unknown', 'application', 'audio', 'image', 'text', 'video', 'message', 'model', 'multipart')), + img_major_mime varchar(16) not null default 'unknown', -- minor part of a MIME media type as defined by IANA -- the minor parts are not required to adher to any standard @@ -591,7 +592,10 @@ CREATE TABLE /*_*/image ( img_timestamp nvarchar(14) NOT NULL default '', -- SHA-1 content hash in base-36 - img_sha1 nvarchar(32) NOT NULL default '' + img_sha1 nvarchar(32) NOT NULL default '', + + CONSTRAINT img_major_mime_ckc check (img_major_mime IN('unknown', 'application', 'audio', 'image', 'text', 'video', 'message', 'model', 'multipart')), + CONSTRAINT img_media_type_ckc check (img_media_type in('UNKNOWN', 'BITMAP', 'DRAWING', 'AUDIO', 'VIDEO', 'MULTIMEDIA', 'OFFICE', 'TEXT', 'EXECUTABLE', 'ARCHIVE')) ); CREATE INDEX /*i*/img_usertext_timestamp ON /*_*/image (img_user_text,img_timestamp); @@ -629,11 +633,14 @@ CREATE TABLE /*_*/oldimage ( oi_timestamp varchar(14) NOT NULL default '', oi_metadata nvarchar(max) NOT NULL, - oi_media_type varchar(16) default null check (oi_media_type IN('UNKNOWN', 'BITMAP', 'DRAWING', 'AUDIO', 'VIDEO', 'MULTIMEDIA', 'OFFICE', 'TEXT', 'EXECUTABLE', 'ARCHIVE')), - oi_major_mime varchar(16) not null default 'unknown' check (oi_major_mime IN('unknown', 'application', 'audio', 'image', 'text', 'video', 'message', 'model', 'multipart')), + oi_media_type varchar(16) default null, + oi_major_mime varchar(16) not null default 'unknown', oi_minor_mime nvarchar(100) NOT NULL default 'unknown', oi_deleted tinyint NOT NULL default 0, - oi_sha1 nvarchar(32) NOT NULL default '' + oi_sha1 nvarchar(32) NOT NULL default '', + + CONSTRAINT oi_major_mime_ckc check (oi_major_mime IN('unknown', 'application', 'audio', 'image', 'text', 'video', 'message', 'model', 'multipart')), + CONSTRAINT oi_media_type_ckc check (oi_media_type IN('UNKNOWN', 'BITMAP', 'DRAWING', 'AUDIO', 'VIDEO', 'MULTIMEDIA', 'OFFICE', 'TEXT', 'EXECUTABLE', 'ARCHIVE')) ); CREATE INDEX /*i*/oi_usertext_timestamp ON /*_*/oldimage (oi_user_text,oi_timestamp); @@ -679,8 +686,8 @@ CREATE TABLE /*_*/filearchive ( fa_height int default 0, fa_metadata nvarchar(max), fa_bits int default 0, - fa_media_type varchar(16) default null check (fa_media_type in('UNKNOWN', 'BITMAP', 'DRAWING', 'AUDIO', 'VIDEO', 'MULTIMEDIA', 'OFFICE', 'TEXT', 'EXECUTABLE', 'ARCHIVE')), - fa_major_mime varchar(16) not null default 'unknown' check (fa_major_mime in('unknown', 'application', 'audio', 'image', 'text', 'video', 'message', 'model', 'multipart')), + fa_media_type varchar(16) default null, + fa_major_mime varchar(16) not null default 'unknown', fa_minor_mime nvarchar(100) default 'unknown', fa_description nvarchar(255), fa_user int default 0 REFERENCES /*_*/mwuser(user_id) ON DELETE SET NULL, @@ -691,7 +698,10 @@ CREATE TABLE /*_*/filearchive ( fa_deleted tinyint NOT NULL default 0, -- sha1 hash of file content - fa_sha1 nvarchar(32) NOT NULL default '' + fa_sha1 nvarchar(32) NOT NULL default '', + + CONSTRAINT fa_major_mime_ckc check (fa_major_mime in('unknown', 'application', 'audio', 'image', 'text', 'video', 'message', 'model', 'multipart')), + CONSTRAINT fa_media_type_ckc check (fa_media_type in('UNKNOWN', 'BITMAP', 'DRAWING', 'AUDIO', 'VIDEO', 'MULTIMEDIA', 'OFFICE', 'TEXT', 'EXECUTABLE', 'ARCHIVE')) ); -- pick out by image name @@ -746,12 +756,13 @@ CREATE TABLE /*_*/uploadstash ( us_sha1 nvarchar(31) NOT NULL, us_mime nvarchar(255), -- Media type as defined by the MEDIATYPE_xxx constants, should duplicate definition in the image table - us_media_type varchar(16) default null check (us_media_type in('UNKNOWN', 'BITMAP', 'DRAWING', 'AUDIO', 'VIDEO', 'MULTIMEDIA', 'OFFICE', 'TEXT', 'EXECUTABLE', 'ARCHIVE')), + us_media_type varchar(16) default null, -- image-specific properties us_image_width int, us_image_height int, - us_image_bits smallint + us_image_bits smallint, + CONSTRAINT us_media_type_ckc check (us_media_type in('UNKNOWN', 'BITMAP', 'DRAWING', 'AUDIO', 'VIDEO', 'MULTIMEDIA', 'OFFICE', 'TEXT', 'EXECUTABLE', 'ARCHIVE')) ); -- sometimes there's a delete for all of a user's stuff. -- 2.20.1