Support AUTO_INCREMENT in bin/storage adjust

Summary:
Ref T1191. When changing the column type of an AUTO_INCREMENT column, we currently may lose the autoincrement attribute.

Instead, support it. This is a bit messy because AUTO_INCREMENT columns interact with PRIMARY KEY columns (tables may only have one AUTO_INCREMENT column, and it must be a primary key). We need to migrate in more phases to avoid this issue.

Introduce new `auto` and `auto64` types to represent autoincrement IDs.

Test Plan:
  - Saw autoincrement show up correctly in web UI.
  - Fixed an autoincrement issue on the XHProf storage table with `bin/storage adjust` safely.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1191

Differential Revision: https://secure.phabricator.com/D10607

src/applications/cache/storage/PhabricatorCacheSchemaSpec.php

@@ -9,7 +9,7 @@ final class PhabricatorCacheSchemaSpec extends PhabricatorConfigSchemaSpec {
       'cache',
       id(new PhabricatorKeyValueDatabaseCache())->getTableName(),
       array(
-        'id' => 'id64',
+        'id' => 'auto64',
         'cacheKeyHash' => 'bytes12',
         'cacheKey' => 'text128',
         'cacheFormat' => 'text16',

src/applications/conduit/storage/PhabricatorConduitMethodCallLog.php

@@ -13,7 +13,7 @@ final class PhabricatorConduitMethodCallLog
   public function getConfiguration() {
     return array(
       self::CONFIG_COLUMN_SCHEMA => array(
-        'id' => 'id64',
+        'id' => 'auto64',
         'connectionID' => 'id64?',
         'method' => 'text64',
         'error' => 'text255',

src/applications/config/controller/PhabricatorConfigDatabaseStatusController.php

@@ -282,6 +282,7 @@ final class PhabricatorConfigDatabaseStatusController
     $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) {
@@ -339,6 +340,9 @@ final class PhabricatorConfigDatabaseStatusController
         $this->renderAttr(
           $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)),
@@ -356,6 +360,7 @@ final class PhabricatorConfigDatabaseStatusController
           pht('Data Type'),
           pht('Column Type'),
           pht('Nullable'),
+          pht('Autoincrement'),
           pht('Character Set'),
           pht('Collation'),
         ))
@@ -366,6 +371,7 @@ final class PhabricatorConfigDatabaseStatusController
           null,
           null,
           null,
+          null,
           null
         ));
 
@@ -521,11 +527,13 @@ final class PhabricatorConfigDatabaseStatusController
       $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 @@ final class PhabricatorConfigDatabaseStatusController
       $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 @@ final class PhabricatorConfigDatabaseStatusController
           pht('Expected Nullable'),
           $this->renderBoolean($expect_nullable),
         ),
+        array(
+          pht('Autoincrement'),
+          $this->renderBoolean($actual_auto),
+        ),
+        array(
+          pht('Expected Autoincrement'),
+          $this->renderBoolean($expect_auto),
+        ),
       ),
       $column->getIssues());
 

src/applications/config/schema/PhabricatorConfigColumnSchema.php

@@ -8,6 +8,16 @@ final class PhabricatorConfigColumnSchema
   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 @@ final class PhabricatorConfigColumnSchema
       $issues[] = self::ISSUE_NULLABLE;
     }
 
+    if ($this->getAutoIncrement() !== $expect->getAutoIncrement()) {
+      $issues[] = self::ISSUE_AUTOINCREMENT;
+    }
+
     return $issues;
   }
 

src/applications/config/schema/PhabricatorConfigSchemaQuery.php

@@ -60,7 +60,7 @@ final class PhabricatorConfigSchemaQuery extends Phobject {
       $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 @@ final class PhabricatorConfigSchemaQuery extends Phobject {
 
         $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);
         }

src/applications/config/schema/PhabricatorConfigSchemaSpec.php

@@ -102,14 +102,15 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject {
       }
 
       $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 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject {
       $object->getApplicationName(),
       PhabricatorEdgeConfig::TABLE_NAME_EDGEDATA,
       array(
-        'id' => 'id',
+        'id' => 'auto',
         'data' => 'text',
       ),
       array(
@@ -233,6 +234,7 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject {
     $column_type = null;
     $charset = null;
     $collation = null;
+    $auto = false;
 
     // If the type ends with "?", make the column nullable.
     $nullable = false;
@@ -246,6 +248,14 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject {
     // 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 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject {
         break;
     }
 
-    return array($column_type, $charset, $collation, $nullable);
+    return array($column_type, $charset, $collation, $nullable, $auto);
   }
 
 }

src/applications/config/schema/PhabricatorConfigStorageSchema.php

@@ -15,6 +15,7 @@ abstract class PhabricatorConfigStorageSchema extends Phobject {
   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 @@ abstract class PhabricatorConfigStorageSchema extends Phobject {
         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 @@ abstract class PhabricatorConfigStorageSchema extends Phobject {
         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 @@ abstract class PhabricatorConfigStorageSchema extends Phobject {
       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));

src/applications/fact/storage/PhabricatorFactAggregate.php

@@ -9,7 +9,7 @@ final class PhabricatorFactAggregate extends PhabricatorFactDAO {
   public function getConfiguration() {
     return array(
       self::CONFIG_COLUMN_SCHEMA => array(
-        'id' => 'id64',
+        'id' => 'auto64',
         'factType' => 'text32',
         'valueX' => 'uint64',
       ),

src/applications/fact/storage/PhabricatorFactRaw.php

@@ -15,7 +15,7 @@ final class PhabricatorFactRaw extends PhabricatorFactDAO {
   public function getConfiguration() {
     return array(
       self::CONFIG_COLUMN_SCHEMA => array(
-        'id' => 'id64',
+        'id' => 'auto64',
         'factType' => 'text32',
         'objectA' => 'phid',
         'valueX' => 'sint64',

src/applications/harbormaster/storage/HarbormasterSchemaSpec.php

@@ -24,7 +24,7 @@ final class HarbormasterSchemaSpec extends PhabricatorConfigSchemaSpec {
       id(new HarbormasterBuildable())->getApplicationName(),
       'harbormaster_buildlogchunk',
       array(
-        'id' => 'id',
+        'id' => 'auto',
         'logID' => 'id',
         'encoding' => 'text32',
 

src/applications/project/storage/PhabricatorProjectSchemaSpec.php

@@ -24,7 +24,7 @@ final class PhabricatorProjectSchemaSpec extends PhabricatorConfigSchemaSpec {
       id(new PhabricatorProject())->getApplicationName(),
       PhabricatorProject::TABLE_DATASOURCE_TOKEN,
       array(
-        'id' => 'id',
+        'id' => 'auto',
         'projectID' => 'id',
         'token' => 'text128',
       ),

src/applications/repository/storage/PhabricatorRepositorySchemaSpec.php

@@ -29,7 +29,7 @@ final class PhabricatorRepositorySchemaSpec
       id(new PhabricatorRepository())->getApplicationName(),
       PhabricatorRepository::TABLE_COVERAGE,
       array(
-        'id' => 'id',
+        'id' => 'auto',
         'branchID' => 'id',
         'commitID' => 'id',
         'pathID' => 'id',
@@ -70,7 +70,7 @@ final class PhabricatorRepositorySchemaSpec
       id(new PhabricatorRepository())->getApplicationName(),
       PhabricatorRepository::TABLE_LINTMESSAGE,
       array(
-        'id' => 'id',
+        'id' => 'auto',
         'branchID' => 'id',
         'path' => 'text',
         'line' => 'uint32',
@@ -100,7 +100,7 @@ final class PhabricatorRepositorySchemaSpec
       id(new PhabricatorRepository())->getApplicationName(),
       PhabricatorRepository::TABLE_PARENTS,
       array(
-        'id' => 'id',
+        'id' => 'auto',
         'childCommitID' => 'id',
         'parentCommitID' => 'id',
       ),
@@ -122,7 +122,7 @@ final class PhabricatorRepositorySchemaSpec
       id(new PhabricatorRepository())->getApplicationName(),
       PhabricatorRepository::TABLE_PATH,
       array(
-        'id' => 'id',
+        'id' => 'auto',
         'path' => 'text',
         'pathHash' => 'bytes32',
       ),

src/applications/tokens/storage/PhabricatorTokenCount.php

@@ -10,6 +10,7 @@ final class PhabricatorTokenCount extends PhabricatorTokenDAO {
       self::CONFIG_IDS => self::IDS_MANUAL,
       self::CONFIG_TIMESTAMPS => false,
       self::CONFIG_COLUMN_SCHEMA => array(
+        'id' => 'auto',
         'tokenCount' => 'uint32',
       ),
       self::CONFIG_KEY_SCHEMA => array(

src/infrastructure/daemon/workers/storage/PhabricatorWorkerArchiveTask.php

@@ -10,7 +10,12 @@ final class PhabricatorWorkerArchiveTask extends PhabricatorWorkerTask {
   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',

src/infrastructure/storage/lisk/LiskDAO.php

@@ -1744,8 +1744,15 @@ abstract class LiskDAO {
 
     $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',

src/infrastructure/storage/management/workflow/PhabricatorStorageManagementAdjustWorkflow.php

@@ -132,68 +132,103 @@ final class PhabricatorStorageManagementAdjustWorkflow
 
     $failed = array();
 
-    // We make changes in three phases:
+    // We make changes in several phases.
-    //
+    $phases = array(
-    // Phase 0: Drop all keys which we're going to adjust. This prevents them
+      // Drop surplus autoincrements. This allows us to drop primary keys on
-    // from interfering with column changes.
+      // autoincrement columns.
-    //
+      'drop_auto',
-    // Phase 1: Apply all database, table, and column changes.
+
-    //
+      // Drop all keys we're going to adjust. This prevents them from
-    // Phase 2: Restore adjusted keys.
+      // interfering with column changes.
-    $phases = 3;
+      '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) {
+              if ($phase == 'main') {
-                break;
+                queryfx(
-              }
-              queryfx(
-                $conn,
-                'ALTER DATABASE %T CHARACTER SET = %s COLLATE = %s',
-                $adjust['database'],
-                $adjust['charset'],
-                $adjust['collation']);
-              break;
-            case 'table':
-              if ($phase != 1) {
-                break;
-              }
-              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',
+                  'ALTER DATABASE %T CHARACTER SET = %s COLLATE = %s',
+                  $adjust['database'],
                   $adjust['charset'],
                   $adjust['collation']);
               }
+              break;
+            case 'table':
+              if ($phase == 'main') {
+                queryfx(
+                  $conn,
+                  'ALTER TABLE %T.%T COLLATE = %s',
+                  $adjust['database'],
+                  $adjust['table'],
+                  $adjust['collation']);
+              }
+              break;
+            case 'column':
+              $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(
+              if ($apply) {
-                $conn,
+                $parts = array();
-                '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 ($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 @@ final class PhabricatorStorageManagementAdjustWorkflow
     $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 @@ final class PhabricatorStorageManagementAdjustWorkflow
           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 @@ final class PhabricatorStorageManagementAdjustWorkflow
               $collation = $expect_column->getCollation();
             }
 
-
+            $adjustment = array(
-            $adjustments[] = array(
               'kind' => 'column',
               'database' => $database_name,
               'table' => $table_name,
@@ -394,7 +432,17 @@ final class PhabricatorStorageManagementAdjustWorkflow
               // 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;
           }
         }