diff --git a/src/applications/cache/storage/PhabricatorCacheSchemaSpec.php b/src/applications/cache/storage/PhabricatorCacheSchemaSpec.php --- a/src/applications/cache/storage/PhabricatorCacheSchemaSpec.php +++ b/src/applications/cache/storage/PhabricatorCacheSchemaSpec.php @@ -9,7 +9,7 @@ 'cache', id(new PhabricatorKeyValueDatabaseCache())->getTableName(), array( - 'id' => 'id64', + 'id' => 'auto64', 'cacheKeyHash' => 'bytes12', 'cacheKey' => 'text128', 'cacheFormat' => 'text16', diff --git a/src/applications/conduit/storage/PhabricatorConduitMethodCallLog.php b/src/applications/conduit/storage/PhabricatorConduitMethodCallLog.php --- a/src/applications/conduit/storage/PhabricatorConduitMethodCallLog.php +++ b/src/applications/conduit/storage/PhabricatorConduitMethodCallLog.php @@ -13,7 +13,7 @@ public function getConfiguration() { return array( self::CONFIG_COLUMN_SCHEMA => array( - 'id' => 'id64', + 'id' => 'auto64', 'connectionID' => 'id64?', 'method' => 'text64', 'error' => 'text255', diff --git a/src/applications/config/controller/PhabricatorConfigDatabaseStatusController.php b/src/applications/config/controller/PhabricatorConfigDatabaseStatusController.php --- a/src/applications/config/controller/PhabricatorConfigDatabaseStatusController.php +++ b/src/applications/config/controller/PhabricatorConfigDatabaseStatusController.php @@ -282,6 +282,7 @@ $unique_issue = PhabricatorConfigStorageSchema::ISSUE_UNIQUE; $columns_issue = PhabricatorConfigStorageSchema::ISSUE_KEYCOLUMNS; $longkey_issue = PhabricatorConfigStorageSchema::ISSUE_LONGKEY; + $auto_issue = PhabricatorConfigStorageSchema::ISSUE_AUTOINCREMENT; $database = $comp->getDatabase($database_name); if (!$database) { @@ -340,6 +341,9 @@ $this->renderBoolean($column->getNullable()), $column->hasIssue($nullable_issue)), $this->renderAttr( + $this->renderBoolean($column->getAutoIncrement()), + $column->hasIssue($auto_issue)), + $this->renderAttr( $column->getCharacterSet(), $column->hasIssue($charset_issue)), $this->renderAttr( @@ -356,6 +360,7 @@ pht('Data Type'), pht('Column Type'), pht('Nullable'), + pht('Autoincrement'), pht('Character Set'), pht('Collation'), )) @@ -366,6 +371,7 @@ null, null, null, + null, null )); @@ -521,11 +527,13 @@ $actual_charset = $actual_column->getCharacterSet(); $actual_collation = $actual_column->getCollation(); $actual_nullable = $actual_column->getNullable(); + $actual_auto = $actual_column->getAutoIncrement(); } else { $actual_coltype = null; $actual_charset = null; $actual_collation = null; $actual_nullable = null; + $actual_auto = null; } if ($expect_column) { @@ -534,12 +542,14 @@ $expect_charset = $expect_column->getCharacterSet(); $expect_collation = $expect_column->getCollation(); $expect_nullable = $expect_column->getNullable(); + $expect_auto = $expect_column->getAutoIncrement(); } else { $data_type = null; $expect_coltype = null; $expect_charset = null; $expect_collation = null; $expect_nullable = null; + $expect_auto = null; } @@ -587,6 +597,14 @@ pht('Expected Nullable'), $this->renderBoolean($expect_nullable), ), + array( + pht('Autoincrement'), + $this->renderBoolean($actual_auto), + ), + array( + pht('Expected Autoincrement'), + $this->renderBoolean($expect_auto), + ), ), $column->getIssues()); diff --git a/src/applications/config/schema/PhabricatorConfigColumnSchema.php b/src/applications/config/schema/PhabricatorConfigColumnSchema.php --- a/src/applications/config/schema/PhabricatorConfigColumnSchema.php +++ b/src/applications/config/schema/PhabricatorConfigColumnSchema.php @@ -8,6 +8,16 @@ private $columnType; private $dataType; private $nullable; + private $autoIncrement; + + public function setAutoIncrement($auto_increment) { + $this->autoIncrement = $auto_increment; + return $this; + } + + public function getAutoIncrement() { + return $this->autoIncrement; + } public function setNullable($nullable) { $this->nullable = $nullable; @@ -131,6 +141,10 @@ $issues[] = self::ISSUE_NULLABLE; } + if ($this->getAutoIncrement() !== $expect->getAutoIncrement()) { + $issues[] = self::ISSUE_AUTOINCREMENT; + } + return $issues; } diff --git a/src/applications/config/schema/PhabricatorConfigSchemaQuery.php b/src/applications/config/schema/PhabricatorConfigSchemaQuery.php --- a/src/applications/config/schema/PhabricatorConfigSchemaQuery.php +++ b/src/applications/config/schema/PhabricatorConfigSchemaQuery.php @@ -60,7 +60,7 @@ $column_info = queryfx_all( $conn, 'SELECT TABLE_SCHEMA, TABLE_NAME, COLUMN_NAME, CHARACTER_SET_NAME, - COLLATION_NAME, COLUMN_TYPE, IS_NULLABLE + COLLATION_NAME, COLUMN_TYPE, IS_NULLABLE, EXTRA FROM INFORMATION_SCHEMA.COLUMNS WHERE (%Q)', '('.implode(') OR (', $sql).')'); @@ -96,12 +96,19 @@ $columns = idx($database_column_info, $table_name, array()); foreach ($columns as $column) { + if (strpos($column['EXTRA'], 'auto_increment') === false) { + $auto_increment = false; + } else { + $auto_increment = true; + } + $column_schema = id(new PhabricatorConfigColumnSchema()) ->setName($column['COLUMN_NAME']) ->setCharacterSet($column['CHARACTER_SET_NAME']) ->setCollation($column['COLLATION_NAME']) ->setColumnType($column['COLUMN_TYPE']) - ->setNullable($column['IS_NULLABLE'] == 'YES'); + ->setNullable($column['IS_NULLABLE'] == 'YES') + ->setAutoIncrement($auto_increment); $table_schema->addColumn($column_schema); } diff --git a/src/applications/config/schema/PhabricatorConfigSchemaSpec.php b/src/applications/config/schema/PhabricatorConfigSchemaSpec.php --- a/src/applications/config/schema/PhabricatorConfigSchemaSpec.php +++ b/src/applications/config/schema/PhabricatorConfigSchemaSpec.php @@ -102,14 +102,15 @@ } $details = $this->getDetailsForDataType($type); - list($column_type, $charset, $collation, $nullable) = $details; + list($column_type, $charset, $collation, $nullable, $auto) = $details; $column = $this->newColumn($name) ->setDataType($type) ->setColumnType($column_type) ->setCharacterSet($charset) ->setCollation($collation) - ->setNullable($nullable); + ->setNullable($nullable) + ->setAutoIncrement($auto); $table->addColumn($column); } @@ -162,7 +163,7 @@ $object->getApplicationName(), PhabricatorEdgeConfig::TABLE_NAME_EDGEDATA, array( - 'id' => 'id', + 'id' => 'auto', 'data' => 'text', ), array( @@ -233,6 +234,7 @@ $column_type = null; $charset = null; $collation = null; + $auto = false; // If the type ends with "?", make the column nullable. $nullable = false; @@ -246,6 +248,14 @@ // totally disallowed in a MODIFY statement vs a CREATE TABLE statement. switch ($data_type) { + case 'auto': + $column_type = 'int(10) unsigned'; + $auto = true; + break; + case 'auto64': + $column_type = 'bigint(20) unsigned'; + $auto = true; + break; case 'id': case 'epoch': case 'uint32': @@ -392,7 +402,7 @@ break; } - return array($column_type, $charset, $collation, $nullable); + return array($column_type, $charset, $collation, $nullable, $auto); } } diff --git a/src/applications/config/schema/PhabricatorConfigStorageSchema.php b/src/applications/config/schema/PhabricatorConfigStorageSchema.php --- a/src/applications/config/schema/PhabricatorConfigStorageSchema.php +++ b/src/applications/config/schema/PhabricatorConfigStorageSchema.php @@ -15,6 +15,7 @@ const ISSUE_LONGKEY = 'longkey'; const ISSUE_SUBWARN = 'subwarn'; const ISSUE_SUBFAIL = 'subfail'; + const ISSUE_AUTOINCREMENT = 'autoincrement'; const STATUS_OKAY = 'okay'; const STATUS_WARN = 'warn'; @@ -124,6 +125,8 @@ return pht('Subschemata Have Warnings'); case self::ISSUE_SUBFAIL: return pht('Subschemata Have Failures'); + case self::ISSUE_AUTOINCREMENT: + return pht('Column has Wrong Autoincrement'); default: throw new Exception(pht('Unknown schema issue "%s"!', $issue)); } @@ -157,6 +160,8 @@ return pht('Subschemata have setup warnings.'); case self::ISSUE_SUBFAIL: return pht('Subschemata have setup failures.'); + case self::ISSUE_AUTOINCREMENT: + return pht('This column has the wrong autoincrement setting.'); default: throw new Exception(pht('Unknown schema issue "%s"!', $issue)); } @@ -178,6 +183,7 @@ case self::ISSUE_UNIQUE: case self::ISSUE_KEYCOLUMNS: case self::ISSUE_LONGKEY: + case self::ISSUE_AUTOINCREMENT: return self::STATUS_WARN; default: throw new Exception(pht('Unknown schema issue "%s"!', $issue)); diff --git a/src/applications/fact/storage/PhabricatorFactAggregate.php b/src/applications/fact/storage/PhabricatorFactAggregate.php --- a/src/applications/fact/storage/PhabricatorFactAggregate.php +++ b/src/applications/fact/storage/PhabricatorFactAggregate.php @@ -9,7 +9,7 @@ public function getConfiguration() { return array( self::CONFIG_COLUMN_SCHEMA => array( - 'id' => 'id64', + 'id' => 'auto64', 'factType' => 'text32', 'valueX' => 'uint64', ), diff --git a/src/applications/fact/storage/PhabricatorFactRaw.php b/src/applications/fact/storage/PhabricatorFactRaw.php --- a/src/applications/fact/storage/PhabricatorFactRaw.php +++ b/src/applications/fact/storage/PhabricatorFactRaw.php @@ -15,7 +15,7 @@ public function getConfiguration() { return array( self::CONFIG_COLUMN_SCHEMA => array( - 'id' => 'id64', + 'id' => 'auto64', 'factType' => 'text32', 'objectA' => 'phid', 'valueX' => 'sint64', diff --git a/src/applications/harbormaster/storage/HarbormasterSchemaSpec.php b/src/applications/harbormaster/storage/HarbormasterSchemaSpec.php --- a/src/applications/harbormaster/storage/HarbormasterSchemaSpec.php +++ b/src/applications/harbormaster/storage/HarbormasterSchemaSpec.php @@ -24,7 +24,7 @@ id(new HarbormasterBuildable())->getApplicationName(), 'harbormaster_buildlogchunk', array( - 'id' => 'id', + 'id' => 'auto', 'logID' => 'id', 'encoding' => 'text32', diff --git a/src/applications/project/storage/PhabricatorProjectSchemaSpec.php b/src/applications/project/storage/PhabricatorProjectSchemaSpec.php --- a/src/applications/project/storage/PhabricatorProjectSchemaSpec.php +++ b/src/applications/project/storage/PhabricatorProjectSchemaSpec.php @@ -24,7 +24,7 @@ id(new PhabricatorProject())->getApplicationName(), PhabricatorProject::TABLE_DATASOURCE_TOKEN, array( - 'id' => 'id', + 'id' => 'auto', 'projectID' => 'id', 'token' => 'text128', ), diff --git a/src/applications/repository/storage/PhabricatorRepositorySchemaSpec.php b/src/applications/repository/storage/PhabricatorRepositorySchemaSpec.php --- a/src/applications/repository/storage/PhabricatorRepositorySchemaSpec.php +++ b/src/applications/repository/storage/PhabricatorRepositorySchemaSpec.php @@ -29,7 +29,7 @@ id(new PhabricatorRepository())->getApplicationName(), PhabricatorRepository::TABLE_COVERAGE, array( - 'id' => 'id', + 'id' => 'auto', 'branchID' => 'id', 'commitID' => 'id', 'pathID' => 'id', @@ -70,7 +70,7 @@ id(new PhabricatorRepository())->getApplicationName(), PhabricatorRepository::TABLE_LINTMESSAGE, array( - 'id' => 'id', + 'id' => 'auto', 'branchID' => 'id', 'path' => 'text', 'line' => 'uint32', @@ -100,7 +100,7 @@ id(new PhabricatorRepository())->getApplicationName(), PhabricatorRepository::TABLE_PARENTS, array( - 'id' => 'id', + 'id' => 'auto', 'childCommitID' => 'id', 'parentCommitID' => 'id', ), @@ -122,7 +122,7 @@ id(new PhabricatorRepository())->getApplicationName(), PhabricatorRepository::TABLE_PATH, array( - 'id' => 'id', + 'id' => 'auto', 'path' => 'text', 'pathHash' => 'bytes32', ), diff --git a/src/applications/tokens/storage/PhabricatorTokenCount.php b/src/applications/tokens/storage/PhabricatorTokenCount.php --- a/src/applications/tokens/storage/PhabricatorTokenCount.php +++ b/src/applications/tokens/storage/PhabricatorTokenCount.php @@ -10,6 +10,7 @@ self::CONFIG_IDS => self::IDS_MANUAL, self::CONFIG_TIMESTAMPS => false, self::CONFIG_COLUMN_SCHEMA => array( + 'id' => 'auto', 'tokenCount' => 'uint32', ), self::CONFIG_KEY_SCHEMA => array( diff --git a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerArchiveTask.php b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerArchiveTask.php --- a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerArchiveTask.php +++ b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerArchiveTask.php @@ -10,7 +10,12 @@ protected $result; public function getConfiguration() { - $config = parent::getConfiguration(); + $config = array( + // We manage the IDs in this table; they are allocated in the ActiveTask + // table and moved here without alteration. + self::CONFIG_IDS => self::IDS_MANUAL, + ) + parent::getConfiguration(); + $config[self::CONFIG_COLUMN_SCHEMA] = array( 'result' => 'uint32', diff --git a/src/infrastructure/storage/lisk/LiskDAO.php b/src/infrastructure/storage/lisk/LiskDAO.php --- a/src/infrastructure/storage/lisk/LiskDAO.php +++ b/src/infrastructure/storage/lisk/LiskDAO.php @@ -1744,8 +1744,15 @@ $binary_map = $this->getBinaryColumns(); + $id_mechanism = $this->getConfigOption(self::CONFIG_IDS); + if ($id_mechanism == self::IDS_AUTOINCREMENT) { + $id_type = 'auto'; + } else { + $id_type = 'id'; + } + $builtin = array( - 'id' => 'id', + 'id' => $id_type, 'phid' => 'phid', 'viewPolicy' => 'policy', 'editPolicy' => 'policy', diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementAdjustWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementAdjustWorkflow.php --- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementAdjustWorkflow.php +++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementAdjustWorkflow.php @@ -132,68 +132,103 @@ $failed = array(); - // We make changes in three phases: - // - // Phase 0: Drop all keys which we're going to adjust. This prevents them - // from interfering with column changes. - // - // Phase 1: Apply all database, table, and column changes. - // - // Phase 2: Restore adjusted keys. - $phases = 3; + // We make changes in several phases. + $phases = array( + // Drop surplus autoincrements. This allows us to drop primary keys on + // autoincrement columns. + 'drop_auto', + + // Drop all keys we're going to adjust. This prevents them from + // interfering with column changes. + 'drop_keys', + + // Apply all database, table, and column changes. + 'main', + + // Restore adjusted keys. + 'add_keys', + + // Add missing autoincrements. + 'add_auto', + ); $bar = id(new PhutilConsoleProgressBar()) - ->setTotal(count($adjustments) * $phases); + ->setTotal(count($adjustments) * count($phases)); - for ($phase = 0; $phase < $phases; $phase++) { + foreach ($phases as $phase) { foreach ($adjustments as $adjust) { try { switch ($adjust['kind']) { case 'database': - if ($phase != 1) { - break; + if ($phase == 'main') { + queryfx( + $conn, + 'ALTER DATABASE %T CHARACTER SET = %s COLLATE = %s', + $adjust['database'], + $adjust['charset'], + $adjust['collation']); } - queryfx( - $conn, - 'ALTER DATABASE %T CHARACTER SET = %s COLLATE = %s', - $adjust['database'], - $adjust['charset'], - $adjust['collation']); break; case 'table': - if ($phase != 1) { - break; + if ($phase == 'main') { + queryfx( + $conn, + 'ALTER TABLE %T.%T COLLATE = %s', + $adjust['database'], + $adjust['table'], + $adjust['collation']); } - queryfx( - $conn, - 'ALTER TABLE %T.%T COLLATE = %s', - $adjust['database'], - $adjust['table'], - $adjust['collation']); break; case 'column': - if ($phase != 1) { - break; - } - $parts = array(); - if ($adjust['charset']) { - $parts[] = qsprintf( - $conn, - 'CHARACTER SET %Q COLLATE %Q', - $adjust['charset'], - $adjust['collation']); + $apply = false; + $auto = false; + $new_auto = idx($adjust, 'auto'); + if ($phase == 'drop_auto') { + if ($new_auto === false) { + $apply = true; + $auto = false; + } + } else if ($phase == 'main') { + $apply = true; + if ($new_auto === false) { + $auto = false; + } else { + $auto = $adjust['is_auto']; + } + } else if ($phase == 'add_auto') { + if ($new_auto === true) { + $apply = true; + $auto = true; + } } - queryfx( - $conn, - 'ALTER TABLE %T.%T MODIFY %T %Q %Q %Q', - $adjust['database'], - $adjust['table'], - $adjust['name'], - $adjust['type'], - implode(' ', $parts), - $adjust['nullable'] ? 'NULL' : 'NOT NULL'); + if ($apply) { + $parts = array(); + + if ($auto) { + $parts[] = qsprintf( + $conn, + 'AUTO_INCREMENT'); + } + + if ($adjust['charset']) { + $parts[] = qsprintf( + $conn, + 'CHARACTER SET %Q COLLATE %Q', + $adjust['charset'], + $adjust['collation']); + } + queryfx( + $conn, + 'ALTER TABLE %T.%T MODIFY %T %Q %Q %Q', + $adjust['database'], + $adjust['table'], + $adjust['name'], + $adjust['type'], + implode(' ', $parts), + $adjust['nullable'] ? 'NULL' : 'NOT NULL'); + } break; case 'key': if (($phase == 0) && $adjust['exists']) { @@ -298,6 +333,7 @@ $issue_columns = PhabricatorConfigStorageSchema::ISSUE_KEYCOLUMNS; $issue_unique = PhabricatorConfigStorageSchema::ISSUE_UNIQUE; $issue_longkey = PhabricatorConfigStorageSchema::ISSUE_LONGKEY; + $issue_auto = PhabricatorConfigStorageSchema::ISSUE_AUTOINCREMENT; $adjustments = array(); foreach ($comp->getDatabases() as $database_name => $database) { @@ -368,6 +404,9 @@ if ($column->hasIssue($issue_columntype)) { $issues[] = $issue_columntype; } + if ($column->hasIssue($issue_auto)) { + $issues[] = $issue_auto; + } if ($issues) { if ($expect_column->getCharacterSet() === null) { @@ -380,8 +419,7 @@ $collation = $expect_column->getCollation(); } - - $adjustments[] = array( + $adjustment = array( 'kind' => 'column', 'database' => $database_name, 'table' => $table_name, @@ -394,7 +432,17 @@ // NOTE: We don't adjust column nullability because it is // dangerous, so always use the current nullability. 'nullable' => $actual_column->getNullable(), + + // NOTE: This always stores the current value, because we have + // to make these updates separately. + 'is_auto' => $actual_column->getAutoIncrement(), ); + + if ($column->hasIssue($issue_auto)) { + $adjustment['auto'] = $expect_column->getAutoIncrement(); + } + + $adjustments[] = $adjustment; } }