View Issue Details

IDProjectCategoryView StatusLast Update
586RackTablesdefaultpublic2012-08-22 18:01
Reporterhg-xing2 Assigned Toinfrastation  
PrioritynormalSeverityminorReproducibilityN/A
Status closedResolutionfixed 
Product Version0.19.13 
Target Version0.19.14Fixed in Version0.19.14 
Summary586: Change Indices and Constrains on AttributeValue
DescriptionHi,

The indices and constrains on AttributeValue look a bit strange to me. First of all: It has no primary index althought the unique index object_id-attr_id seems to be a really good candidate for that. Furthermore object_id and attr_id explicitly allow null, which makes the unique index not really unique. Is this correct? Maybe I overlook something, but I guess inserting object_id = NULL or attr_id = NULL will break things anyway.

Can we make object_id and attr_id NON NULL and create a primary key object_id-attr_id ?

Thank you
Hannes
TagsNo tags attached.

Activities

infrastation

infrastation

2012-07-22 21:17

administrator   ~0000712

From MySQL reference manual: "KEY is normally a synonym for INDEX. The key attribute PRIMARY KEY can also be specified as just KEY when given in a column definition. This was implemented for compatibility with other database systems."

This way, this index already serves as the primary key:
UNIQUE KEY `object_id` (`object_id`,`attr_id`)

Regarding the NOT NULL constraint, yes, this makes sense. Can you use "object_tid" as an example and send a complete patch including both upgrade.php and install.php?
hg-xing2

hg-xing2

2012-07-24 08:15

reporter  

change_indices.patch (1,707 bytes)   
diff --git a/wwwroot/inc/install.php b/wwwroot/inc/install.php
index e35d8fb..45a2853 100644
--- a/wwwroot/inc/install.php
+++ b/wwwroot/inc/install.php
@@ -447,15 +447,15 @@ CREATE TABLE `AttributeMap` (
 ) ENGINE=InnoDB;
 
 CREATE TABLE `AttributeValue` (
-  `object_id` int(10) unsigned default NULL,
+  `object_id` int(10) unsigned NOT NULL,
   -- Default value intentionally breaks the constraint, this blocks
   -- any insertion, which doesn't have 'object_tid' on the column list.
   `object_tid` int(10) unsigned NOT NULL default '0',
-  `attr_id` int(10) unsigned default NULL,
+  `attr_id` int(10) unsigned NOT NULL,
   `string_value` char(255) default NULL,
   `uint_value` int(10) unsigned default NULL,
   `float_value` float default NULL,
-  UNIQUE KEY `object_id` (`object_id`,`attr_id`),
+  PRIMARY KEY (`object_id`,`attr_id`),
   KEY `attr_id-uint_value` (`attr_id`,`uint_value`),
   KEY `attr_id-string_value` (`attr_id`,`string_value`(12)),
   KEY `id-tid` (`object_id`,`object_tid`),
diff --git a/wwwroot/inc/upgrade.php b/wwwroot/inc/upgrade.php
index 5d0600a..8b43f4c 100644
--- a/wwwroot/inc/upgrade.php
+++ b/wwwroot/inc/upgrade.php
@@ -1254,6 +1254,9 @@ CREATE TABLE `CactiGraph` (
 
 			$query[] = "UPDATE Config SET varvalue = '0.19.13' WHERE varname = 'DB_VERSION'";
 			break;
+		case '0.19.14':
+			$query[] = 'ALTER TABLE `racktables`.`AttributeValue` CHANGE COLUMN `object_id` `object_id` INT(10) UNSIGNED NOT NULL  , CHANGE COLUMN `attr_id` `attr_id` INT(10) UNSIGNED NOT NULL;';
+			$query[] = 'ALTER TABLE `racktables`.`AttributeValue` ADD PRIMARY KEY (`object_id`, `attr_id`) , DROP INDEX `object_id` ;';
 		case 'dictionary':
 			$query = reloadDictionary();
 			break;
change_indices.patch (1,707 bytes)   
hg-xing2

hg-xing2

2012-07-24 08:21

reporter   ~0000713

Yep, KEY is a synonym for INDEX. The point is that UNIQUE indices don't behave as expected with null values ( http://bugs.mysql.com/bug.php?id=8173 ) and mainly ensure validity whereas primary keys identify the rows. Since we want to identify the rows here, primary key is it.
infrastation

infrastation

2012-07-26 08:03

administrator   ~0000714

Do you see how this change breaks an upgrade?
infrastation

infrastation

2012-08-22 18:01

administrator   ~0000745

I have committed this change to the indices, but please note that it was necessary to fix the switch block and to remove database name in the patch.

Issue History

Date Modified Username Field Change
2012-07-19 12:28 hg-xing2 New Issue
2012-07-21 19:36 adoom42 Assigned To => infrastation
2012-07-21 19:36 adoom42 Status new => assigned
2012-07-22 21:17 infrastation Note Added: 0000712
2012-07-24 08:15 hg-xing2 File Added: change_indices.patch
2012-07-24 08:21 hg-xing2 Note Added: 0000713
2012-07-26 08:03 infrastation Note Added: 0000714
2012-08-22 18:01 infrastation Note Added: 0000745
2012-08-22 18:01 infrastation Status assigned => closed
2012-08-22 18:01 infrastation Resolution open => fixed
2012-08-22 18:01 infrastation Fixed in Version => 0.19.14
2012-08-22 18:01 infrastation Target Version => 0.19.14