Change column change entries to store indices as well

We need this to produce consistent column orders after diff application
and after inverse of drop.

Author: dvdkon

docs/changeset-format.md

@@ -87,6 +87,7 @@ This entry adds a new column to an existing table. All existing rows will have
 
 - 1 byte: Constant 0x63 (lowercase 'c')
 - Null-terminated string: Table name
+- Varint: Zero-based index the column has after being added.
 - Table column info.
 
 ## Drop column entry
@@ -97,6 +98,7 @@ rebasing and inverting the changeset.
 
 - 1 byte: Constant 0x43 (uppercase 'C')
 - Null-terminated string: Table name
+- Varint: Zero-based index the column has before being dropped.
 - Table column info.
 
 # Record Format

docs/schema-changes.md

@@ -3,6 +3,10 @@
 Geodiff supports diffing databases with different schemata. It identifies table
 and column additions/deletions.
 
+Columns are identified by their index as well as their name, and are added and
+dropped at a specific index, so that applying a diff reproduces the column order
+of the modified database.
+
 Tables and columns are always created empty and any data present in the
 database is recreated manually via `INSERT`/`UPDATE` entries, written after the
 schema change entry. Likewise, deletion entries expect the table/column to be
@@ -24,3 +28,8 @@ be moved. Same with renaming tables.
 
 The intermediate states created by applying the resulting diff (e.g. "nulling
 out" column before dropping it) may conflict with database constraints.
+
+Adding a column somewhere else than at the end of the column list is not always
+possible without modifying the database around it (e.g. dropping constraints).
+We currently don't do that and just fail if a column addition is not easily
+possible.

geodiff/src/changeset.h

@@ -287,13 +287,17 @@ struct ChangesetDropTableEntry
 struct ChangesetAddColumnEntry
 {
   std::string tableName;
+  //! Zero-based index of new column after creation
+  size_t columnIdx;
   TableColumnInfo column;
 };
 
 //! Entry for ALTER TABLE ... DROP COLUMN command
 struct ChangesetDropColumnEntry
 {
   std::string tableName;
+  //! Zero-based index of column before deletion
+  size_t columnIdx;
   TableColumnInfo column;
 };
 
@@ -320,6 +324,19 @@ struct ChangesetEntry : public std::variant <
       return ChangesetEntryType::OpDropColumn;
     throw std::invalid_argument( "Unreachable - operationType()" );
   }
+
+  std::string schemaChangeTableName() const
+  {
+    if ( const ChangesetCreateTableEntry *ctEntry = std::get_if<ChangesetCreateTableEntry>( this ) )
+      return ctEntry->tableName;
+    else if ( const ChangesetDropTableEntry *dtEntry = std::get_if<ChangesetDropTableEntry>( this ) )
+      return dtEntry->tableName;
+    else if ( const ChangesetAddColumnEntry *acEntry = std::get_if<ChangesetAddColumnEntry>( this ) )
+      return acEntry->tableName;
+    else if ( const ChangesetDropColumnEntry *dcEntry = std::get_if<ChangesetDropColumnEntry>( this ) )
+      return dcEntry->tableName;
+    return "";
+  }
 };
 
 #endif // CHANGESET_H

geodiff/src/changesetconcat.cpp

@@ -246,9 +246,9 @@ void concatChangesets(
       }
       else if ( ChangesetDropColumnEntry *dcEntry = std::get_if<ChangesetDropColumnEntry>( &fullEntry ) )
       {
-        // This entry only contains the column's name, not its index, so we
-        // can't apply its effects to the existing entries. The best we can do
-        // is just forward this entry.
+        // Short-circuting column removals is hard, since we can't just prepend
+        // it to other entries (the column needs to be NULLed out first). So
+        // for now we treat it as a barrier.
         phase[dcEntry->tableName].entries.push_back( *dcEntry );
         // We also need to start a new phase, since we can't merge entries
         // anymore.
@@ -273,9 +273,9 @@ void concatChangesets(
             newColumnCount = existingDEntry->table->columnCount() + 1;
           if ( existingDEntry->table->columnCount() != newColumnCount )
             existingDEntry->table->primaryKeys.push_back( false );
-          if ( existingDEntry->oldValues.size() != newColumnCount )
+          if ( existingDEntry->oldValues.size() != 0 && existingDEntry->oldValues.size() != newColumnCount )
             existingDEntry->oldValues.push_back( Value::makeNull() );
-          if ( existingDEntry->newValues.size() != newColumnCount )
+          if ( existingDEntry->newValues.size() != 0 && existingDEntry->newValues.size() != newColumnCount )
             existingDEntry->newValues.push_back( Value::makeNull() );
         }
       }

geodiff/src/changesetreader.cpp

@@ -280,6 +280,7 @@ ChangesetAddColumnEntry ChangesetReader::readAddColumnEntry()
 {
   ChangesetAddColumnEntry entry;
   entry.tableName = readNullTerminatedString();
+  entry.columnIdx = readVarint();
   entry.column = readColumnInfo();
   return entry;
 }
@@ -288,6 +289,7 @@ ChangesetDropColumnEntry ChangesetReader::readDropColumnEntry()
 {
   ChangesetDropColumnEntry entry;
   entry.tableName = readNullTerminatedString();
+  entry.columnIdx = readVarint();
   entry.column = readColumnInfo();
   return entry;
 }

geodiff/src/changesetutils.cpp

@@ -83,6 +83,9 @@ std::vector<ChangesetEntry> invertChangesetReverse( ChangesetReader &reader )
     {
       ChangesetDropColumnEntry out;
       out.tableName = acEntry->tableName;
+      // The index of the added column before its deletion is the same as after
+      // its addition, so it can be carried over as-is.
+      out.columnIdx = acEntry->columnIdx;
       out.column = acEntry->column;
       invertedEntries.push_back( out );
     }
@@ -97,6 +100,7 @@ std::vector<ChangesetEntry> invertChangesetReverse( ChangesetReader &reader )
     {
       ChangesetAddColumnEntry out;
       out.tableName = dcEntry->tableName;
+      out.columnIdx = dcEntry->columnIdx;
       out.column = dcEntry->column;
       invertedEntries.push_back( out );
     }
@@ -277,6 +281,7 @@ nlohmann::json changesetEntryToJSON( const ChangesetEntry &entry )
     nlohmann::json res;
     res["type"] = "add_column";
     res["tableName"] = acEntry->tableName;
+    res["columnIdx"] = acEntry->columnIdx;
     res["column"] = columnInfoToJSON( acEntry->column );
     return res;
   }
@@ -285,6 +290,7 @@ nlohmann::json changesetEntryToJSON( const ChangesetEntry &entry )
     nlohmann::json res;
     res["type"] = "drop_column";
     res["tableName"] = dcEntry->tableName;
+    res["columnIdx"] = dcEntry->columnIdx;
     res["column"] = columnInfoToJSON( dcEntry->column );
     return res;
   }

geodiff/src/changesetwriter.cpp

@@ -174,12 +174,14 @@ void ChangesetWriter::writeAddColumnEntry( const ChangesetAddColumnEntry &entry
 {
   writeByte( static_cast<char>( ChangesetEntryType::OpAddColumn ) );
   writeNullTerminatedString( entry.tableName );
+  writeVarint( entry.columnIdx );
   writeColumnInfo( entry.column );
 }
 
 void ChangesetWriter::writeDropColumnEntry( const ChangesetDropColumnEntry &entry )
 {
   writeByte( static_cast<char>( ChangesetEntryType::OpDropColumn ) );
   writeNullTerminatedString( entry.tableName );
+  writeVarint( entry.columnIdx );
   writeColumnInfo( entry.column );
 }